Fix initialization cycle in `target_config` by nnethercote · Pull Request #161903 · rust-lang/rust

nnethercote · 2026-08-28T06:45:28Z

There is an initialization cycle in target_config. Details in the final commit. The previous commits are precursors that support the changes in the final commit.

r? @nikic
cc @cuviper @RalfJung @Zalathar @Patryk27

rustbot · 2026-08-28T06:45:31Z

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

nnethercote · 2026-08-28T06:46:27Z

LLM discosure: an LLM suggested the LLVM-related changes to fix the cycle and did extensive review of my changes. I made all the code and text changes myself.

RalfJung

which calls
target_machine_factory, which uses internal_target_features

Good catch! That was not supposed to happen, that's why it receives the target features explicitly as a slice.

This mostly seems to be about using OwnedMCSubtargetInfo rather than an actual target machine to read the feature list off of LLVM. I don't know these LLVM APIs so I can't comment on how to best do that.

View changes since this review

The `require-explicit-cpu.json` case currently prints a "default target CPU" line; test for this. (It will change in the next commit.)

Specifically, don't print it when `need_explicit_cpu` is set, because it doesn't really make sense in that context. Right now among builtin targets this only affects the `amdgcn-amd-amdhsa` target, but it will also be relevant for the `avr2` target in the next commit. It also affects the `require-explicit-cpu.json` case in `tests/run-make/target-specs/rmake.rs`.

Currently rustc uses LLVM's `TargetMachine::getMCSubtargetInfo` method to access an `MCSubtargetInfo` to do feature testing. The next commit will change the feature testing to instead use an alternative pathway, LLVM's `Target::createMCSubtargetInfo` method. The two pathways have some slight differences. One difference relates to the `avr-none` target. Currently its `cpu` field isn't set so it gets the default "generic" value, which is not a valid AVR CPU name. This was hidden by the fact that the current LLVM pathway goes through the `getCPU` function in `AVRTargetMachine.cpp`, which rewrites "generic" as "avr2". But the alternative LLVM pathway doesn't rewrite "generic". Without an adjustment, we would get some behavioural differences with the alternative pathway, such as "unrecognized processor" errors and empty base feature sets. Therefore, this commit sets `cpu` to "avr2", a more obviously correct choice, and what the current LLVM pathway is effectively doing behind the scenes. You might think this would change the code generated by default, but `avr-none` has `need_explicit_cpu` set to true, so that's not the case, because a missing `-Ctarget-cpu` will trigger a fatal error before codegen. But `cpu` can still reach non-codegen paths (e.g. feature/cfg computation in session setup, and `--print`) so we need a valid backend name. A consequence of this is that `--print target-spec-json` will emit `cpu: "avr2"`. Another consequence is that the `requires_consistent_cpu` check will compare a crate built without `-Ctarget-cpu` (non-codegen only) against "avr2" instead of "generic". The commit also modifies two tests. In both cases, the test passes in this commit with or without the explicit `cpu` field. But in the next commit (using the alternative pathway) both tests would fail without the explicit `cpu` field: - `tests/ui/abi/avr-sram.rs` would fail with ``` 'generic' is not a recognized processor for this target (ignoring processor) 'generic' is not a recognized processor for this target (ignoring processor) warning: target feature `sram` must be enabled to ensure that the ABI of the current target can be implemented correctly ``` - `tests/run-make/print-cfg/rmake.rs` would fail because all features would be missing. Finally, the field docs for `TargetOptions` are tweaked to clarify the interplay between `cpu` and `need_explicit_cpu`.

The `repr(transparent)` isn't necessary: there are no casts or transmutes involving it, and it's not passed by value across an FFI boundary. The `PhantomData` also isn't necessary: the type isn't generic so variance isn't a factor; the `Drop` impl doesn't involve `may_dangle`; and the `NonNull` field means the type is `!Send`/`!Sync` with or without the `PhantomData`.

rustbot · 2026-08-31T02:10:40Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

`llvm::target_config` creates `target_machine` by calling `create_informational_target_machine`, which calls `target_machine_factory`, which uses `internal_target_features`. But this is just before `internal_target_features` is initialized! So we should move `internal_target_features` initialization before `target_machine`, right? But `internal_target_features` initialization involves a closure that inspects `target_machine`. There is a cyclic dependency. There is enough function nesting here that it's hard to spot. In practice this cycle doesn't cause problems because the closure doesn't inspect the parts of `target_machine` that depend on `internal_target_features`. But it demonstrates how startup initialization is all tangled up, and it's blocking some cleanups I am doing in rust-lang#161432 relating to the dangerous uses of `Session` before it's fully initialized. Therefore, this commit changes the first part: instead of creating an `OwnedTargetMachine` we create an `OwnedMCSubtargetInfo`. This is a smaller type that has the feature information we need but doesn't depend on `internal_target_features`. Under the covers we are now using LLVM's `Target::createMCSubtargetInfo` instead of `TargetMachine::getMCSubtargetInfo` so that we avoid having to create a `TargetMachine` at this early stage. This eliminates the cycle. (`TargetMachine` can still be created later on, once we're past this fraught initialization.) There are some slight differences between these two approaches, and the preceding commits fixed up some issues there. Some details about this commit: - The new `OwnedMCSubtargetInfo` is similar to the existing `OwnedTargetMachine`. - `create_informational_target_machine` no longer needs a `for_cfg` parameter, because the one site where `for_cfg` was true has been removed. - `LLVMRustCreateMCSubtargetInfo` mostly replicates part of `LLVMRustCreateTargetMachine` - `LLVMRustMCSubtargetInfoHasFeature` partly replicates `LLVMRustHasFeature`. - `LLVMRustHasFeature` is no longer needed. - The error message for `custom-target-invalid-llvm-target.rs` changed.

rustbot · 2026-08-31T05:09:59Z

⚠️ Warning ⚠️

There are issue links (such as #123) in the commit messages of the following commits.
Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
- 7f0581d

nnethercote · 2026-09-13T22:02:16Z

It has been two weeks. Let's try a different reviewer.

r? @cuviper

nikic · 2026-09-14T07:49:02Z

@bors r+

rust-bors · 2026-09-14T07:49:05Z

📌 Commit 7f0581d has been approved by nikic

It is now in the queue for this repository.

@nikic

…e, r=nikic Fix initialization cycle in `target_config` There is an initialization cycle in `target_config`. Details in the final commit. The previous commits are precursors that support the changes in the final commit. r? @nikic cc @cuviper @RalfJung @Zalathar @Patryk27

@nikic

…e, r=nikic Fix initialization cycle in `target_config` There is an initialization cycle in `target_config`. Details in the final commit. The previous commits are precursors that support the changes in the final commit. r? @nikic cc @cuviper @RalfJung @Zalathar @Patryk27

…uwer Rollup of 11 pull requests Successful merges: - #162752 (`rust-analyzer` subtree update) - #161868 (libtest: never iterate over all tests in `--exact` mode) - #161903 (Fix initialization cycle in `target_config`) - #162240 (Garbage-collect old incremental compilation sessions) - #161675 (document that t-lang does not need involvement for unobservable intrisics) - #162630 (Simplify the `G` in `Diag<'a, G>`) - #162647 (Add regression test for previous overflow evaluating the requirement) - #162703 (regression test for valtree leaf const) - #162723 (Add regression test for unexpected type for constructor) - #162735 (Remove pointless `A: Allocator` bounds in boxed.rs) - #162736 (clean up trivial region constraint filtering)

@nikic

…e, r=nikic Fix initialization cycle in `target_config` There is an initialization cycle in `target_config`. Details in the final commit. The previous commits are precursors that support the changes in the final commit. r? @nikic cc @cuviper @RalfJung @Zalathar @Patryk27

…uwer Rollup of 15 pull requests Successful merges: - #162752 (`rust-analyzer` subtree update) - #161903 (Fix initialization cycle in `target_config`) - #162240 (Garbage-collect old incremental compilation sessions) - #162610 (fix tailcall indirect return) - #162634 (Implement semantic analysis for named `Fn` trait params) - #161675 (document that t-lang does not need involvement for unobservable intrisics) - #162160 (turn aligned-in-packed error into lint) - #162504 (Stabilize `unsafe_cell_access`) - #162516 (tidy: Sort multi-line types by treating `>` as a closing bracket) - #162630 (Simplify the `G` in `Diag<'a, G>`) - #162647 (Add regression test for previous overflow evaluating the requirement) - #162703 (regression test for valtree leaf const) - #162723 (Add regression test for unexpected type for constructor) - #162735 (Remove pointless `A: Allocator` bounds in boxed.rs) - #162736 (clean up trivial region constraint filtering)

…uwer Rollup of 16 pull requests Successful merges: - #162762 (Subtree sync for rustc_codegen_cranelift) - #162752 (`rust-analyzer` subtree update) - #161903 (Fix initialization cycle in `target_config`) - #162240 (Garbage-collect old incremental compilation sessions) - #162610 (fix tailcall indirect return) - #162634 (Implement semantic analysis for named `Fn` trait params) - #161675 (document that t-lang does not need involvement for unobservable intrisics) - #162160 (turn aligned-in-packed error into lint) - #162504 (Stabilize `unsafe_cell_access`) - #162516 (tidy: Sort multi-line types by treating `>` as a closing bracket) - #162630 (Simplify the `G` in `Diag<'a, G>`) - #162647 (Add regression test for previous overflow evaluating the requirement) - #162703 (regression test for valtree leaf const) - #162723 (Add regression test for unexpected type for constructor) - #162735 (Remove pointless `A: Allocator` bounds in boxed.rs) - #162736 (clean up trivial region constraint filtering)

@nikic

Rollup merge of #161903 - nnethercote:fix-target_config-cycle, r=nikic Fix initialization cycle in `target_config` There is an initialization cycle in `target_config`. Details in the final commit. The previous commits are precursors that support the changes in the final commit. r? @nikic cc @cuviper @RalfJung @Zalathar @Patryk27

rust-timer · 2026-09-14T18:47:25Z

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (5fa9d1d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-0.6%	[-0.9%, -0.2%]	7
Improvements ✅ (secondary)	-0.4%	[-0.8%, -0.1%]	27
All ❌✅ (primary)	-0.6%	[-0.9%, -0.2%]	7

Max RSS (memory usage)

Results (secondary -3.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-4.0%, -2.7%]	3
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 406.96 MiB -> 407.01 MiB (0.01%)

nnethercote · 2026-09-14T21:57:37Z

The small perf wins make sense for the short-running benchmarks like helloworld, because the LLVM feature checking is a bit on the slow side, and this PR streamlined that a bit, and there are lots of features on x86-64. The smalls wins on the longer-running libc are curious, though.

…uwer Rollup of 16 pull requests Successful merges: - rust-lang/rust#162762 (Subtree sync for rustc_codegen_cranelift) - rust-lang/rust#162752 (`rust-analyzer` subtree update) - rust-lang/rust#161903 (Fix initialization cycle in `target_config`) - rust-lang/rust#162240 (Garbage-collect old incremental compilation sessions) - rust-lang/rust#162610 (fix tailcall indirect return) - rust-lang/rust#162634 (Implement semantic analysis for named `Fn` trait params) - rust-lang/rust#161675 (document that t-lang does not need involvement for unobservable intrisics) - rust-lang/rust#162160 (turn aligned-in-packed error into lint) - rust-lang/rust#162504 (Stabilize `unsafe_cell_access`) - rust-lang/rust#162516 (tidy: Sort multi-line types by treating `>` as a closing bracket) - rust-lang/rust#162630 (Simplify the `G` in `Diag<'a, G>`) - rust-lang/rust#162647 (Add regression test for previous overflow evaluating the requirement) - rust-lang/rust#162703 (regression test for valtree leaf const) - rust-lang/rust#162723 (Add regression test for unexpected type for constructor) - rust-lang/rust#162735 (Remove pointless `A: Allocator` bounds in boxed.rs) - rust-lang/rust#162736 (clean up trivial region constraint filtering)

rustbot assigned nikic Aug 28, 2026

This comment has been minimized.

Sign in to view

Zalathar reviewed Aug 28, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_llvm/src/back/owned_mc_subtarget_info.rs Outdated

Zalathar reviewed Aug 28, 2026

View reviewed changes

Comment thread compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp Outdated

Zalathar reviewed Aug 28, 2026

View reviewed changes

Comment thread compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp Outdated

RalfJung reviewed Aug 28, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_llvm/src/back/write.rs Outdated

Comment thread compiler/rustc_codegen_llvm/src/llvm_util.rs Outdated

Comment thread compiler/rustc_codegen_llvm/src/llvm_util.rs Outdated

nnethercote added 4 commits August 31, 2026 08:17

Slightly extend tests/run-make/target-specs/rmake.rs

1b08f98

The `require-explicit-cpu.json` case currently prints a "default target CPU" line; test for this. (It will change in the next commit.)

nnethercote force-pushed the fix-target_config-cycle branch from 590ecfb to 0107f29 Compare August 31, 2026 02:10

This comment has been minimized.

Sign in to view

nnethercote mentioned this pull request Aug 31, 2026

Improve Session/CodegenBackend construction #161432

Merged

nnethercote force-pushed the fix-target_config-cycle branch from 0107f29 to 7f0581d Compare August 31, 2026 05:09

rustbot assigned cuviper and unassigned nikic Sep 13, 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 14, 2026

Zalathar mentioned this pull request Sep 14, 2026

Rollup of 7 pull requests #162753

Closed

JonathanBrouwer mentioned this pull request Sep 14, 2026

Rollup of 11 pull requests #162755

Closed

JonathanBrouwer mentioned this pull request Sep 14, 2026

Rollup of 15 pull requests #162764

Closed

JonathanBrouwer mentioned this pull request Sep 14, 2026

Rollup of 16 pull requests #162767

Merged

rust-bors Bot merged commit 09c7c12 into rust-lang:main Sep 14, 2026
13 checks passed

rustbot added this to the 1.100.0 milestone Sep 14, 2026

nnethercote deleted the fix-target_config-cycle branch September 14, 2026 21:55

github-actions Bot mentioned this pull request Oct 2, 2026

Toolchain upgrade to nightly-2026-09-25 flux-rs/flux#1803

Merged

Uh oh!

Conversation

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

Uh oh!

Uh oh!

rustbot commented Aug 28, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Aug 28, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

RalfJung left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Aug 31, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Aug 31, 2026

Uh oh!

nnethercote commented Sep 13, 2026

Uh oh!

nikic commented Sep 14, 2026

Uh oh!

rust-bors Bot commented Sep 14, 2026

Uh oh!

Uh oh!

rust-timer commented Sep 14, 2026

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

nnethercote commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nnethercote commented Aug 28, 2026 •

edited by rustbot

Loading

RalfJung left a comment •

edited by rustbot

Loading