organize and extend forbidden target feature tests #140395
Open
+131
−78
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
r=me with tidy fixed. |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
RalfJung
force-pushed
the
target-feature-tests
branch
from
1b9008f to
240cc10
Compare
This comment has been minimized.
This comment has been minimized.
RalfJung
commented
Apr 28, 2025
| | | ||
| = 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 |
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
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.
|
So somehow on CI we get a bunch of extra warnings: I don't get those when running the test locally. Is that an LLVM version thing? Does |
Thank you for everything you've done. IIRC, when the LLVM backend doesn't support target features listed in Would it be possible to add a 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-passIt would also be useful for the warning: |
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.
I don't think so, that warning appears consistently for all LLVM versions it seems (so it's not a problem). |
This comment has been minimized.
This comment has been minimized.
|
Oh I see what you mean, that RISCV error didn't yet show in LLVM 19. |
RalfJung
force-pushed
the
target-feature-tests
branch
from
f78cc1b to
5b7e669
Compare
|
@bors r=petrochenkov |
bors
added
S-waiting-on-bors
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Apr 29, 2025
…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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 29, 2025
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
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Apr 30, 2025
…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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 30, 2025
Rollup of 14 pull requests Successful merges: - rust-lang#140380 (transmutability: uninit transition matches unit byte only) - rust-lang#140385 (Subtree update of `rust-analyzer`) - rust-lang#140395 (organize and extend forbidden target feature tests) - rust-lang#140430 (Improve test coverage of HIR pretty printing.) - rust-lang#140458 (Fix for async drop ice with partly dropped tuple) - rust-lang#140460 (Fix handling of LoongArch target features not supported by LLVM 19) - rust-lang#140465 (chore: edit and move tests) - rust-lang#140467 (Don't FCW assoc consts in patterns) - rust-lang#140468 (Minor tweaks to make some normalization (adjacent) code less confusing) - rust-lang#140470 (CI: rfl: move job forward to Linux v6.15-rc4) - rust-lang#140476 (chore: delete unused ui/auxiliary crates) - rust-lang#140481 (Require sanitizers be enabled for asan_odr_windows.rs) - rust-lang#140486 (rustfmt: Also allow bool literals as first item of let chain) - rust-lang#140494 (Parser: Document restrictions) r? `@ghost` `@rustbot` modify labels: rollup
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Apr 30, 2025
…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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 30, 2025
Rollup of 13 pull requests Successful merges: - rust-lang#140380 (transmutability: uninit transition matches unit byte only) - rust-lang#140385 (Subtree update of `rust-analyzer`) - rust-lang#140395 (organize and extend forbidden target feature tests) - rust-lang#140458 (Fix for async drop ice with partly dropped tuple) - rust-lang#140460 (Fix handling of LoongArch target features not supported by LLVM 19) - rust-lang#140465 (chore: edit and move tests) - rust-lang#140467 (Don't FCW assoc consts in patterns) - rust-lang#140468 (Minor tweaks to make some normalization (adjacent) code less confusing) - rust-lang#140470 (CI: rfl: move job forward to Linux v6.15-rc4) - rust-lang#140476 (chore: delete unused ui/auxiliary crates) - rust-lang#140481 (Require sanitizers be enabled for asan_odr_windows.rs) - rust-lang#140486 (rustfmt: Also allow bool literals as first item of let chain) - rust-lang#140494 (Parser: Document restrictions) r? `@ghost` `@rustbot` modify labels: rollup
|
@bors r- |
bors
added
S-waiting-on-author
RalfJung
force-pushed
the
target-feature-tests
branch
from
5b7e669 to
e90dafa
Compare
|
Yeah looks like it. We should let #140460 land first. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 30, 2025
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. try-job: `x86_64-gnu-llvm-*`
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
try-job:
x86_64-gnu-llvm-20-*