-
Notifications
You must be signed in to change notification settings - Fork 12.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc #101662
Conversation
`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-pick 63a7786 |
…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)
/pull-request #101847 |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/3153 Here is the relevant piece of the build log for the reference:
|
* 'main' of https://github.com/llvm/llvm-project: [RISCV] Improve hasAllNBitUsers for users of SLLI. [RISCV] Invert if conditions in the switch in RISCVDAGToDAGISel::hasAllNBitUsers. NFC [Transforms] Construct SmallVector with ArrayRef (NFC) (llvm#101851) [RISCV] Capitalize some variable names. NFC [sanitizer_common] Fix UnwindFast on SPARC (llvm#101634) [builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc (llvm#101662) [NFC][asan] Track current dynamic init module (llvm#101597) [libc] enable most of the entrypoints on aarch64 (llvm#101797) [SandboxIR][Tracker] Track InsertIntoBB (llvm#101595) [SCEV] Use const SCEV * explicitly in more places. [ELF] Move outputSections into Ctx. NFC [ELF] Move ElfSym into Ctx. NFC [ELF] Move Out into Ctx. NFC [test][asan] Fix the test checks [NFC][asan] Switch from list to DynInitGlobalsByModule (llvm#101596) Signed-off-by: Edwiin Kusuma Jaya <kutemeikito0905@gmail.com>
…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`.
@rorth Rainer, this change broke the following libcxx'`s lit tests on z/OS:
They all failed since after this change On z/OS we don't define CRT_HAD_128BIT and according to this snippet code from Would you be able to revert this change and come up with the proper fix for this? |
In a nutshell the issue is when |
Sorry for the failure.
Me knowing nothing about z/OS and not having access to such a system, could you please provide the output of the compilation of
Given that the patch has been in for two weeks without complaint, could you give me a day or two to investigate and come up with a fix? This avoid the constant churn of Revert/Reland/Re-Revert/Re-Reland. Thanks. |
Thank you for responding. The -save-temps
Waiting fews days should be fine with us,. |
There are a few things that confuse me, though:
All the functions use (
Good, thanks. |
Unfortunately the
Our downstream code has modified guard as follows:
Sorry I included wrong file, let me try again,
Unfortunately, we don't have CRT_HAS_TF_MODE defined. I will check how come calling |
His is the downstream code (not up streamed yet) guarded with
Maybe the guard for the
|
…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`.
If you can just call the This would be equivalent to |
We are fine with either one. |
Neither of those works for 32-bit Solaris/sparc with gcc 14: the declarations of The more I see this, the more I wonder if it's wise to make changes here that no in-tree target needs, arguing about a WIP z/OS port that may or may not need changes. Maybe it's better to keep things as is for now and revisit this when that port is submitted for real and can be reviewed/tested as is, rather than arguing about unknown downstream code. That would mean making whatever change is necessary to fix your issue locally for now. |
I would rather deal with this issue now when it's still fresh. Can you provide more details I'm confused, If |
The z/OS port is not a WIP. It's real and we are in the process of upstreaming everything. These breakages impact us and delay us being able to upstream everything. Alex and I worked on getting things working. I'd like to keep it working. |
…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)
compiler-rt/lib/builtins/divtc3.c
andmultc3.c
don't compile on Solaris/sparcv9 withgcc -m32
:and many more. It turns out that while the definition of
__divtc3
is guarded withCRT_HAS_F128
, the__compiler_rt_logbtf
and other declarations useCRT_HAS_128BIT && CRT_HAS_F128
as guard. This only shows up withgcc
since, as documented in Issue #41838,clang
violates the SPARC psABI in not using 128-bitlong double
, so this code path isn't used.Fixed by changing the guards to match.
Tested on
sparcv9-sun-solaris2.11
.