bootstrap: Make rust options configurable per-stage

[jyn514]

There are several cases where it makes sense to have different configuration between different stages. A simple one is debuginfo - we enable rust.debuginfo-level-std = 1 in dist builders, but only need it for stage 2, not stage 1. Another is optimization levels; for developing, it can make sense to build rustc with optimize = true but std with optimize = false (#112678).

We should make it easier to configure this without using RUSTFLAGS_NOT_BOOTSTRAP, which is hard to discover and breaks caching if you get it wrong. I'm imagining something like rust.stage1.debuginfo-level-std, where any option under rust is allowed to be configured per-stage. This also implies that any option under rust should be applicable to a single stage; anything today that isn't should be moved to build.

Mentoring instructions:

1. Audit https://github.com/rust-lang/rust/blob/5dee431d87783cc9fff2e80613959594c20e3336/src/bootstrap/config.rs#L825-L876 for anything that doesn't make sense to set for only a single stage (e.g. channel, musl_root, rpath).
2. Take the options from 1. and move them to build. I expect there to be a lot of these, so if possible add aliases from the old name to the new one - maybe add something like struct MovedOptions { ... } and then #[serde(flatten)] renamed: MovedOptions in both Build and Rust? Make an entry in src/bootstrap/changelog.md.
3. Add a new struct PerStage<T> { stage0: T, stage1: T, stage2: T, default: T } and use it everywhere in Rust; add it to the define_config macro if that makes it easier. This is based off of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/def/struct.PerNS.html and should have similar apis.
4. Add tests for your change in src/bootstrap/config/tests.rs.

[jyn514]

@Kobzol would applying this to profile_use and profile_generate make your life any easier in the dist-opt tool?

[Kobzol]

PGO is already only applied to stage 2, so that is already working fine. Per stage profiles could help for options that are more "global" and which are used for everything (std, tools, compiler) in the function which generates shared cargo opts, as that is now difficult to override. So things like optimize=true, debuginfo, cgu=1, etc.

[Rustin170506]

@rustbot claim

Let me give it a shot.

[Rustin170506]

MovedOptions
optimize
codegen_units
codegen_units_std
debuginfo_level
debuginfo_level_rustc
debuginfo_level_std

NoChanged	Need to move
debug
debug_assertions
overflow_checks
overflow_checks_std
debug_logging
debuginfo_level_tools
debuginfo_level_tests
split_debuginfo
run_dsymutil
backtrace
incremental
parallel_compiler
default_linker
channel
description
musl_root
rpath
verbose_tests
optimize_tests
codegen_tests
omit_git_hash
dist_src
save_toolstates
codegen_backends
lld
use_lld
llvm_tools
deny_warnings
backtrace_on_ice
verify_llvm_ir
thin_lto_import_instr_limit
remap_debuginfo
jemalloc
test_compare_mode
llvm_libunwind
control_flow_guard
new_symbol_mangling
profile_generate
profile_use
download_rustc
lto
validate_mir_opts

@jyn514 Could you please take a look? Does this make sense? Do you think we need to move more parameters? You can modify the checkbox.

[Kobzol]

jyn514 is currently not actively working on Rust, so I'd like to try help move this forward.

There are several things to resolve in the "design". We could apply the per-stage configuration in two ways:

1. Per whole section

[rust]
codegen-units = 1

[rust.stage1]
codegen-units = 16

2. Per configuration

[rust]
codegen-units = { default = 1, stage1 = 16 }

I think that the first option is better, since we define TomlConfig attributes via macros, and it will be easier to create two instances of the whole struct, rather than to micromanage stages per configuration option.

Then, we need a way to query this information. As suggested by @Mark-Simulacrum here, I think that it would make sense to allow querying configuration options with an optional parameter that describes the stage (I would make it optional and introduce a new function for this to avoid changing a million callsites in bootstrap at once). So if you ask for build.rust_codegen_units(Stage::Stage1), if there is some record for stage1, it would return that. If not, it would return the default. One issue with this is that the TOML config is converted eagerly to a different Config struct, but we probably cannot resolve the stages so soon, because e.g. for a stage2 build, we actually want to make a different query for each stage (first stage1, then stage2).

An additional question is how will this interact with the bootstrap stage re-design, and if we even want to keep a distinction of stageN.

[onur-ozkan]

jyn514 is currently not actively working on Rust, so I'd like to try help move this forward.

There are several things to resolve in the "design". We could apply the per-stage configuration in two ways:

1. Per whole section

[rust]
codegen-units = 1

[rust.stage1]
codegen-units = 16

2. Per configuration

[rust]
codegen-units = { default = 1, stage1 = 16 }

I think that the first option is better, since we define TomlConfig attributes via macros, and it will be easier to create two instances of the whole struct, rather than to micromanage stages per configuration option.

I agree, the first option appears to be the better choice in my opinion as well. The second option adds complexity and could potentially lead to breaking changes for current 'config.toml' usage.

With the first option, existing configurations can continue to work seamlessly, and if a stage is not explicitly specified, the same configuration can be applied across all stages. This approach simplifies the handling of configurations and maintains backward compatibility.

One challenge with this implementation is how to load configurations efficiently. For instance, if we lazy-load configurations at each stage, we won't be able to detect errors in invalid configurations until we reach that specific stage.

If we change the Config in bootstrap in a way that can load/hold all the stage specific configurations, then this problem might be solved and also querying configuration options with an optional parameter that describes the stage will not be needed. However, this goes against the concept of 'distinction of stageN'.

[onur-ozkan]

Hi @hi-rustin, have you made any progress so far? Asking because this issue is somewhat of a blocker for #102600.

[Rustin170506]

Hi @hi-rustin, have you made any progress so far? Asking because this issue is somewhat of a blocker for #102600

No, I haven't. I have no time to work on it recently.

[onur-ozkan]

Hi @hi-rustin, have you made any progress so far? Asking because this issue is somewhat of a blocker for #102600

No, I haven't. I have no time to work on it recently.

It's okay. I can work on it.

@rustbot claim