Open rorth opened 5 years ago
rorth
commented
3 years ago Thanks to your hints I've now been able to get the 128-bit long double support working. I've posted the current state of affairs at https://reviews.llvm.org/D89130 where I believe it's easier to discuss than in this PR.
rorth
commented
3 years ago 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 .
Nice. Unfortunately, this is clang-only, so ultimately a different solution will have to be chose to continue supporting builds with gcc. For the moment, it certainly helps me along.
fatal error: error in backend: SPARCv8 does not handle f128 in calls; pass indirectly
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.
That would certainly be helpful: I've looked at both SparcCallingConv.td and SparcISelLowering.cpp a bit, but am somewhat lost there.
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.
From a quick look, maybe the FP128TyID needs to be carried over from SparcV9; can't tell yet.
I must admit I'm pretty confused about the interaction of the LongDouble* variables above, resetDataLayout calls and maybe more. Besides, there's SparcCallingConv.td and support routines in llvm, so this tends to get more complex.
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.
Ah, thanks. Strangely SparcV8 already uses f128:64 in clang/lib/Basic/Targets/Sparc.h although clang otherwise uses 64-bit long double on SparcV8.
I've meanwhile found that despite the SVR4 SPARC psABI requiring 128-bit long double, several targets do their own thing here which complicates things tremendously:
Solaris/SPARC with -m32 always uses 128-bit long double, following the psABI
so do FreeBSD (but supports -mlong-double-64, too), OpenBSD (always 128-bit), sparc64--elf and sparc64--rtems
NetBSD/sparc (i.e. 32-bit kernel) uses 64-bit long double, NetBSD/sparc64 has 128-bit instead, optionally with support for -mlong-double-64
similarly, Linux/sparc goes for 64-bit, while Linux/sparc64 does 128-bit, both with the -mlong-double-64 option
With the possible exception of Linux/sparc64 (in the GCC compile farm) I can test none of those, so I'll leave them alone, allowing for the respective maintainers to make adjustments.
Right now there are only two failing tests due to this issue in a 2-stage build:
UBSan-Standalone-sparc :: TestCases/Float/cast-overflow.cpp UBSan-Standalone-sparc :: TestCases/Misc/log-path_test.cpp
I wonder if it wouldn't be appropriate to XFAIL them to try and get the Solaris/sparcv9 buildbot green finally, and either work in parallel on this issue or leave it to the Sparc maintainers?
rorth
commented
3 years ago I ran into quite a number of issues, however:
To make things worse, even on sparcv9 (where long double was supposed to be correct), I find quite a number of long double test failing in a 2-stage build:
Builtins-sparcv9-sunos :: addtf3_test.c Builtins-sparcv9-sunos :: divtf3_test.c Builtins-sparcv9-sunos :: extenddftf2_test.c Builtins-sparcv9-sunos :: extendsftf2_test.c Builtins-sparcv9-sunos :: floatditf_test.c Builtins-sparcv9-sunos :: floatsitf_test.c Builtins-sparcv9-sunos :: floattitf_test.c Builtins-sparcv9-sunos :: floatunditf_test.c Builtins-sparcv9-sunos :: floatunsitf_test.c Builtins-sparcv9-sunos :: floatuntitf_test.c Builtins-sparcv9-sunos :: multf3_test.c Builtins-sparcv9-sunos :: subtf3_test.c
while the tests PASS in a 1-stage build with gcc.
I haven't yet investigated more closely, but this shows that long double on sparc has a considerable number of issues, not just on 32-bit.
I've now analyzed this further and filed Bug 47729 for the (unrelated) issue.
efriedma-quic
commented
3 years ago 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 .
fatal error: error in backend: SPARCv8 does not handle f128 in calls; pass indirectly
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.
I must admit I'm pretty confused about the interaction of the LongDouble* variables above, resetDataLayout calls and maybe more. Besides, there's SparcCallingConv.td and support routines in llvm, so this tends to get more complex.
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.
rorth
commented
3 years ago 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:
The long double tests in compiler-rt assume __int128_t for 128-bit long double. However, that doesn't exist on 32-bit (sparc or otherwise).
I also ran into
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* variables above, resetDataLayout calls and maybe more. Besides, there's SparcCallingConv.td and support routines in llvm, so this tends to get more complex.
To make things worse, even on sparcv9 (where long double was supposed to be correct), I find quite a number of long double test failing in a 2-stage build:
Builtins-sparcv9-sunos :: addtf3_test.c Builtins-sparcv9-sunos :: divtf3_test.c Builtins-sparcv9-sunos :: extenddftf2_test.c Builtins-sparcv9-sunos :: extendsftf2_test.c Builtins-sparcv9-sunos :: floatditf_test.c Builtins-sparcv9-sunos :: floatsitf_test.c Builtins-sparcv9-sunos :: floattitf_test.c Builtins-sparcv9-sunos :: floatunditf_test.c Builtins-sparcv9-sunos :: floatunsitf_test.c Builtins-sparcv9-sunos :: floatuntitf_test.c Builtins-sparcv9-sunos :: multf3_test.c Builtins-sparcv9-sunos :: subtf3_test.c
while the tests PASS in a 1-stage build with gcc.
I haven't yet investigated more closely, but this shows that long double on sparc has a considerable number of issues, not just on 32-bit.
efriedma-quic
commented
3 years ago 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.)
rorth
commented
3 years ago I just discovered that things are even worse:
$ cat sald.c
int main (void) { printf ("sizeof (long double) = %ld\n", (long) sizeof (long double)); printf ("alignof(long double) = %ld\n", (long) alignof (long double)); return 0; } $ cc -m32 -o sald-cc sald.c $ ./sald-cc sizeof (long double) = 16 alignof(long double) = 8 $ gcc -m32 -o sald-gcc sald.c $ ./sald-gcc sizeof (long double) = 16 alignof(long double) = 8 $ clang -m32 -o sald-clang sald.c ./sald-clang sizeof (long double) = 8 alignof(long double) = 8
So clang even gets the size of long double wrong on 32-bit SPARC.
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).