organize and extend forbidden target feature tests #140395

RalfJung · 2025-04-28T13:42:18Z

In particular this adds some loongarch tests for #135015, Cc @heiher

Seems like the tests change so much git does not detect the renames; a commit-by-commit review should help.

…e one with revisions

rustbot · 2025-04-28T13:42:22Z

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

petrochenkov · 2025-04-28T14:07:19Z

r=me with tidy fixed.
@rustbot author

rustbot · 2025-04-28T14:07:24Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

RalfJung · 2025-04-28T15:14:47Z

tests/ui/target-feature/abi-required-target-feature-flag-disable.loongarch.stderr

+   |
+   = note: this feature is not stably supported; its behavior can change in the future
+
+warning: both target-abi and the triple-implied ABI are invalid, ignoring and using feature-implied ABI


@heiher any idea what this warning is about? Does it come from LLVM?

For riscv it says

Hard-float 'd' ABI can't be used for a target that doesn't support the D instruction set extension (ignoring target-abi)

which is a lot more clear. Is this trying to say the same thing?

Yes, they are quite similar. The LoongArch backend attempts to compute the ABI based on the target-abi and triple-name. If there’s a conflict between the resulting ABI and the target features, the final ABI will be determined by the target features.

Fore more details:

https://github.com/rust-lang/llvm-project/blob/rustc-1.86.0/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp#L171-L172

Okay. I find the LoongArch error quite unclear compared to the RISC-V error... but anyway it should be impossible to trigger this warning from rustc without also triggering "target feature d must be enabled to ensure that the ABI of the current target can be implemented correctly" so users will hopefully understand what the problem is.

RalfJung · 2025-04-28T15:17:00Z

So somehow on CI we get a bunch of extra warnings:

2025-04-28T14:35:22.9208959Z '+div32' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9209370Z '+div32' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9209778Z '+lam-bh' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9210178Z '+lam-bh' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9210688Z '+lamcas' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211099Z '+lamcas' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211515Z '+ld-seq-sa' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211932Z '+ld-seq-sa' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9212335Z '+scq' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9212721Z '+scq' is not a recognized feature for this target (ignoring feature)

I don't get those when running the test locally. Is that an LLVM version thing? Does loongarch64-unknown-none not work on older LLVM versions (not without a bunch of warnings, anyway)? How would one write a test for that target then...? And what even enables those target features?!?

heiher · 2025-04-29T01:31:42Z

So somehow on CI we get a bunch of extra warnings:

2025-04-28T14:35:22.9208959Z '+div32' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9209370Z '+div32' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9209778Z '+lam-bh' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9210178Z '+lam-bh' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9210688Z '+lamcas' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211099Z '+lamcas' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211515Z '+ld-seq-sa' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9211932Z '+ld-seq-sa' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9212335Z '+scq' is not a recognized feature for this target (ignoring feature)
2025-04-28T14:35:22.9212721Z '+scq' is not a recognized feature for this target (ignoring feature)

I don't get those when running the test locally. Is that an LLVM version thing? Does loongarch64-unknown-none not work on older LLVM versions (not without a bunch of warnings, anyway)? How would one write a test for that target then...? And what even enables those target features?!?

Thank you for everything you've done.

IIRC, when the LLVM backend doesn't support target features listed in compiler/rustc_target/src/target_features.rs, the rustc throws these kinds of warnings. For LoongArch, features like div32 were introduced starting from LLVM 20, so they are not supported in LLVM 19, which is expected behavior.

Would it be possible to add a min-llvm-version to the test cases? This would help to handle the differences between LLVM version.

diff --git a/tests/ui/target-feature/abi-required-target-feature-flag-disable.rs b/tests/ui/target-feature/abi-required-target-feature-flag-disable.rs
index 02841b6690a..5b0d7b7b252 100644
--- a/tests/ui/target-feature/abi-required-target-feature-flag-disable.rs
+++ b/tests/ui/target-feature/abi-required-target-feature-flag-disable.rs
@@ -12,6 +12,7 @@
 //@[riscv] needs-llvm-components: riscv
 //@[loongarch] compile-flags: --target=loongarch64-unknown-none -Ctarget-feature=-d
 //@[loongarch] needs-llvm-components: loongarch
+//@[loongarch] min-llvm-version: 20
 // For now this is just a warning.
 //@ build-pass

It would also be useful for the warning: Hard-float 'd' ABI can't be used ... for RISC-V.

RalfJung · 2025-04-29T06:49:49Z

IIRC, when the LLVM backend doesn't support target features listed in compiler/rustc_target/src/target_features.rs, the rustc throws these kinds of warnings.

I'm surprised by this. I thought the warning should should only show up when the target feature is actually used, but I did a quick test and you are right. I hope these warnings do not show up on stable?

We may have to come up with a better way to deal with target features that are not supported by all of the LLVM versions we support, the current approach feels fragile.

It would also be useful for the warning: Hard-float 'd' ABI can't be used ... for RISC-V.

I don't think so, that warning appears consistently for all LLVM versions it seems (so it's not a problem).

RalfJung · 2025-04-29T08:20:01Z

Oh I see what you mean, that RISCV error didn't yet show in LLVM 19.

RalfJung · 2025-04-29T09:39:21Z

@bors r=petrochenkov

bors · 2025-04-29T09:39:24Z

📌 Commit 5b7e669 has been approved by petrochenkov

It is now in the queue for this repository.

…etrochenkov organize and extend forbidden target feature tests In particular this adds some loongarch tests for rust-lang#135015, Cc `@heiher` Seems like the tests change so much git does not detect the renames; a commit-by-commit review should help.

Rollup of 6 pull requests Successful merges: - rust-lang#139883 (crashes: more tests) - rust-lang#140312 (Improve pretty-printing of braces) - rust-lang#140392 (compiletest: Remove the libtest-based executor and its dependency) - rust-lang#140395 (organize and extend forbidden target feature tests) - rust-lang#140422 (unwind: bump `unwinding` dependency to 0.2.6) - rust-lang#140432 (Update documentation for `fn target_config`) r? `@ghost` `@rustbot` modify labels: rollup

RalfJung added 2 commits April 28, 2025 15:17

forbidden target feature tests: consolidate multiple tests in a singl…

21b2dc1

…e one with revisions

give the abi-relevant target feature tests better names

c73687f

rustbot assigned petrochenkov Apr 28, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 28, 2025

RalfJung mentioned this pull request Apr 28, 2025

Partially stabilize LoongArch target features #135015

Open

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 28, 2025

add more revisions to cover more architectures

240cc10

RalfJung force-pushed the target-feature-tests branch from 1b9008f to 240cc10 Compare April 28, 2025 14:09

This comment has been minimized.

Sign in to view

RalfJung commented Apr 28, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

loongarch tests need LLVM version 20 to avoid warnings

5b7e669

RalfJung force-pushed the target-feature-tests branch from f78cc1b to 5b7e669 Compare April 29, 2025 08:20

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 29, 2025

Zalathar mentioned this pull request Apr 29, 2025

Rollup of 6 pull requests #140444

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

organize and extend forbidden target feature tests #140395

organize and extend forbidden target feature tests #140395

RalfJung commented Apr 28, 2025

rustbot commented Apr 28, 2025

This comment has been minimized.

petrochenkov commented Apr 28, 2025

rustbot commented Apr 28, 2025

This comment has been minimized.

RalfJung Apr 28, 2025 •

edited

Loading

heiher Apr 29, 2025

RalfJung Apr 29, 2025 •

edited

Loading

RalfJung commented Apr 28, 2025 •

edited

Loading

heiher commented Apr 29, 2025

RalfJung commented Apr 29, 2025

This comment has been minimized.

RalfJung commented Apr 29, 2025

RalfJung commented Apr 29, 2025

bors commented Apr 29, 2025

organize and extend forbidden target feature tests #140395

Are you sure you want to change the base?

organize and extend forbidden target feature tests #140395

Conversation

RalfJung commented Apr 28, 2025

rustbot commented Apr 28, 2025

This comment has been minimized.

petrochenkov commented Apr 28, 2025

rustbot commented Apr 28, 2025

This comment has been minimized.

RalfJung Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

heiher Apr 29, 2025

Choose a reason for hiding this comment

RalfJung Apr 29, 2025 • edited Loading

Choose a reason for hiding this comment

RalfJung commented Apr 28, 2025 • edited Loading

heiher commented Apr 29, 2025

RalfJung commented Apr 29, 2025

This comment has been minimized.

RalfJung commented Apr 29, 2025

RalfJung commented Apr 29, 2025

bors commented Apr 29, 2025

RalfJung Apr 28, 2025 •

edited

Loading

RalfJung Apr 29, 2025 •

edited

Loading

RalfJung commented Apr 28, 2025 •

edited

Loading