Skip to content
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-sparc-sunos:: divtc3_test.c XPASSes on Solaris/sparc #72398

Open
rorth opened this issue Nov 15, 2023 · 3 comments
Open

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

rorth opened this issue Nov 15, 2023 · 3 comments

Comments

@rorth
Copy link
Collaborator

rorth commented Nov 15, 2023

As originally reported in [builtins] Guard the divtc3_test.c test with CRT_HAS_TF_MODE, that patch 77d75dc 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:

  • In LLVM 17, the 32-bit test XFAILs: it does compile, but died with SIGBUS at runtime, as originally reported in Bug 42493/Issue Passing long double args on 32-bit SPARC violates ABI #41838. The 32-bit divtc3.c.o contains a definition of __divtc3
  • On main, the test now XPASSes. Before the culprit patch, the test would fail to compile (missing definitions of Qcomplex and tf_float), which was hidden by the XFAIL. After the culprit patch, compilation wasn't attempted, resulting in a XPASS. Unlike LLVM 17, divtc3.c.o now contains no symbols (i.e. no definition of __divtc3).

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
Copy link
Member

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

rorth added a commit to rorth/llvm-project that referenced this issue Nov 15, 2023
…r now

As detailed in Issue llvm#72398, the recent builtins rework and
77d75dc caused `Builtins-sparc-sunos:: divtc3_test.c`
to `XPASS` on 32-bit Solaris/SPARC.  Since there are several underlying issues,
un-XFAIL the test for now until those are resolved, to turn the Solaris/sparcv9
buildbot green again after 5 days.
rorth added a commit that referenced this issue Nov 15, 2023
#72403)

As detailed in Issue #72398, the recent builtins rework and
77d75dc caused `Builtins-sparc-sunos::
divtc3_test.c` to `XPASS` on 32-bit Solaris/SPARC. Since there are
several underlying issues, un-`XFAIL` the test for now until those are
resolved, to turn the Solaris/sparcv9 buildbot green again after 5 days.

Tested on `sparcv9-sun-solaris2.11` and `x86_64-pc-linux-gnu`.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this issue Nov 20, 2023
llvm#72403)

As detailed in Issue llvm#72398, the recent builtins rework and
77d75dc caused `Builtins-sparc-sunos::
divtc3_test.c` to `XPASS` on 32-bit Solaris/SPARC. Since there are
several underlying issues, un-`XFAIL` the test for now until those are
resolved, to turn the Solaris/sparcv9 buildbot green again after 5 days.

Tested on `sparcv9-sun-solaris2.11` and `x86_64-pc-linux-gnu`.
@brad0
Copy link
Contributor

brad0 commented Jan 26, 2024

This can be closed now?

@rorth
Copy link
Collaborator Author

rorth commented Jan 26, 2024

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.

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

No branches or pull requests

4 participants