llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.51k stars 11.3k forks source link

Builtins-sparc-sunos:: divtc3_test.c XPASSes on Solaris/sparc #72398

Open rorth opened 9 months ago

rorth commented 9 months ago

As originally reported in [builtins] Guard the divtc3_test.c test with CRT_HAS_TF_MODE, that patch 77d75dc9be5c4bb1d9986a475e1c661718067c6a caused Builtins-sparc-sunos:: divtc3_test.c to XPASS on Solaris/sparc, breaking the Solaris/sparcv9 buildbot.

Some investigation into the failure that prompted the culprit patch have been reported in Issue #71971.

The same issues apply to 32-bit SPARC, with an additional compliction:

Both in LLVM 17 and main, librt_has_divtc3, is always true, which seems totallly wrong if the symbol is missing from libclang_rt.builtins-sparc.a.

It doesn't help either (though not an issue for the Solaris/sparcv9 buildbot) that the builtins tests aren't run in a runtimes build at all, missing any possible issues.

I guess for the moment the best way forward is to remove/disable the XFAIL in the test to turn the Solaris/sparcv9 green again after 5 days, with a prominent comment referring to this Issue. However, there are way more issues in the builtins code that need to be adressed.

arichardson commented 9 months ago

Thanks for the detailed analysis. It sounds like the right way forward is to drop the XFAIL for now and to fix the librthas* checks. I agree that the tests need improving...

brad0 commented 6 months ago

This can be closed now?

rorth commented 6 months ago

Good question: I just ran a main build and the test PASSes for both sparc and sparcv9:

PASS: Builtins-sparc-sunos :: divtc3_test.c (130 of 87734)
PASS: Builtins-sparcv9-sunos :: divtc3_test.c (433 of 87734)

However, when I run them manually, I get

$ test/builtins/Unit/SPARCSunOSConfig/Output/divtc3_test.c.tmp
skipped
$ test/builtins/Unit/SPARCV9SunOSConfig/Output/divtc3_test.c.tmp
No errors found.

Doing nothing and just printing skipped seems very wrong to me since it hides the actual outcome from the testsuite: the test should probably turn UNSUPPORTED instead. However, this applies not only to this single test but to many (all?) builtin tests.