Remove the "codegen" profile from bootstrap #121278
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @clubby789 rustbot has assigned @clubby789. Use r? to explicitly pick a reviewer |
rustbot
added
A-meta
|
This PR changes src/bootstrap/defaults/config.compiler.toml. If appropriate, please also update This PR modifies If appropriate, please update This PR changes src/bootstrap/defaults/config.codegen.toml. If appropriate, please also update |
Nilstrieb
force-pushed
the
no-more-codegen
branch
from
bc79763
to
c9677c9
Compare
Nilstrieb
commented
Feb 18, 2024
| // The submodule will be downloaded. It does not work in the | ||
| // tarball case since they don't include Git and submodules | ||
| // are already included. | ||
| if !builder.rust_info().is_from_tarball() { |
There was a problem hiding this comment.
I don't think there's much benefit from this. Not everyone doing codegen will be modifying LLVM and anyone that wants to will figure out that they need to init the submodule somehow
|
☔ The latest upstream changes (presumably #121203) made this pull request unmergeable. Please resolve the merge conflicts. |
This profile originally made sense when download-ci-llvm = if-unchanged didn't exist and we had the bad tradeoff of "never modify or always compile". Thankfully, these grim times are over and we have discovered clean water, so the only differentiator between the two profiles is the codegen profile having LLVM assertions. Adding them doesn't cause that much of a slowdown, <10% on UI tests from an unscientific benchmark. It also had LLVM warnings when compiling, which makes sense for every compiler contributor brave enough to compile LLVM. The way I removed is by just issueing a nice error message. Given that everyone with this profile should be a contributor and not someone like a distro who is more upset when things break, this should be fine. If it isn't, we can always fall back to just letting codegen mean compiler.
Nilstrieb
force-pushed
the
no-more-codegen
branch
from
c9677c9
to
9e68d89
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
Nilstrieb
added a commit
to Nilstrieb/rust
that referenced
this pull request
Feb 20, 2024
…y789 Remove the "codegen" profile from bootstrap This profile originally made sense when download-ci-llvm = if-unchanged didn't exist and we had the bad tradeoff of "never modify or always compile". Thankfully, these grim times are over and we have discovered clean water, so the only differentiator between the two profiles is the codegen profile having LLVM assertions. Adding them doesn't cause that much of a slowdown, <10% on UI tests from an unscientific benchmark. It also had LLVM warnings when compiling, which makes sense for every compiler contributor brave enough to compile LLVM. The way I removed is by just issueing a nice error message. Given that everyone with this profile should be a contributor and not someone like a distro who is more upset when things break, this should be fine. If it isn't, we can always fall back to just letting codegen mean compiler.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 20, 2024
Rollup of 8 pull requests Successful merges: - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates) - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)) - rust-lang#121206 (Top level error handling) - rust-lang#121223 (intrinsics::simd: add missing functions) - rust-lang#121241 (Implement `NonZero` traits generically.) - rust-lang#121242 (Generate `getelementptr` instead of `inttoptr` for `ptr::invalid`) - rust-lang#121278 (Remove the "codegen" profile from bootstrap) - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`) r? `@ghost` `@rustbot` modify labels: rollup
Nilstrieb
added a commit
to Nilstrieb/rust
that referenced
this pull request
Feb 20, 2024
…y789 Remove the "codegen" profile from bootstrap This profile originally made sense when download-ci-llvm = if-unchanged didn't exist and we had the bad tradeoff of "never modify or always compile". Thankfully, these grim times are over and we have discovered clean water, so the only differentiator between the two profiles is the codegen profile having LLVM assertions. Adding them doesn't cause that much of a slowdown, <10% on UI tests from an unscientific benchmark. It also had LLVM warnings when compiling, which makes sense for every compiler contributor brave enough to compile LLVM. The way I removed is by just issueing a nice error message. Given that everyone with this profile should be a contributor and not someone like a distro who is more upset when things break, this should be fine. If it isn't, we can always fall back to just letting codegen mean compiler.
This was referenced Feb 20, 2024
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 20, 2024
Rollup of 8 pull requests Successful merges: - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates) - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)) - rust-lang#121241 (Implement `NonZero` traits generically.) - rust-lang#121278 (Remove the "codegen" profile from bootstrap) - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`) - rust-lang#121291 (target: Revert default to the medium code model on LoongArch targets) - rust-lang#121302 (Remove `RefMutL` hack in `proc_macro::bridge`) - rust-lang#121318 (Trigger `unsafe_code` lint on invocations of `global_asm`) Failed merges: - rust-lang#121206 (Top level error handling) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 20, 2024
Rollup merge of rust-lang#121278 - Nilstrieb:no-more-codegen, r=clubby789 Remove the "codegen" profile from bootstrap This profile originally made sense when download-ci-llvm = if-unchanged didn't exist and we had the bad tradeoff of "never modify or always compile". Thankfully, these grim times are over and we have discovered clean water, so the only differentiator between the two profiles is the codegen profile having LLVM assertions. Adding them doesn't cause that much of a slowdown, <10% on UI tests from an unscientific benchmark. It also had LLVM warnings when compiling, which makes sense for every compiler contributor brave enough to compile LLVM. The way I removed is by just issueing a nice error message. Given that everyone with this profile should be a contributor and not someone like a distro who is more upset when things break, this should be fine. If it isn't, we can always fall back to just letting codegen mean compiler.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This profile originally made sense when download-ci-llvm = if-unchanged didn't exist and we had the bad tradeoff of "never modify or always compile".
Thankfully, these grim times are over and we have discovered clean water, so the only differentiator between the two profiles is the codegen profile having LLVM assertions. Adding them doesn't cause that much of a slowdown, <10% on UI tests from an unscientific benchmark.
It also had LLVM warnings when compiling, which makes sense for every compiler contributor brave enough to compile LLVM.
The way I removed is by just issueing a nice error message. Given that everyone with this profile should be a contributor and not someone like a distro who is more upset when things break, this should be fine. If it isn't, we can always fall back to just letting codegen mean compiler.