Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap: Remove the distinction between compiler and compiler_for #96176

Open
jyn514 opened this issue Apr 18, 2022 · 7 comments
Open

bootstrap: Remove the distinction between compiler and compiler_for #96176

jyn514 opened this issue Apr 18, 2022 · 7 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 18, 2022

I am not sure whether compiler/compiler_for is a meaningful + worthwhile distinction; I think the best way to find out is probably to try to unify and see what goes wrong. I suspect the intent behind the addition was largely the typical "it seems like this one place wants something slightly different" that perhaps was more of a patchwork fix than done with long-term correctness, so it's unlikely to be super principled I expect in terms of where it's used and such.

I tried changing this and it went ... not well. Removing compiler altogether gives 71 build errors, and changing it to alias compiler_for(stage, host, host) fails a bunch of the tests, including some that look like real issues:

---- builder::tests::dist::dist_with_targets_and_hosts stdout ----
thread 'builder::tests::dist::dist_with_targets_and_hosts' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
 [
     Rustc {
         compiler: Compiler {
<            stage: 1,
>            stage: 2,
             host: A,
>        },
>    },
>    Rustc {
>        compiler: Compiler {
>            stage: 2,
>            host: B,
         },
     },
 ]

', src/bootstrap/builder/tests.rs:314:9

Looking at the call sites, they usually look like this:

            fn make_run(run: RunConfig<'_>) {
                run.builder.ensure($name {
                    compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
                    target: run.target,
                    extra_features: Vec::new(),
                });
            }

so I think the "proper" fix is to pass run.target into compiler_for in most of these cases; at very least, we can't assume it's always host.

In some cases it may not be clear what target is appropriate - feel free to ask on Zulip.

@rustbot label +A-rustbuild +E-mentor +E-medium

Originally posted by @jyn514 in #96000 (comment)

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Apr 18, 2022
@ohno418
Copy link
Contributor

ohno418 commented Apr 28, 2022

I'd like to work on this.

@rustbot claim

@jyn514
Copy link
Member Author

jyn514 commented May 6, 2022

Hi @ohno418, have you had time to work on this? Happy to help out if you're running into trouble :)

@ohno418
Copy link
Contributor

ohno418 commented May 6, 2022

Currently, I'm looking into the bootstrapping process. I'll ask you on Zulip about the details that I don't understand, maybe within a week or so.
Thanks so much for your concern, @jyn514 !

@jyn514
Copy link
Member Author

jyn514 commented May 6, 2022

@ohno418 here are some existing resources on how bootstrapping works: https://discord.com/channels/442252698964721669/487245758739906560/964730715222732800
I will try to write this up into a proper README at some point.

@ohno418
Copy link
Contributor

ohno418 commented May 7, 2022

Thank you! I'll check it.

@jyn514
Copy link
Member Author

jyn514 commented May 24, 2022

@ohno418 I worry this is harder than I expected it to be originally :( don't feel obligated to keep working on it if you're stuck, I can find other useful work for you to do :)

@rustbot label +E-hard -E-medium

@rustbot rustbot added E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. and removed E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels May 24, 2022
@ohno418
Copy link
Contributor

ohno418 commented Sep 13, 2022

I've been working on this for a while but found some difficulties. Or maybe I just don't understand much.

For future tips, leaving the part that I got stuck, and some advice that @jyn514 gave me:
https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/bootstrap.3A.20compiler.28.29.20and.20compiler_for.28.29/near/285070882

@rustbot release-assignment

@jieyouxu jieyouxu added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Feb 15, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 16, 2025
bootstrap: add more tracing to compiler/std/llvm flows

- Add more tracing to compiler/std/llvm flows.
- Two drive-by nits:
    1. Take `TargetSelection` by-value for `builder.is_builder_target()`. Noticed while adding tracing; follow-up to rust-lang#136767.
    2. Coalesce enzyme build logic into one branch.
- Document `COMPILER{,_FOR}` tracing targets for rust-lang#96176.
- No functional changes.

### Testing

You can play with the tracing locally with:

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

### Previews

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
```

![Screenshot 2025-02-15 230824](https://github.com/user-attachments/assets/c3b02b62-d52e-4c03-a00a-da0d95618989)

```
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

![Screenshot 2025-02-15 233859](https://github.com/user-attachments/assets/842e4ece-4c26-4191-acbb-5f93e42de4dc)

r? `@onur-ozkan` (or reroll)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 17, 2025
bootstrap: add more tracing to compiler/std/llvm flows

- Add more tracing to compiler/std/llvm flows.
- Two drive-by nits:
    1. Take `TargetSelection` by-value for `builder.is_builder_target()`. Noticed while adding tracing; follow-up to rust-lang#136767.
    2. Coalesce enzyme build logic into one branch.
- Document `COMPILER{,_FOR}` tracing targets for rust-lang#96176.
- No functional changes.

### Testing

You can play with the tracing locally with:

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

### Previews

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
```

![Screenshot 2025-02-15 230824](https://github.com/user-attachments/assets/c3b02b62-d52e-4c03-a00a-da0d95618989)

```
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

![Screenshot 2025-02-15 233859](https://github.com/user-attachments/assets/842e4ece-4c26-4191-acbb-5f93e42de4dc)

r? ``@onur-ozkan`` (or reroll)
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 17, 2025
Rollup merge of rust-lang#137080 - jieyouxu:more-tracing, r=onur-ozkan

bootstrap: add more tracing to compiler/std/llvm flows

- Add more tracing to compiler/std/llvm flows.
- Two drive-by nits:
    1. Take `TargetSelection` by-value for `builder.is_builder_target()`. Noticed while adding tracing; follow-up to rust-lang#136767.
    2. Coalesce enzyme build logic into one branch.
- Document `COMPILER{,_FOR}` tracing targets for rust-lang#96176.
- No functional changes.

### Testing

You can play with the tracing locally with:

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace ./x build library
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

### Previews

```
$ BOOTSTRAP_TRACING=bootstrap=debug ./x build library
```

![Screenshot 2025-02-15 230824](https://github.com/user-attachments/assets/c3b02b62-d52e-4c03-a00a-da0d95618989)

```
$ BOOTSTRAP_TRACING=bootstrap=trace,COMPILER=trace,COMPILER_FOR=trace ./x build library
```

![Screenshot 2025-02-15 233859](https://github.com/user-attachments/assets/842e4ece-4c26-4191-acbb-5f93e42de4dc)

r? ``@onur-ozkan`` (or reroll)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

4 participants