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: add more tracing to compiler/std/llvm flows #137080

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Feb 15, 2025

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

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

Screenshot 2025-02-15 233859

r? @onur-ozkan (or reroll)

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Comment on lines 2030 to +2033
// Build enzyme
let enzyme_install = if builder.config.llvm_enzyme {
Some(builder.ensure(llvm::Enzyme { target: build_compiler.host }))
} else {
None
};

if let Some(enzyme_install) = enzyme_install {
if builder.config.llvm_enzyme {
debug!("`llvm_enzyme` requested");
let enzyme_install = builder.ensure(llvm::Enzyme { target: build_compiler.host });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I collapsed this into one single block, I had to do a double-take on this.

Comment on lines -1933 to 1944
fn is_builder_target(&self, target: &TargetSelection) -> bool {
&self.config.build == target
fn is_builder_target(&self, target: TargetSelection) -> bool {
self.config.build == target
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try very hard in bootstrap to keep TargetSelection Copyable, noticed while adding some tracing steps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather say that we have to live with the fact that TargeSelection is Copyable 😅

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member Author

Ugh, it's because trace! expands to empty w/out tracing feature.

@jieyouxu jieyouxu force-pushed the more-tracing branch 2 times, most recently from a8e6b84 to 087d8eb Compare February 15, 2025 15:32
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
[DO NOT MERGE] `./x test rust-analyzer`

I somehow made `./x test rust-analyzer` work on my machine[^machine], **but at what cost?**

Not intended for merge but only as a reference. If we do want to land this, I'll need to tidy this up.

Notes:

- Unrelated tracing bits are extracted to rust-lang#137080.
- I abused a bunch of cargo features  `in-rust-tree`. It probably doesn't need to be, and simply `--cfg` might work. I was trying to get the main rust-analyzer tests to build *at all*. Anything building is already a miracle.
- I had to slap a bunch of the following to all the r-a crates to get the tests to build at all. I don't 100% understand why, but otherwise I get a whole ton of ``expected `rustc_lexer` to be available in rlib format`` build failures.

```rs
#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))]
#[cfg(all(feature = "in-rust-tree", test))]
extern crate rustc_driver as _;
```

- Skipped one config test that was fixed on r-a master but not synced here in r-l/r yet.

[^machine]: `x86_64-unknown-linux-gnu`, haven't bothered trying this on msvc yet.

try-job: aarch64-gnu
try-job: x86_64-apple-1
try-job: aarch64-apple
try-job: i686-mingw-1
try-job: x86_64-mingw-1
try-job: i686-msvc-1
try-job: x86_64-msvc-1
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me other than one nit

@onur-ozkan
Copy link
Member

@rustbot author

@rustbot rustbot 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 16, 2025
@jieyouxu
Copy link
Member Author

Removed "recursively", no other changes since last review.
@rustbot ready

@rustbot 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 16, 2025
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACk 05ba1a4

@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 16, 2025

📌 Commit 05ba1a4 has been approved by onur-ozkan

It is now in the queue for this repository.

@bors 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 Feb 16, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request 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)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#136466 (Start removing `rustc_middle::hir::map::Map`)
 - rust-lang#136671 (Overhaul `rustc_middle::limits`)
 - rust-lang#136817 (Pattern Migration 2024: clean up and comment)
 - rust-lang#136844 (Use `const_error!` when possible)
 - rust-lang#137080 (bootstrap: add more tracing to compiler/std/llvm flows)
 - rust-lang#137101 (`invalid_from_utf8[_unchecked]`: also lint inherent methods)
 - rust-lang#137140 (Fix const items not being allowed to be called `r#move` or `r#static`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5dd48bc into rust-lang:master Feb 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request 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)
@jieyouxu jieyouxu deleted the more-tracing branch February 17, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants