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

tests/ui/abi/sparcv8plus.rs#sparc_cpu_v9 broken in LLVM 20 #132957

Open
durin42 opened this issue Nov 12, 2024 · 6 comments
Open

tests/ui/abi/sparcv8plus.rs#sparc_cpu_v9 broken in LLVM 20 #132957

durin42 opened this issue Nov 12, 2024 · 6 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-SPARC Target: SPARC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@durin42
Copy link
Contributor

durin42 commented Nov 12, 2024

It looks like this is basically expected: the test has some FIXME comments referencing LLVM 20 around the failure. I tried to write a fix that would let the tests work on both LLVM 19 and 20, and I just can't thread the needle correctly.

I think we'd probably have to have an llvm-19 flavor of the test file and an llvm-20 one, because the revisions feature is already being used to specify various flavors of SPARC here.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 12, 2024
@durin42
Copy link
Contributor Author

durin42 commented Nov 12, 2024

In case anyone wants to dig, the test was added in #132552.

@taiki-e
Copy link
Member

taiki-e commented Nov 12, 2024

It looks like this is basically expected

Yes, failure in LLVM 20 is expected. (I'm the author of that PR)

I think we need to split the revisions failing in LLVM 20 into llvm20 and pre-llvm20 like in this test:

//@ revisions: llvm-pre-20 llvm-20
//@ [llvm-20] min-llvm-version: 20
//@ [llvm-pre-20] ignore-llvm-version: 20 - 99

@durin42
Copy link
Contributor Author

durin42 commented Nov 12, 2024

Yeah, except I don't think we can have multiple revisions: stanzas in the test. If you like, I'm happy to figure out a plan if you can show me what the test should look like for LLVM 20. I got confused by the error-output checking logic and couldn't make effective progress. :(

@jieyouxu jieyouxu added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-SPARC Target: SPARC processors C-bug Category: This is a bug. labels Nov 13, 2024
@jieyouxu
Copy link
Member

I think we need to split the revisions failing in LLVM 20 into llvm20 and pre-llvm20 like in this test:

Something like

//@ revisions: ... sparc_cpu_v9_pre_llvm20
//@[sparc_cpu_v9_pre_llvm20] compile-flags: --target sparc-unknown-none-elf -C target-cpu=v9
//@[sparc_cpu_v9_pre_llvm20] needs-llvm-components: sparc
//@[sparc_cpu_v9_pre_llvm20] max-llvm-major-version: 19

after #132310 lands.

@jieyouxu jieyouxu added A-ABI Area: Concerning the application binary interface (ABI) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 13, 2024
@taiki-e
Copy link
Member

taiki-e commented Nov 13, 2024

I haven't tested it, but the following changes are probably needed

-//@ revisions: sparc sparcv8plus sparc_cpu_v9 sparc_feature_v8plus sparc_cpu_v9_feature_v8plus
+//@ revisions: sparc sparcv8plus sparc_cpu_v9 sparc_cpu_v9_pre_llvm20 sparc_feature_v8plus sparc_cpu_v9_feature_v8plus
 //@[sparc] compile-flags: --target sparc-unknown-none-elf
 //@[sparc] needs-llvm-components: sparc
 //@[sparcv8plus] compile-flags: --target sparc-unknown-linux-gnu
 //@[sparcv8plus] needs-llvm-components: sparc
+//@[sparcv8plus] min-llvm-version: 19
 //@[sparc_cpu_v9] compile-flags: --target sparc-unknown-none-elf -C target-cpu=v9
 //@[sparc_cpu_v9] needs-llvm-components: sparc
+//@[sparc_cpu_v9] min-llvm-version: 20
+//@[sparc_cpu_v9_pre_llvm20] compile-flags: --target sparc-unknown-none-elf -C target-cpu=v9
+//@[sparc_cpu_v9_pre_llvm20] needs-llvm-components: sparc
+//@[sparc_cpu_v9_pre_llvm20] min-llvm-version: 19
+//@[sparc_cpu_v9_pre_llvm20] max-llvm-version: 19
 //@[sparc_feature_v8plus] compile-flags: --target sparc-unknown-none-elf -C target-feature=+v8plus
 //@[sparc_feature_v8plus] needs-llvm-components: sparc
 //@[sparc_cpu_v9_feature_v8plus] compile-flags: --target sparc-unknown-none-elf -C target-cpu=v9 -C target-feature=+v8plus
 //@[sparc_cpu_v9_feature_v8plus] needs-llvm-components: sparc
-//@ max-llvm-version: 19
-// FIXME: sparc_cpu_v9 should be in "-v8plus,+v9" group (fixed in LLVM 20)
 #[cfg(all(target_feature = "v8plus", target_feature = "v9"))]
 compile_error!("+v8plus,+v9");
-//[sparcv8plus,sparc_cpu_v9_feature_v8plus,sparc_cpu_v9]~^ ERROR +v8plus,+v9
+//[sparcv8plus,sparc_cpu_v9_feature_v8plus,sparc_cpu_v9_pre_llvm20]~^ ERROR +v8plus,+v9

 // FIXME: should be rejected
 #[cfg(all(target_feature = "v8plus", not(target_feature = "v9")))]
 compile_error!("+v8plus,-v9 (FIXME)");
 //[sparc_feature_v8plus]~^ ERROR +v8plus,-v9 (FIXME)

 #[cfg(all(not(target_feature = "v8plus"), target_feature = "v9"))]
 compile_error!("-v8plus,+v9");
+//[sparc_cpu_v9]~^ ERROR -v8plus,+v9

@jieyouxu
Copy link
Member

+//@[sparc_cpu_v9_pre_llvm20] min-llvm-version: 19
+//@[sparc_cpu_v9_pre_llvm20] max-llvm-version: 19

After #132995 lands we will be able to write //@ exact-llvm-major-version: 19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-SPARC Target: SPARC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants