[compiler-rt] Tests fail on Darwin stage1 build after the ABI change of half type on X86

Hi folks,

I met this problem when I landing the _Float16 enablement patch.

A previous patch changed calling conversion of half type from GPRs to XMMs. This patch is to enable the _Float16 type support in Clang.

In theory, it won’t result in tests fail in compiler-rt because we firstly check if the _Float16 is supported on the target by cmake: https://github.com/llvm/llvm-project/blob/main/compiler-rt/cmake/builtin-config-ix.cmake#L25-L31 and switch the ABI accordingly: https://github.com/llvm/llvm-project/blob/527ef8ca981e88a35758c0e4143be6853ea26dfc/compiler-rt/test/builtins/CMakeLists.txt#L47-L50.

Before this patch, _Float16 in not supported by Clang. Thus, both compiler-rt and the tests are using GPR conversion. And after the patch, both will use XMM conversion.

It works well on Linux, but it fails on Darwin because (AFAIU) the native Clang on Darwin supports _Float16 even without this patch. So it will generate compiler-rt with GPR conversion while the tests expect the conversion is XMM.

I’m not very sure of my suspicion. The problem is I neither am expert in compiler-rt nor have Darwin environment. Does any folks have suggestion on how to solve the problem? Thanks in advance!

cc @fhahn

Maybe @TNorthover has any ideas.

Hello, I want to disable them on Darwin temporarily, see the latest revison on D128571. I’ll land it in one day if no objections for quick moving on to unblock other users. Thanks!

This isn’t my area of expertise, but when I was looking at this I noticed that this code that adds -DCOMPILER_RT_HAS_FLOAT16:

https://github.com/llvm/llvm-project/blob/8bc0bb956421e02bc1a1797363e979655dd326d6/compiler-rt/lib/builtins/CMakeLists.txt#L699

Is inside the else() for an if(APPLE) check, and I don’t see any code that would add this flag elsewhere when building the builtins library. But your review for ⚙ D128571 [X86] Support `_Float16` on SSE2 and up looks like it may be setting that -D when building the tests.

Thanks @blangmuir for taking look this. Great! I missed that. I think this probably the root cause. So I can exclude the change for APPLE ATM. This should be the best way to solve the fails. Updated on the new revision.

I’ll do a follow up to enable it for Darwin.