fuchsia: dynamically enable fix-cortex-a53-835769 for generic CPUs by PiJoules · Pull Request #162566 · rust-lang/rust

PiJoules · 2026-09-09T23:13:44Z

Previously, +fix-cortex-a53-835769 was statically included in base.features for aarch64-unknown-fuchsia. This caused the Cortex-A53 erratum workaround to be unconditionally enabled regardless of the selected target CPU (e.g., when passing -C target-cpu=cortex-a73).

The behavior we want is to have it enabled by default only for the generic CPU (armv8-a), and it should be togglable via the normal -Ctarget-features= flag. This matches the target-feature logic in clang.

AI: Note gemini was used to help verify this does match Clang's behavior and write out the matrix of different test invocations. I reviewed this code to the best of my ability before submitting.

Previously, `+fix-cortex-a53-835769` was statically included in `base.features` for `aarch64-unknown-fuchsia`. This caused the Cortex-A53 erratum workaround to be unconditionally enabled regardless of the selected target CPU (e.g., when passing `-C target-cpu=cortex-a73`). The behavior we want is to have it enabled by default only for the generic CPU (armv8-a), and it should be togglable via the normal `-Ctarget-features=` flag. This matches the target-feature logic in clang. AI: Note gemini was used to help verify this does match Clang's behavior and write out the matrix of different test invocations. I reviewed this code to the best of my ability before submitting.

rustbot · 2026-09-09T23:13:47Z

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

rustbot · 2026-09-09T23:13:50Z

r? @petrochenkov

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

Why was this reviewer chosen?

The reviewer was selected based on:

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

PiJoules · 2026-09-09T23:14:05Z

cc @erickt @ilovepi

petrochenkov · 2026-09-10T13:00:10Z

r? @RalfJung afaik you've been reworking the target feature system, not sure how this fits into that

rustbot · 2026-09-10T13:00:15Z

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

RalfJung · 2026-09-10T14:11:16Z

+            "generic" | "cortex-a53"
+        )
+    {
+        extend_backend_features("fix-cortex-a53-835769", true);


This hack (wtf is even going on there) looks LLVM-specific but the file you put it in is also used for other backends.

View changes since the review

Above this is what looks like another instance of something target/LLVM-specific that was added here so I assumed the existing precedent was that it was ok to include such things here.

RalfJung · 2026-09-10T14:13:12Z

and it should be togglable via the normal -Ctarget-features= flag.

For that you have to make it a Rust target feature, i.e., add it in compiler/rustc_target/src/target_features.rs. This PR doesn't do that. It would be by far the silliest target feature we have I think.^^

I can give general advice for target feature but cannot review whatever insanity this weird hack is. I just wonder if this is really the best way to deal with whatever problem this solves.

@rustbot reroll

RalfJung · 2026-09-10T14:17:01Z

Previously, +fix-cortex-a53-835769 was statically included in base.features for aarch64-unknown-fuchsia. This caused the Cortex-A53 erratum workaround to be unconditionally enabled regardless of the selected target CPU (e.g., when passing -C target-cpu=cortex-a73).

Yes, features is for things that must be configured like that on this target.
There was recently another PR where people also lamented that it's unfortunate that target spec features cannot be overwritten with -Ctarget-feature. However, it is not a solution to instead add target-specific hacks in target-independent code. So I don't think we should land this. If we need a concept of base target features that can be overwritten by -Ctarget-feature, then (a) we should figure out why features in the target spec doesn't behave like that, and (b) if features can't be changed, add a new target spec field so we can properly represent this in a generic way.

RalfJung · 2026-09-10T14:23:07Z

Actually, I mixed things up. features in the target spec can be overwritten by -Ctarget-feature. But they cannot be overwritten by -Ctarget-cpu. It is unclear what that would even mean.

RalfJung · 2026-09-10T14:29:24Z

TBH IMO this would be best handled by LLVM. LLVM can realize that the selected target CPU does not need whatever that "fix" thing is any more, and then stop applying it. Asking frontends to do this means that it requires hacks in a whole bunch of places as it does not follow the usual structure of a target feature.

Cc @nikic

JonathanBrouwer · 2026-09-11T09:21:08Z

Also not at all familiar with this code
@rustbot reroll

adwinwhite · 2026-09-15T02:23:12Z

@rustbot reroll

chenyukang · 2026-09-15T02:24:47Z

@rustbot reroll

rust-bors · 2026-09-16T14:25:59Z

☔ The latest upstream changes (presumably #161432) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

PiJoules · 2026-09-22T18:54:14Z

Actually, I mixed things up. features in the target spec can be overwritten by -Ctarget-feature. But they cannot be overwritten by -Ctarget-cpu. It is unclear what that would even mean.

This particular target feature is only relevant to this CPU since later arm CPUs have a hardware fix for it so it's worthwhile conditioning the target feature on the CPU.

TBH IMO this would be best handled by LLVM. LLVM can realize that the selected target CPU does not need whatever that "fix" thing is any more, and then stop applying it. Asking frontends to do this means that it requires hacks in a whole bunch of places as it does not follow the usual structure of a target feature.

There is an ongoing effort to move all the target-specific stuff handled by drivers into the TargetParser but that migration isn't complete and there's still a bunch of stuff missing before it could be used by other frontends. After that, we'd also need to teach rust to use it which might take some time.

RalfJung · 2026-09-22T19:51:20Z

This particular target feature is only relevant to this CPU since later arm CPUs have a hardware fix for it so it's worthwhile conditioning the target feature on the CPU.

I understand. So why does LLVM not do that? It's natural do to it there. It's a pretty bad hack and a maintenance hassle to do it in rustc (and every other frontend).

The target feature we are asking for is "please make sure this hardware bug doesn't affect the code". Newer hardware implements that feature for free so it should be a NOP if LLVM knows that the hardware is new enough. I think this doesn't need any moving around of anything in LLVM, just a slightly smarter logic for what that target feature actually does: only apply the fix if the feature is enabled and the selected CPU actually needs the fix.

davidtwco · 2026-09-23T16:05:29Z

The target feature we are asking for is "please make sure this hardware bug doesn't affect the code". Newer hardware implements that feature for free so it should be a NOP if LLVM knows that the hardware is new enough. I think this doesn't need any moving around of anything in LLVM, just a slightly smarter logic for what that target feature actually does: only apply the fix if the feature is enabled and the selected CPU actually needs the fix.

This strikes me as the correct approach here - is there a reason that LLVM couldn't do this?

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 Sep 9, 2026

rustbot assigned petrochenkov Sep 9, 2026

rustbot assigned RalfJung and unassigned petrochenkov Sep 10, 2026

RalfJung reviewed Sep 10, 2026

View reviewed changes

rustbot assigned JonathanBrouwer and unassigned RalfJung Sep 10, 2026

rustbot assigned adwinwhite and unassigned JonathanBrouwer Sep 11, 2026

rustbot assigned chenyukang and unassigned adwinwhite Sep 15, 2026

rustbot assigned davidtwco and unassigned chenyukang Sep 15, 2026

Uh oh!

Conversation

PiJoules commented Sep 9, 2026

Uh oh!

rustbot commented Sep 9, 2026

Uh oh!

rustbot commented Sep 9, 2026

Uh oh!

PiJoules commented Sep 9, 2026

Uh oh!

petrochenkov commented Sep 10, 2026

Uh oh!

rustbot commented Sep 10, 2026

Uh oh!

RalfJung Sep 10, 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!

PiJoules Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Sep 10, 2026

Uh oh!

RalfJung commented Sep 10, 2026

Uh oh!

JonathanBrouwer commented Sep 11, 2026

Uh oh!

adwinwhite commented Sep 15, 2026

Uh oh!

chenyukang commented Sep 15, 2026

Uh oh!

rust-bors Bot commented Sep 16, 2026

Uh oh!

PiJoules commented Sep 22, 2026

Uh oh!

RalfJung commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidtwco commented Sep 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

RalfJung Sep 10, 2026 •

edited by rustbot

Loading

RalfJung commented Sep 10, 2026 •

edited

Loading

RalfJung commented Sep 10, 2026 •

edited

Loading

RalfJung commented Sep 22, 2026 •

edited

Loading