release/19.x: [builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc (#101662)#101847
release/19.x: [builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc (#101662)#101847tru merged 1 commit intollvm:release/19.xfrom
Conversation
|
@arichardson What do you think about merging this PR to the release branch? |
|
Any news on this? Should it be withdrawn considering there are issues with it? |
|
It's difficult: on one hand it fixes a Solaris/SPARC build failure. On the other, it's said to cause problems for an out-of-tree z/OS port. Unfortunately, the developers refuse to publish their code, so it's almost impossible to reason about that code. |
|
That does sound like it should be acceptable to merge if it's only blocking a out-of-tree implementation, since we don't officially support that config in that case. There is also the question as if we need to backport this - since if the main complaint for it not going into main is because of a external port is breaking might not be relevant in this case. |
|
I have concerns about this change just thinking about this from the Solaris point of view. A couple questions:
I take exception to the statement "the developers refuse to publish". The z/OS code is in the process of being published. No one has refused to publish code and we have worked with other contributors to compiler-rt to ensure the code builds on all platforms while we get the z/OS changes upstreamed. This change will need to be undone and a proper fix applied for Sparc when we enable the z/OS buildbots. I have asked @rorth to review #82789 and provide any changes required for Sparc. I haven't heard back yet. For z/OS, I am ok with this change in the 19.x release since we only depend on trunk and 18.x. We do need to find a proper solution for Sparc in trunk. |
|
I'll merge this for 19.x then. Hopefully when 20 rolls around it will work out of the box. |
…lvm#101662) `compiler-rt/lib/builtins/divtc3.c` and `multc3.c` don't compile on Solaris/sparcv9 with `gcc -m32`: ``` FAILED: projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-sparc.dir/divtc3.c.o [...] compiler-rt/lib/builtins/divtc3.c: In function ‘__divtc3’: compiler-rt/lib/builtins/divtc3.c:22:18: error: implicit declaration of function ‘__compiler_rt_logbtf’ [-Wimplicit-function-declaration] 22 | fp_t __logbw = __compiler_rt_logbtf( | ^~~~~~~~~~~~~~~~~~~~ ``` and many more. It turns out that while the definition of `__divtc3` is guarded with `CRT_HAS_F128`, the `__compiler_rt_logbtf` and other declarations use `CRT_HAS_128BIT && CRT_HAS_F128` as guard. This only shows up with `gcc` since, as documented in Issue llvm#41838, `clang` violates the SPARC psABI in not using 128-bit `long double`, so this code path isn't used. Fixed by changing the guards to match. Tested on `sparcv9-sun-solaris2.11`. (cherry picked from commit 63a7786)
|
@rorth (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport 63a7786
Requested by: @rorth