Skip to content

Conversation

@koachan
Copy link
Contributor

@koachan koachan commented Dec 1, 2025

It appears that using it will result in callers mistakenly thinking that complex f128 returns is done the sret-way, when it should be returned in registers.

@koachan
Copy link
Contributor Author

koachan commented Dec 1, 2025

There's also another failure reported there: the frexpl_interceptor.cpp failure which seem to happen because the sanitizer parts are built with system LLVM, which still doesn't have the long double fixes, while the sources being pulled already enables all the long double tests.

Any idea what should I do for it?

@rorth
Copy link
Collaborator

rorth commented Dec 1, 2025

I fear this is just a hack at best:

  • I've run a 2-stage build on main without this patch: only two tests FAIL there:
     Builtins-sparc-sunos :: divtc3_test.c
     Builtins-sparc-sunos :: multc3_test.c
    
  • However, in a 1-stage build with clang-21, there are way more, as seen in the Solaris/sparcv9 buildbot:
    AddressSanitizer-sparc-sunos :: TestCases/frexpl_interceptor.cpp
    AddressSanitizer-sparc-sunos-dynamic :: TestCases/frexpl_interceptor.cpp
    Builtins-sparc-sunos :: addtf3_test.c
    Builtins-sparc-sunos :: divtc3_test.c
    Builtins-sparc-sunos :: divtf3_test.c
    Builtins-sparc-sunos :: extenddftf2_test.c
    Builtins-sparc-sunos :: extendsftf2_test.c
    Builtins-sparc-sunos :: fixtfdi_test.c
    Builtins-sparc-sunos :: fixtfsi_test.c
    Builtins-sparc-sunos :: fixtfti_test.c
    Builtins-sparc-sunos :: fixunstfdi_test.c
    Builtins-sparc-sunos :: fixunstfsi_test.c
    Builtins-sparc-sunos :: fixunstfti_test.c
    Builtins-sparc-sunos :: floatditf_test.c
    Builtins-sparc-sunos :: floatsitf_test.c
    Builtins-sparc-sunos :: floattitf_test.c
    Builtins-sparc-sunos :: floatunditf_test.c
    Builtins-sparc-sunos :: floatunsitf_test.c
    Builtins-sparc-sunos :: floatuntitf_test.c
    Builtins-sparc-sunos :: multc3_test.c
    Builtins-sparc-sunos :: multf3_test.c
    Builtins-sparc-sunos :: powitf2_test.c
    Builtins-sparc-sunos :: subtf3_test.c
    Builtins-sparc-sunos :: trunctfdf2_test.c
    Builtins-sparc-sunos :: trunctfsf2_test.c
    Flang :: Driver/gcc-triple.f90
    UBSan-AddressSanitizer-sparc :: TestCases/Float/cast-overflow.cpp
    UBSan-AddressSanitizer-sparc :: TestCases/Misc/log-path_test.cpp
    UBSan-Standalone-sparc :: TestCases/Float/cast-overflow.cpp
    UBSan-Standalone-sparc :: TestCases/Misc/log-path_test.cpp
    

The buildbot cannot run full two-stage builds for performance reasons: to address this, one would have to use a fixed main clang instead of the current clang-21 or backport the patch to the release/21.x branch.

Even for the 2-stage build, the two FAILing tests die with SIGILL:

Thread 2 received signal SIGILL, Illegal instruction.
[Switching to Thread 1 (LWP 1)]
0x00011500 in test.divtc3 ()
    at /vol/llvm/src/llvm-project/local/compiler-rt/test/builtins/Unit/divtc3_test.c:47
47	  Qcomplex r = __divtc3(a, b, c, d);
1: x/i $pc
=> 0x11500 <test__divtc3+432>:	illtrap  0x20
(gdb) bt
#0  0x00011500 in test.divtc3 ()
    at /vol/llvm/src/llvm-project/local/compiler-rt/test/builtins/Unit/divtc3_test.c:47
#1  0x000111a8 in main ()
    at /vol/llvm/src/llvm-project/local/compiler-rt/test/builtins/Unit/divtc3_test.c:356

I'm pretty certain that this patch is wrong at least for the 2-stage case (though I'm currently testing it). Besides, only the 32-bit tests FAIL in both cases, the 64-bit ones (sparcv9) work just fine.

Besides, the 32-bit libclang_rt.builtins-sparc.a is a total mess: e.g. up to LLVM-17 it did contain a definition of __multc3, which afterwards was lost. libgcc_s.so.1 has it two, as it should. The same is true for several of the other functions that cause the link failures in the 1-stage build:

__divtc3
__fixtfdi
__fixunstfdi
__floatditf
__floatunditf
__multc3
__powitf2

However, builtins has been badly been messed with in recent times, e.g. by the s390x port which is reponsible for some of this. Combine this with the lack of testing in the default runtimes builds and the breakage wasn't even noticed. I've meanwhile lost both time and patience to deal with issue since I don't believe it will ever be fixed (even the loss of builtins testing has been completely ignored so far).

@koachan
Copy link
Contributor Author

koachan commented Dec 1, 2025

Builtins-sparc-sunos :: divtc3_test.c
Builtins-sparc-sunos :: multc3_test.c

Even for the 2-stage build, the two FAILing tests die with SIGILL

From a cursory look this seems like a genuine case of ABI mismatch bug; the caller expects an sret return but the callee returns the value in the register. Will look into it, thanks!

For the rest...
Over here it turns out with or without this patch only the SIGILL ones failed too, so this might as well isn't needed.
If so then I'll close or rework this PR to fix the SIGILL tests for the time being at least (?)

@efriedma-quic
Copy link
Collaborator

even the loss of builtins testing has been completely ignored so far

#166837

@rorth
Copy link
Collaborator

rorth commented Dec 2, 2025

Builtins-sparc-sunos :: divtc3_test.c
Builtins-sparc-sunos :: multc3_test.c

Even for the 2-stage build, the two FAILing tests die with SIGILL

From a cursory look this seems like a genuine case of ABI mismatch bug; the caller expects an sret return but the callee returns the value in the register. Will look into it, thanks!

Good: I've now switched the buildbot over to a current clang-22 as build compiler, so the only remaining FAILs are the ones above.

For the rest... Over here it turns out with or without this patch only the SIGILL ones failed too, so this might as well isn't needed. If so then I'll close or rework this PR to fix the SIGILL tests for the time being at least (?)

Agreed: this matches my testing with and without this patch.

@rorth
Copy link
Collaborator

rorth commented Dec 2, 2025

even the loss of builtins testing has been completely ignored so far

#166837

Finally :-) That's great news, I'll give the patch a try myself.

It appears that using it will result in callers mistakenly thinking that
complex f128 returns is done the sret-way.
@koachan koachan changed the title [SPARC][builtins] Add sparcv9 arch name for 32-bit SPARC [SPARC] Remove CCIfConsecutiveRegs for f128 returns Dec 2, 2025
@koachan
Copy link
Contributor Author

koachan commented Dec 2, 2025

Okay, reworked the PR, it should fix the remaining SIGILL issues in 2-stage builds.

@rorth
Copy link
Collaborator

rorth commented Dec 2, 2025

Okay, reworked the PR, it should fix the remaining SIGILL issues in 2-stage builds.

It does indeed, thanks a lot. I'll wait until this PR has been committed, than replace the clang-22 on the bot with the fixed on, which should get it green again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants