Allow targets to override default codegen backend #116793

WaffleLapkin · 2023-10-16T12:48:17Z

Implements rust-lang/compiler-team#670.

rustbot · 2023-10-16T12:48:27Z

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

cjgillot · 2023-10-16T16:37:21Z

cc @davidtwco

cjgillot

@davidtwco do you have any comment?
r=me with one nit unless @davidtwco says otherwise

cjgillot · 2023-11-04T16:11:17Z

compiler/rustc_driver_impl/src/lib.rs

+        let sopts = config::Options::default();
+        let sysroot = sopts.maybe_sysroot.clone().unwrap_or_else(|| {
+            filesearch::get_or_default_sysroot().expect("Failed finding sysroot")
+        });
+        let target = config::build_target_config(handler, &sopts, None, &sysroot);


This snippet appears several times. Any way to refactor it into the normal code path?

Idk, the crate structure and everything is very confusing, so I'm not sure how to refactor this...

I've moved the maybe_sysroot->sysroot into a function, but I don't think I can do much more

davidtwco

LGTM, with @cjgillot's comments addressed to their satisfaction :)

rustbot · 2023-11-14T20:35:32Z

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

cjgillot · 2024-02-18T17:54:27Z

compiler/rustc_interface/src/interface.rs

+                    // target_override is documented to be called before init(), so this is okay
+                    let target_override = backend.target_override(&config.opts);
+
+                    let target = config::build_target_config(
+                        &early_dcx,
+                        &config.opts,
+                        target_override,
+                        &sysroot,
+                    );


This part is duplicated on both arms. Any way to deduplicate?

6d115f5. can't move target_override because of the assertion.

cjgillot · 2024-03-05T23:26:47Z

@bors r+

bors · 2024-03-05T23:26:50Z

📌 Commit 6d115f5 has been approved by cjgillot

It is now in the queue for this repository.

…end, r=cjgillot Allow targets to override default codegen backend Implements rust-lang/compiler-team#670.

…kingjubilee Rollup of 13 pull requests Successful merges: - rust-lang#113525 (Dynamically size sigaltstk in std) - rust-lang#114009 (compiler: allow transmute of ZST arrays with generics) - rust-lang#116793 (Allow targets to override default codegen backend) - rust-lang#118623 (Improve std::fs::read_to_string example) - rust-lang#120504 (Vec::try_with_capacity) - rust-lang#121089 (Remove `feed_local_def_id`) - rust-lang#121280 (Implement MaybeUninit::fill{,_with,_from}) - rust-lang#122087 (Add missing background color for top-level rust documentation page and increase contrast by setting text color to black) - rust-lang#122104 (Rust is a proper name: rust → Rust) - rust-lang#122110 (Make `x t miri` respect `MIRI_TEMP`) - rust-lang#122114 (Make not finding core a fatal error) - rust-lang#122115 (Cancel parsing ever made during recovery) - rust-lang#122126 (Fix `tidy --bless` on ̶X̶e̶n̶i̶x̶ Windows) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 15 pull requests Successful merges: - rust-lang#116791 (Allow codegen backends to opt-out of parallel codegen) - rust-lang#116793 (Allow targets to override default codegen backend) - rust-lang#117458 (LLVM Bitcode Linker: A self contained linker for nvptx and other targets) - rust-lang#119385 (Fix type resolution of associated const equality bounds (take 2)) - rust-lang#121438 (std support for wasm32 panic=unwind) - rust-lang#121893 (Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking) - rust-lang#122080 (Clarity improvements to `DropTree`) - rust-lang#122152 (Improve diagnostics for parenthesized type arguments) - rust-lang#122166 (Remove the unused `field_remapping` field from `TypeLowering`) - rust-lang#122249 (interpret: do not call machine read hooks during validation) - rust-lang#122299 (Store backtrace for `must_produce_diag`) - rust-lang#122318 (Revision-related tweaks for next-solver tests) - rust-lang#122320 (Use ptradd for vtable indexing) - rust-lang#122328 (unix_sigpipe: Replace `inherit` with `sig_dfl` in syntax tests) - rust-lang#122330 (bootstrap readme: fix, improve, update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#116793 - WaffleLapkin:target_rules_the_backend, r=cjgillot Allow targets to override default codegen backend Implements rust-lang/compiler-team#670.

RalfJung · 2024-03-25T08:15:49Z

compiler/rustc_session/src/config.rs

@@ -2909,7 +2901,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
        describe_lints,
        output_types,
        search_paths,
-        maybe_sysroot: sysroot_opt,
+        maybe_sysroot: Some(sysroot),


This made it impossible for the config driver callback to determine whether the --sysroot flag was passed or not, which is a problem for drivers that want to overwrite the default sysroot but honor explicit --sysroot. I'm trying to fix this in #122993 but I don't really know what I am doing...

eddyb · 2024-08-03T13:16:34Z

compiler/rustc_interface/src/interface.rs

-            let codegen_backend = if let Some(make_codegen_backend) = config.make_codegen_backend {
-                make_codegen_backend(&config.opts)
-            } else {
-                util::get_codegen_backend(
-                    &early_dcx,
-                    &config.opts.maybe_sysroot,
-                    config.opts.unstable_opts.codegen_backend.as_deref(),
-                )
+            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);


This doesn't really do the same thing as before, or rather it assumes custom codegen backends require config.make_codegen_backend, whereas config.opts.unstable_opts.codegen_backend is also possible.

The new version calls config::build_target_config once, without the override, which means the backend cannot add new targets, only at most slightly tweak existing ones.

To be clear, none of this really matters anymore due to:

Remove target_override #122810

(but I ran into it while trying to update Rust-GPU to nightly-2024-03-21, just before #122810, just so I can get everything else out of the way before tackling the target_override removal itself)

rustbot assigned cjgillot Oct 16, 2023

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 Oct 16, 2023

This comment was marked as resolved.

Sign in to view

cjgillot reviewed Nov 4, 2023

View reviewed changes

davidtwco approved these changes Nov 6, 2023

View reviewed changes

WaffleLapkin 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 Nov 6, 2023

rust-cloud-vms bot force-pushed the target_rules_the_backend branch from a699d87 to eedd680 Compare November 14, 2023 20:35

Allow targets to override default codegen backend

a03d19e

rust-cloud-vms bot force-pushed the target_rules_the_backend branch from eedd680 to a03d19e Compare February 14, 2024 23:43

Refactor out a repeating pattern with get_or_default_sysroot

5441523

rust-cloud-vms bot force-pushed the target_rules_the_backend branch from 37fc0ba to 5441523 Compare February 15, 2024 00:09

WaffleLapkin requested a review from cjgillot February 15, 2024 00:12

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 Feb 15, 2024

cjgillot reviewed Feb 18, 2024

View reviewed changes

cjgillot 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 Feb 18, 2024

Refactor out another repeating pattern

6d115f5

WaffleLapkin requested a review from cjgillot February 18, 2024 19:27

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 Feb 18, 2024

bors 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 Mar 5, 2024

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 7, 2024

Rollup merge of rust-lang#116793 - WaffleLapkin:target_rules_the_back…

9ed2a53

…end, r=cjgillot Allow targets to override default codegen backend Implements rust-lang/compiler-team#670.

workingjubilee mentioned this pull request Mar 7, 2024

Rollup of 13 pull requests #122131

Closed

This was referenced Mar 11, 2024

Rollup of 11 pull requests #122337

Closed

Rollup of 15 pull requests #122338

Merged

bors merged commit 86af4d2 into rust-lang:master Mar 11, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Mar 11, 2024

WaffleLapkin deleted the target_rules_the_backend branch March 12, 2024 15:03

RalfJung reviewed Mar 25, 2024

View reviewed changes

eddyb reviewed Aug 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow targets to override default codegen backend #116793

Allow targets to override default codegen backend #116793

WaffleLapkin commented Oct 16, 2023

rustbot commented Oct 16, 2023

cjgillot commented Oct 16, 2023

This comment was marked as resolved.

cjgillot left a comment

cjgillot Nov 4, 2023

WaffleLapkin Feb 15, 2024

davidtwco left a comment

rustbot commented Nov 14, 2023

cjgillot Feb 18, 2024

WaffleLapkin Feb 18, 2024

cjgillot commented Mar 5, 2024

bors commented Mar 5, 2024

RalfJung Mar 25, 2024

eddyb Aug 3, 2024

Allow targets to override default codegen backend #116793

Allow targets to override default codegen backend #116793

Conversation

WaffleLapkin commented Oct 16, 2023

rustbot commented Oct 16, 2023

cjgillot commented Oct 16, 2023

This comment was marked as resolved.

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot Nov 4, 2023

Choose a reason for hiding this comment

WaffleLapkin Feb 15, 2024

Choose a reason for hiding this comment

davidtwco left a comment

Choose a reason for hiding this comment

rustbot commented Nov 14, 2023

cjgillot Feb 18, 2024

Choose a reason for hiding this comment

WaffleLapkin Feb 18, 2024

Choose a reason for hiding this comment

cjgillot commented Mar 5, 2024

bors commented Mar 5, 2024

RalfJung Mar 25, 2024

Choose a reason for hiding this comment

eddyb Aug 3, 2024

Choose a reason for hiding this comment