From 23ee523ea61355c332481566bb5a093f29cdce59 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 21 Mar 2024 10:35:19 +1100 Subject: [PATCH 1/2] Remove `CodegenBackend::target_override`. Backend and target selection is a mess: the target can override the backend (via `Target::default_codegen_backend`), *and* the backend can override the target (via `CodegenBackend::target_override`). The code that handles this is ugly. It calls `build_target_config` twice, once before getting the backend and once again afterward. It also must check that both overrides aren't triggering at the same time. This commit removes the latter override. It's used in rust-gpu but @eddyb said via Zulip that removing it would be ok. This simplifies the code greatly, and will allow some nice follow-up refactorings. --- .../rustc_codegen_ssa/src/traits/backend.rs | 7 --- compiler/rustc_driver_impl/src/lib.rs | 4 +- compiler/rustc_interface/src/interface.rs | 55 +++++-------------- compiler/rustc_interface/src/tests.rs | 3 +- compiler/rustc_session/src/config.rs | 45 ++++++--------- compiler/rustc_target/src/spec/mod.rs | 3 + 6 files changed, 37 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index e6d42c596d2af..e8b9490d4010a 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -21,7 +21,6 @@ use rustc_session::{ }; use rustc_span::symbol::Symbol; use rustc_target::abi::call::FnAbi; -use rustc_target::spec::Target; use std::fmt; @@ -70,12 +69,6 @@ pub trait CodegenBackend { fn print_passes(&self) {} fn print_version(&self) {} - /// If this plugin provides additional builtin targets, provide the one enabled by the options here. - /// Be careful: this is called *before* init() is called. - fn target_override(&self, _opts: &config::Options) -> Option { - None - } - /// The metadata loader used to load rlib and dylib metadata. /// /// Alternative codegen backends may want to use different rlib or dylib formats than the diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 938f9f0beaacd..716e31080dd30 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -890,7 +890,7 @@ pub fn version_at_macro_invocation( let backend_name = debug_flags.iter().find_map(|x| x.strip_prefix("codegen-backend=")); let opts = config::Options::default(); let sysroot = filesearch::materialize_sysroot(opts.maybe_sysroot.clone()); - let target = config::build_target_config(early_dcx, &opts, None, &sysroot); + let target = config::build_target_config(early_dcx, &opts, &sysroot); get_codegen_backend(early_dcx, &sysroot, backend_name, &target).print_version(); } @@ -1100,7 +1100,7 @@ pub fn describe_flag_categories(early_dcx: &EarlyDiagCtxt, matches: &Matches) -> let opts = config::Options::default(); let sysroot = filesearch::materialize_sysroot(opts.maybe_sysroot.clone()); - let target = config::build_target_config(early_dcx, &opts, None, &sysroot); + let target = config::build_target_config(early_dcx, &opts, &sysroot); get_codegen_backend(early_dcx, &sysroot, backend_name, &target).print_passes(); return true; diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 1a82e6c6910ae..8ba14d3798205 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -341,51 +341,22 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se let sysroot = filesearch::materialize_sysroot(config.opts.maybe_sysroot.clone()); - let (codegen_backend, target_override) = match config.make_codegen_backend { - None => { - // Build a target without override, so that it can override the backend if needed - let target = - config::build_target_config(&early_dcx, &config.opts, None, &sysroot); - - let backend = util::get_codegen_backend( - &early_dcx, - &sysroot, - config.opts.unstable_opts.codegen_backend.as_deref(), - &target, - ); - - // target_override is documented to be called before init(), so this is okay - let target_override = backend.target_override(&config.opts); - - // Assert that we don't use target's override of the backend and - // backend's override of the target at the same time - if config.opts.unstable_opts.codegen_backend.is_none() - && target.default_codegen_backend.is_some() - && target_override.is_some() - { - rustc_middle::bug!( - "Codegen backend requested target override even though the target requested the backend" - ); - } - - (backend, target_override) - } + let target = config::build_target_config(&early_dcx, &config.opts, &sysroot); + + let codegen_backend = match config.make_codegen_backend { + None => util::get_codegen_backend( + &early_dcx, + &sysroot, + config.opts.unstable_opts.codegen_backend.as_deref(), + &target, + ), Some(make_codegen_backend) => { - // N.B. `make_codegen_backend` takes precedence over `target.default_codegen_backend`, - // which is ignored in this case. - let backend = make_codegen_backend(&config.opts); - - // target_override is documented to be called before init(), so this is okay - let target_override = backend.target_override(&config.opts); - - (backend, target_override) + // N.B. `make_codegen_backend` takes precedence over + // `target.default_codegen_backend`, which is ignored in this case. + make_codegen_backend(&config.opts) } }; - // Re-build target with the (potential) override - let target_cfg = - config::build_target_config(&early_dcx, &config.opts, target_override, &sysroot); - let temps_dir = config.opts.unstable_opts.temps_dir.as_deref().map(PathBuf::from); let bundle = match rustc_errors::fluent_bundle( @@ -418,7 +389,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se locale_resources, config.lint_caps, config.file_loader, - target_cfg, + target, sysroot, util::rustc_version_str().unwrap_or("unknown"), config.ice_file, diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 4d8e749d1da7b..c796448e5ee8c 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -41,8 +41,7 @@ fn mk_session(matches: getopts::Matches) -> (Session, Cfg) { let sysroot = filesearch::materialize_sysroot(sessopts.maybe_sysroot.clone()); - let target_cfg = - rustc_session::config::build_target_config(&early_dcx, &sessopts, None, &sysroot); + let target_cfg = rustc_session::config::build_target_config(&early_dcx, &sessopts, &sysroot); let sess = build_session( early_dcx, diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index e6eb1a3e83c6c..a88ae268e277b 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -22,7 +22,7 @@ use rustc_span::{FileName, FileNameDisplayPreference, RealFileName, SourceFileHa use rustc_target::abi::Align; use rustc_target::spec::LinkSelfContainedComponents; use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, SplitDebuginfo}; -use rustc_target::spec::{Target, TargetTriple, TargetWarnings, TARGETS}; +use rustc_target::spec::{Target, TargetTriple, TARGETS}; use std::collections::btree_map::{ Iter as BTreeMapIter, Keys as BTreeMapKeysIter, Values as BTreeMapValuesIter, }; @@ -1549,34 +1549,25 @@ pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg { user_cfg } -pub fn build_target_config( - early_dcx: &EarlyDiagCtxt, - opts: &Options, - target_override: Option, - sysroot: &Path, -) -> Target { - let target_result = target_override.map_or_else( - || Target::search(&opts.target_triple, sysroot), - |t| Ok((t, TargetWarnings::empty())), - ); - let (target, target_warnings) = target_result.unwrap_or_else(|e| { - early_dcx.early_fatal(format!( +pub fn build_target_config(early_dcx: &EarlyDiagCtxt, opts: &Options, sysroot: &Path) -> Target { + match Target::search(&opts.target_triple, sysroot) { + Ok((target, warnings)) => { + for warning in warnings.warning_messages() { + early_dcx.early_warn(warning) + } + if !matches!(target.pointer_width, 16 | 32 | 64) { + early_dcx.early_fatal(format!( + "target specification was invalid: unrecognized target-pointer-width {}", + target.pointer_width + )) + } + target + } + Err(e) => early_dcx.early_fatal(format!( "Error loading target specification: {e}. \ - Run `rustc --print target-list` for a list of built-in targets" - )) - }); - for warning in target_warnings.warning_messages() { - early_dcx.early_warn(warning) - } - - if !matches!(target.pointer_width, 16 | 32 | 64) { - early_dcx.early_fatal(format!( - "target specification was invalid: unrecognized target-pointer-width {}", - target.pointer_width - )) + Run `rustc --print target-list` for a list of built-in targets" + )), } - - target } #[derive(Copy, Clone, PartialEq, Eq, Debug)] diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 941d767b850dc..e10282ae8a9c1 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -2091,6 +2091,9 @@ pub struct TargetOptions { /// compiling `rustc` will be used instead (or llvm if it is not set). /// /// N.B. when *using* the compiler, backend can always be overridden with `-Zcodegen-backend`. + /// + /// This was added by WaffleLapkin in #116793. The motivation is a rustc fork that requires a + /// custom codegen backend for a particular target. pub default_codegen_backend: Option>, /// Whether to generate trap instructions in places where optimization would From 5744be2727a7ebc7726a6e23d517a41cc1e8ffe2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 21 Mar 2024 11:40:13 +1100 Subject: [PATCH 2/2] Rename some `target_cfg` variables as `target`. Because the underlying type is called `Target`. (There is also a separate type called `TargetCfg`.) --- compiler/rustc_interface/src/tests.rs | 4 ++-- compiler/rustc_session/src/session.rs | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index c796448e5ee8c..8a27e9a6453b2 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -41,7 +41,7 @@ fn mk_session(matches: getopts::Matches) -> (Session, Cfg) { let sysroot = filesearch::materialize_sysroot(sessopts.maybe_sysroot.clone()); - let target_cfg = rustc_session::config::build_target_config(&early_dcx, &sessopts, &sysroot); + let target = rustc_session::config::build_target_config(&early_dcx, &sessopts, &sysroot); let sess = build_session( early_dcx, @@ -52,7 +52,7 @@ fn mk_session(matches: getopts::Matches) -> (Session, Cfg) { vec![], Default::default(), None, - target_cfg, + target, sysroot, "", None, diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9c94fd7027fd7..e6d82d6fab352 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1008,7 +1008,7 @@ pub fn build_session( fluent_resources: Vec<&'static str>, driver_lint_caps: FxHashMap, file_loader: Option>, - target_cfg: Target, + target: Target, sysroot: PathBuf, cfg_version: &'static str, ice_file: Option, @@ -1036,7 +1036,7 @@ pub fn build_session( let loader = file_loader.unwrap_or_else(|| Box::new(RealFileLoader)); let hash_kind = sopts.unstable_opts.src_hash_algorithm.unwrap_or_else(|| { - if target_cfg.is_like_msvc { + if target.is_like_msvc { SourceFileHashAlgorithm::Sha256 } else { SourceFileHashAlgorithm::Md5 @@ -1117,11 +1117,10 @@ pub fn build_session( _ => CtfeBacktrace::Disabled, }); - let asm_arch = - if target_cfg.allow_asm { InlineAsmArch::from_str(&target_cfg.arch).ok() } else { None }; + let asm_arch = if target.allow_asm { InlineAsmArch::from_str(&target.arch).ok() } else { None }; let sess = Session { - target: target_cfg, + target, host, opts: sopts, host_tlib_path,