refactor handling of target features in Session by RalfJung · Pull Request #160530 · rust-lang/rust

RalfJung · 2026-08-04T22:29:55Z

Session currently contains two lists of target features: target_features, which is also exposed in cfg, and unstable_target_features, which is used internally to communicate between various parts of the compiler which target features are actually available, including some that we don't have plans to put in cfg, namely "forbidden" target features. The unstable_target_features list is not equivalent to what nightly code sees in cfg(target_features) as the latter excludes "forbidden" target features. Both lists are computed by fn cfg_target_features even though one of them is never used for cfg. It's all kind of messy.

This PR refactors that: fn cfg_target_features is replaced by fn internal_target_features which computes all enabled Rust target features (including "forbidden" ones -- which are really more like "internal-only" ones so the 2nd commit renames them). We then compute cfg(target_features) from that. The session only stores one list, internal_target_features, which corresponds to the previous unstable_target_features.

To simplify computing internal_target_features I also refactored parse_rust_feature_list to better distinguish actual Rust target features from unknown target features that we are just grandfathering in. I also made implied_target_features not rebuild the same hash map over and over again. And I got rid of a bunch of silly temporary vectors and iterations over all Rust target features.

RalfJung · 2026-08-04T22:32:33Z

-            if sess.target_features.contains(&sym::zca) {
+            if sess.internal_target_features.contains(&sym::zca) {
                e_flags |= elf::EF_RISCV_RVC;
            }

            // Check if RVTSO is enabled
-            if sess.target_features.contains(&sym::ztso) {
+            if sess.internal_target_features.contains(&sym::ztso) {
                e_flags |= elf::EF_RISCV_TSO;
            }


This used to check whether the feature is in cfg(target_feature) rather than checking whether the feature is enabled. I don't think that makes a ton of sense... it means that with -Ctarget-feature=+zca on stable, we do enable that target feature (with a warning), but we don't set the flags here.

View changes since the review

Actually these features are stable so this diff is a non-functional change.

rustbot · 2026-08-04T23:00:35Z

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

Some changes occurred in cfg and check-cfg configuration

cc @Urgau

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

These commits modify compiler targets.
(See the Target Tier Policy.)

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

rustbot · 2026-08-04T23:00:37Z

r? @nnethercote

rustbot has assigned @nnethercote.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: codegen, compiler
codegen, compiler expanded to 75 candidates
Random selection from 21 candidates

RalfJung · 2026-08-04T23:06:10Z

+    let features_map = sess.target.rust_target_features_map();

-    // Compute which of the known target features are enabled in the 'base' target machine. We only
-    // consider "supported" features; "forbidden" features are not reflected in `cfg` as of now.


The old comment here was outdated, we are not actually excluding "forbidden" features here.

View changes since the review

RalfJung · 2026-08-05T08:51:39Z

        "atomics-32",
-        Stability::Forbidden {
+        // Not implied by anything. (FIXME: is that true?)
+        Stability::InternalOnly {
            reason: "unsound because it changes the ABI of atomic operations",


@taiki-e is it true that nothing (no CPU model and no other target feature) implies "atomics-32" on ARM?

View changes since the review

Yes, this is a feature regarding ABI, not ISA. (It's the same as riscv's forced-atomics feature.)

RalfJung · 2026-08-05T08:54:12Z

        "forced-atomics",
-        Stability::Forbidden {
+        // Not implied by any CPU model or other feature. (FIXME: is that true?)
+        Stability::InternalOnly {
            reason: "unsound because it changes the ABI of atomic operations",


@kito-cheng @michaelmaitland @robin-randhawa-sifive @topperc is it true that the "forced-atomics" riscv target feature is not implied by any other target feature or -Ctarget-cpu?

View changes since the review

Yes, it indeed will cause ABI incompatible and no implied by any target-cpu for RISC-V

More comments in LLVM source tree: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVFeatures.td#L1986-L1993

RalfJung · 2026-08-05T08:55:51Z

        "windowed",
-        Forbidden { reason: "windowed changes the Xtensa calling convention", hard_error: false },
+        // Not implied by any CPU model or other feature. (FIXME: is that true?)
+        InternalOnly {
+            reason: "windowed changes the Xtensa calling convention",
+            hard_error: false,


@MabezDev is it true that the "windows" xtensa target feature is not implied by any other target feature or -Ctarget-cpu?

View changes since the review

Yes, it's true. It's a standalone dependency free option. Only other ISA features depend on it.

We have two examples of such chips, esp8266 without windowed and esp32 family have windowed enabled, if you want to look further.

It seems to imply "exception" so apparently there is a dependency?

Any dependency in either direction is a problem. The way things stand now, we have to make "exception" also an internal-only target feature, since it affects the ABI via its connection with "windowed".

Okay, I think I understand a bit better what the context of this is. So the window feature is dependency-free no other ISA feature forces it to be enabled by implication.

What I didn't understand on the first run is the CPU side of it. The CPU will enable the windowed feature for certain chips (anything that's not esp8266 basically, which is every modern esp32).

Therefore the comment is incorrect, and needs to be changed to something along the lines of

// Not implied by any other ISA feature, but can be implied by the value of CPU.

So if I mix code with different -Ctarget-cpu values I can get UB because the ABI is different?
Uh, that's not okay. That's a soundness bug.

That's exactly why the esp8266 is retired, and why there are no upstream targets for it. All current targets are windowed, and through the diff highlighted here, it's not possible to set windowed via -Ctarget-feature.

So essentially, yes there could be possible UB, but the current rust targets are all windowed. It would require doing something wacky, like setting cpu to esp8266 on for example, the esp32s3 rust target. If you think we need a more concrete fix, I'm all ears.

I can also ask our LLVM maintainers to drop the esp8266 cpu completely, and therefore making this path uncrossable (unless someone else ads an non windowed target in LLVM).

Oh so Rust should only support the case where "windowed" is enabled? That's easy, that just needs to be configured in abi_required_features. :)

Since "windowed" implies "exception", that means "exception" is also always enabled?

RalfJung · 2026-08-05T08:56:47Z

+            reason: "windowed changes the Xtensa calling convention",
+            hard_error: false,
+        },
        &["exception"],


This implication means that -Ctarget-feature=-exception will turn off "windowed". How does it make sense to reject -Ctarget-feature=-windows but allow -Ctarget-feature=-exception?

View changes since the review

bjorn3 · 2026-08-05T09:37:11Z

Does this still ensure for example -Ctarget-cpu=x86-64-v3 enables the cfg's for avx2 and other target features supported by this target cpu?

Details

rustc -Ctarget-cpu=x86-64-v3 --print cfg
debug_assertions
panic="unwind"
target_abi=""
target_arch="x86_64"
target_endian="little"
target_env="gnu"
target_family="unix"
target_feature="avx"
target_feature="avx2"
target_feature="bmi1"
target_feature="bmi2"
target_feature="cmpxchg16b"
target_feature="f16c"
target_feature="fma"
target_feature="fxsr"
target_feature="lzcnt"
target_feature="movbe"
target_feature="popcnt"
target_feature="sse"
target_feature="sse2"
target_feature="sse3"
target_feature="sse4.1"
target_feature="sse4.2"
target_feature="ssse3"
target_feature="xsave"
target_has_atomic="16"
target_has_atomic="32"
target_has_atomic="64"
target_has_atomic="8"
target_has_atomic="ptr"
target_os="linux"
target_pointer_width="64"
target_vendor="unknown"
unix

nnethercote · 2026-08-06T11:59:36Z

It's probably worth a grep to check that all cfg_target_feature references are removed from comments. r=me once that is done and the above comment about InternalOnlyCTargetFeature is considered.

@rustbot author

RalfJung · 2026-08-06T12:46:56Z

It's probably worth a grep to check that all cfg_target_feature references are removed from comments.

Good point, done. I also found a ForbiddenTargetFeatureAttr and renamed that as well.

@bors r=nnethercote

rust-bors · 2026-08-06T12:46:58Z

📌 Commit 561be60 has been approved by nnethercote

It is now in the queue for this repository.

rust-bors · 2026-08-06T14:50:08Z

⚠️ A new commit e5004d09b0b8ba484ac59586b3124d44a52cf509 was pushed.

This pull request was unapproved.

RalfJung · 2026-08-06T14:50:19Z

I removed the xtensa ABI checks here as those will be handled by #160643.

RalfJung · 2026-08-06T14:50:28Z

@bors r=nnethercote

rust-bors · 2026-08-06T14:50:30Z

📌 Commit e5004d0 has been approved by nnethercote

It is now in the queue for this repository.

This comment was marked as resolved.

Sign in to view

RalfJung force-pushed the internal-target-features branch from 02d834e to db9655e Compare August 4, 2026 22:53

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Aug 4, 2026

RalfJung force-pushed the internal-target-features branch 2 times, most recently from 311dc25 to 36489cd Compare August 4, 2026 22:57

RalfJung commented Aug 4, 2026

View reviewed changes

RalfJung marked this pull request as ready for review August 4, 2026 23:00

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

rustbot assigned nnethercote Aug 4, 2026

This comment has been minimized.

Sign in to view

RalfJung force-pushed the internal-target-features branch from 36489cd to 1555262 Compare August 4, 2026 23:04

RalfJung mentioned this pull request Aug 4, 2026

do not map llvm_features_by_flags back into Rust-level cfg(target_feature) #160465

Closed

RalfJung commented Aug 4, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

RalfJung force-pushed the internal-target-features branch 3 times, most recently from 71ce3cd to 3ccab9c Compare August 5, 2026 06:22

RalfJung commented Aug 5, 2026

View reviewed changes

RalfJung force-pushed the internal-target-features branch from 2cfdc5e to aac414a Compare August 5, 2026 08:52

RalfJung commented Aug 5, 2026

View reviewed changes

RalfJung force-pushed the internal-target-features branch from aac414a to 00dcb30 Compare August 5, 2026 08:58

RalfJung force-pushed the internal-target-features branch 5 times, most recently from 6c2a21f to 1611f60 Compare August 5, 2026 13:33

This was referenced Aug 5, 2026

turn ABI-controlling target features into a target modifier #160593

Open

Missing target feature ABI checks for nvptx #157532

Open

nnethercote approved these changes Aug 6, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_ssa/src/target_features.rs Outdated

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 Aug 6, 2026

RalfJung force-pushed the internal-target-features branch from 1611f60 to ea26f7c Compare August 6, 2026 12:44

refactor handling of target features in Session

8bb9f27

RalfJung force-pushed the internal-target-features branch from ea26f7c to 561be60 Compare August 6, 2026 12:45

rust-bors Bot 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 Aug 6, 2026

MabezDev mentioned this pull request Aug 6, 2026

Require windowed (and exception) for Xtensa ABI #160643

Open

2 tasks

RalfJung added 2 commits August 6, 2026 16:49

rename 'forbidden' target features to 'internal-only'

6253cca

ensure that we never toggle internal target features via the attribute

e5004d0

RalfJung force-pushed the internal-target-features branch from 561be60 to e5004d0 Compare August 6, 2026 14:50

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 6, 2026

rust-bors Bot 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 Aug 6, 2026

Uh oh!

Uh oh!

Conversation

RalfJung commented Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

RalfJung Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Aug 4, 2026

Uh oh!

rustbot commented Aug 4, 2026

Uh oh!

This comment has been minimized.

RalfJung Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

RalfJung Aug 5, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjorn3 commented Aug 5, 2026

Uh oh!

Uh oh!

nnethercote commented Aug 6, 2026

Uh oh!

RalfJung commented Aug 6, 2026

Uh oh!

rust-bors Bot commented Aug 6, 2026

Uh oh!

rust-bors Bot commented Aug 6, 2026

Uh oh!

RalfJung commented Aug 6, 2026

Uh oh!

RalfJung commented Aug 6, 2026

Uh oh!

rust-bors Bot commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

RalfJung commented Aug 4, 2026 •

edited by rustbot

Loading

RalfJung Aug 4, 2026 •

edited by rustbot

Loading

RalfJung Aug 4, 2026 •

edited by rustbot

Loading

RalfJung Aug 5, 2026 •

edited

Loading

RalfJung Aug 5, 2026 •

edited by rustbot

Loading

RalfJung Aug 5, 2026 •

edited by rustbot

Loading

RalfJung Aug 5, 2026 •

edited by rustbot

Loading