-
Notifications
You must be signed in to change notification settings - Fork 12k
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
Passing long double args on 32-bit SPARC violates ABI #41838
Comments
I just discovered that things are even worse: $ cat sald.c int So clang even gets the size of long double wrong on 32-bit SPARC. |
That's easy to fix; changing the underlying type for long double just requires adding a few lines in clang/lib/Basic/Targets/ . (It's controlled by LongDoubleFormat/LongDoubleWidth/LongDoubleAlign.) |
That's what I tried, matching the sparcv9 case except for the smaller alignment. I ran into quite a number of issues, however:
fatal error: error in backend: SPARCv8 does not handle f128 in calls; pass indirectly I must admit I'm pretty confused about the interaction of the LongDouble*
|
Okay, maybe a little more complicated. :) For compiler-rt, see the bit that enables -fforce-enable-int128 for riscv in compiler-rt/lib/builtins/CMakeLists.txt .
Ideally, the fatal error here should be replaced with code to implement this; it doesn't make sense to refuse to lower calls involving f128, and optimizations are likely to explode if the backend doesn't implement it. Let me know if you want tips on implementing this. On the clang side, if you want to mess with the way calls are lowered to LLVM IR, the relevant code is clang/lib/CodeGen/TargetInfo.cpp . There's some SPARC-specific code, although it's pretty bare-bones at the moment.
In general, clang has its own view of types and memory layout, independent of LLVM IR. This is used, for example, to compute sizeof(). Theoretically, you could use the clang libraries to analyze code on a target that doesn't have an LLVM backend. The datalayout is only used when it comes time to emit IR. |
I've now analyzed this further and filed Bug 47729 for the (unrelated) issue. |
Nice. Unfortunately, this is clang-only, so ultimately a different solution
That would certainly be helpful: I've looked at both SparcCallingConv.td and
From a quick look, maybe the FP128TyID needs to be carried over from SparcV9;
Ah, thanks. Strangely SparcV8 already uses f128:64 in clang/lib/Basic/Targets/Sparc.h I've meanwhile found that despite the SVR4 SPARC psABI requiring 128-bit long
With the possible exception of Linux/sparc64 (in the GCC compile farm) I can Right now there are only two failing tests due to this issue in a 2-stage UBSan-Standalone-sparc :: TestCases/Float/cast-overflow.cpp I wonder if it wouldn't be appropriate to XFAIL them to try and get the |
Thanks to your hints I've now been able to get the 128-bit long double support |
As reported in Issue #41838, `clang` doesn't correctly implement `long double` on 32-bit Solaris/SPARC: the psABI requires this to be an 128-bit type. Four sanitizer tests currently `FAIL` for this reason. While there is a WIP patch to fix `clang` (D89130 <https://reviews.llvm.org/D89130>), it isn't complete yet and I've hit so many brick walls while trying to finish it that I'm unsure if I ever will. This patch therefore `XFAIL`s those tests in the meantime. Tested on `sparcv9-sun-solaris2.11`. Differential Revision: https://reviews.llvm.org/D119016
`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`.
For reference, on current LLVM the following LLVM IR (compiler explorer): target triple = "sparc-unknown-linux-gnu"
define void @f(fp128) {
ret void
} fails to compile with the following error message:
|
…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 #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`.
…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)
…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`.
…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`.
…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)
Extended Description
I'm current working to fix the remaining compiler-rt testsuite bugs on SPARC
(https://reviews.llvm.org/D40900). One of the failures is
While the sparcv9 failure is different, the sparc one boils down to incorrectly
passing long double arguments. Consider the following testcase:
$ cat caller.c
extern void callee (long double);
int
main (void)
{
long double ld = 1.0;
callee (ld);
return 0;
}
$ cat callee.c
extern void abort (void);
void
callee (long double ld)
{
if (ld != 1.0)
abort ();
}
$ clang -m32 -c caller.c -o caller.clang.o
$ clang -m32 -c callee.c -o callee.clang.o
$ gcc -m32 -c caller.c -o caller.gcc.o
$ gcc -m32 -c callee.c -o callee.gcc.o
$ gcc -m32 -o clang-gcc caller.clang.o callee.gcc.o
$ ./clang-gcc
Segmentation Fault (core dumped)
$ gcc -m32 -o gcc-clang caller.gcc.o callee.clang.o
$ ./gcc-clang
Abort (core dumped)
The SPARC psABI, p.3-15 (Structure, Union, and Quad-Precision Arguments)
requires long double args to be passed by reference, while clang passes
them by value (as is correct for SPARCv9).
The text was updated successfully, but these errors were encountered: