organize and extend forbidden target feature tests by RalfJung · Pull Request #140395 · rust-lang/rust

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.

try-job: x86_64-gnu-llvm-20-*

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

…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 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

…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.

RalfJung · 2025-04-30T21:35:50Z

Hm no that can't be the issue, that only changes things for older LLVM...
@bors try

@nikic @heiher any idea why the LLVM 20 on the CI runner does not produce the warnings that I get when building these tests locally?

bors · 2025-04-30T21:37:03Z

⌛ Trying commit e90dafa with merge 5143e37...

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-*`

bors · 2025-04-30T23:23:59Z

💔 Test failed - checks-actions

RalfJung · 2025-05-01T09:24:20Z

On the LLVM side, these messages seem to be output via errs() << .... Is that output maybe suppressed under some conditions? (That would honestly be kind of bad, at least some of them indicate critical issues.)

@taiki-e @nagisa maybe you have an idea about what LLVM is doing here?

nagisa · 2025-05-02T09:04:27Z

In my experience you can't suppress error output from LLVM, no, but that's just experience from a long time ago. No idea if they maybe changed things up for LLVM 20+

nikic · 2025-05-02T09:20:25Z

I don't know why there would be a behavior difference between Rust's and Ubuntu's LLVM here.

How does rust intercept LLVM's stderr stream to produce these as warnings?

RalfJung · 2025-05-02T09:54:10Z

I don't know why there would be a behavior difference between Rust's and Ubuntu's LLVM here.

So these CI runners use the Ubuntu LLVM? Maybe they applied some patches...?

How does rust intercept LLVM's stderr stream to produce these as warnings?

Does it do that? I thought this is just LLVM printing directly to stderr. The warnings do not show up in rustc's warning count, for instance.

nikic · 2025-05-02T10:19:30Z

How does rust intercept LLVM's stderr stream to produce these as warnings?

Does it do that? I thought this is just LLVM printing directly to stderr. The warnings do not show up in rustc's warning count, for instance.

Oh, I saw the warning: prefixes and thought rustc is adding those. But apparently LLVM itself emits them in that format.

nikic · 2025-05-02T10:32:11Z

My best guess is that with these warnings being emitted in the backend, there might be some kind of threading interaction?

RalfJung · 2025-05-02T11:25:40Z

Well, I think I'll just try to normalize them away.

…e one with revisions

RalfJung · 2025-05-02T12:11:09Z

@bors try

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-20-*`

bors · 2025-05-02T12:12:21Z

⌛ Trying commit 6873a46 with merge 9081c65...

bors · 2025-05-02T14:19:56Z

☀️ Try build successful - checks-actions
Build commit: 9081c65 (9081c650eb0f67a8654540d94b56ad64895c8e3d)

RalfJung · 2025-05-02T14:27:53Z

@bors r=petrochenkov

bors · 2025-05-02T14:27:57Z

📌 Commit 6873a46 has been approved by petrochenkov

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#138712 (resolve: Support imports of associated types and glob imports from traits) - rust-lang#140395 (organize and extend forbidden target feature tests) - rust-lang#140576 (Remove fragile equal-pointers-unequal tests.) - rust-lang#140582 (Update sysinfo to `0.35.0` in bootstrap and `tools/opt-dist`) - rust-lang#140595 (doc(std): fix typo lchown -> lchmod) - rust-lang#140597 (zkvm: remove schmerik as target maintainer) - rust-lang#140604 (yet another small borrowck cleanup ) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#140395 - RalfJung:target-feature-tests, r=petrochenkov 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-20-*`

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

Merged

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

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

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

Zalathar mentioned this pull request Apr 30, 2025

Rollup of 14 pull requests #140501

Closed

This comment has been minimized.

Sign in to view

RalfJung added 3 commits May 2, 2025 13:29

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

2f2d3c8

…e one with revisions

give the abi-relevant target feature tests better names

7a2d51d

add more revisions to cover more architectures

6873a46

RalfJung force-pushed the target-feature-tests branch from e90dafa to 6873a46 Compare May 2, 2025 12:11

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 May 2, 2025

matthiaskrgr mentioned this pull request May 3, 2025

Rollup of 7 pull requests #140613

Merged

bors merged commit d8b0347 into rust-lang:master May 3, 2025
7 checks passed

rustbot added this to the 1.88.0 milestone May 3, 2025

RalfJung deleted the target-feature-tests branch May 4, 2025 14:13

Uh oh!

Conversation

RalfJung commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 28, 2025

Uh oh!

This comment has been minimized.

petrochenkov commented Apr 28, 2025

Uh oh!

rustbot commented Apr 28, 2025

Uh oh!

This comment has been minimized.

RalfJung Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heiher Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

RalfJung Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

heiher commented Apr 29, 2025

Uh oh!

RalfJung commented Apr 29, 2025

Uh oh!

This comment has been minimized.

RalfJung commented Apr 29, 2025

Uh oh!

RalfJung commented Apr 29, 2025

Uh oh!

bors commented Apr 29, 2025

Uh oh!

RalfJung commented Apr 30, 2025

Uh oh!

bors commented Apr 30, 2025

Uh oh!

This comment has been minimized.

bors commented Apr 30, 2025

Uh oh!

RalfJung commented May 1, 2025

Uh oh!

nagisa commented May 2, 2025

Uh oh!

nikic commented May 2, 2025

Uh oh!

RalfJung commented May 2, 2025

Uh oh!

nikic commented May 2, 2025

Uh oh!

nikic commented May 2, 2025

Uh oh!

RalfJung commented May 2, 2025

Uh oh!

RalfJung commented May 2, 2025

Uh oh!

bors commented May 2, 2025

Uh oh!

bors commented May 2, 2025

Uh oh!

RalfJung commented May 2, 2025

Uh oh!

bors commented May 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

RalfJung commented Apr 28, 2025 •

edited

Loading

RalfJung Apr 28, 2025 •

edited

Loading

RalfJung Apr 29, 2025 •

edited

Loading

RalfJung commented Apr 28, 2025 •

edited

Loading