Skip to content

Commit

Permalink
Remove the "codegen" profile from bootstrap
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Noratrieb committed Feb 19, 2024
1 parent e29a153 commit 9e68d89
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 49 deletions.
28 changes: 0 additions & 28 deletions src/bootstrap/defaults/config.codegen.toml

This file was deleted.

5 changes: 5 additions & 0 deletions src/bootstrap/defaults/config.compiler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,10 @@ lto = "off"
frame-pointers = true

[llvm]
# This enables debug-assertions in LLVM,
# catching logic errors in codegen much earlier in the process.
assertions = true
# Enable warnings during the LLVM compilation (when LLVM is changed, causing a compilation)
enable-warnings = true
# Will download LLVM from CI if available on your platform.
download-ci-llvm = "if-unchanged"
23 changes: 7 additions & 16 deletions src/bootstrap/src/core/build_steps/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ mod tests;
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
pub enum Profile {
Compiler,
Codegen,
Library,
Tools,
Dist,
Expand Down Expand Up @@ -48,15 +47,14 @@ impl Profile {
pub fn all() -> impl Iterator<Item = Self> {
use Profile::*;
// N.B. these are ordered by how they are displayed, not alphabetically
[Library, Compiler, Codegen, Tools, Dist, None].iter().copied()
[Library, Compiler, Tools, Dist, None].iter().copied()
}

pub fn purpose(&self) -> String {
use Profile::*;
match self {
Library => "Contribute to the standard library",
Compiler => "Contribute to the compiler itself",
Codegen => "Contribute to the compiler, and also modify LLVM or codegen",
Tools => "Contribute to tools which depend on the compiler, but do not modify it directly (e.g. rustdoc, clippy, miri)",
Dist => "Install Rust from source",
None => "Do not modify `config.toml`"
Expand All @@ -75,7 +73,6 @@ impl Profile {
pub fn as_str(&self) -> &'static str {
match self {
Profile::Compiler => "compiler",
Profile::Codegen => "codegen",
Profile::Library => "library",
Profile::Tools => "tools",
Profile::Dist => "dist",
Expand All @@ -91,12 +88,15 @@ impl FromStr for Profile {
match s {
"lib" | "library" => Ok(Profile::Library),
"compiler" => Ok(Profile::Compiler),
"llvm" | "codegen" => Ok(Profile::Codegen),
"maintainer" | "dist" | "user" => Ok(Profile::Dist),
"tools" | "tool" | "rustdoc" | "clippy" | "miri" | "rustfmt" | "rls" => {
Ok(Profile::Tools)
}
"none" => Ok(Profile::None),
"llvm" | "codegen" => Err(format!(
"the \"llvm\" and \"codegen\" profiles have been removed,\
use \"compiler\" instead which has the same functionality"
)),
_ => Err(format!("unknown profile: '{s}'")),
}
}
Expand Down Expand Up @@ -170,22 +170,13 @@ impl Step for Profile {
}

fn run(self, builder: &Builder<'_>) {
// During ./x.py setup once you select the codegen profile.
// 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() {
if self == Profile::Codegen {
builder.update_submodule(&Path::new("src/llvm-project"));
}
}
setup(&builder.build.config, self)
setup(&builder.build.config, self);
}
}

pub fn setup(config: &Config, profile: Profile) {
let suggestions: &[&str] = match profile {
Profile::Codegen | Profile::Compiler | Profile::None => &["check", "build", "test"],
Profile::Compiler | Profile::None => &["check", "build", "test"],
Profile::Tools => &[
"check",
"build",
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Info,
summary: "A new `rust.frame-pointers` option has been introduced and made the default in the compiler and codegen profiles.",
},
ChangeInfo {
change_id: 121278,
severity: ChangeSeverity::Warning,
summary: "The \"codegen\"/\"llvm\" profile has been removed and replaced with \"compiler\", use it instead for the same behavior.",
},
];
5 changes: 0 additions & 5 deletions triagebot.toml
Original file line number Diff line number Diff line change
Expand Up @@ -627,11 +627,6 @@ This PR modifies `config.example.toml`.
If appropriate, please update `CONFIG_CHANGE_HISTORY` in `src/bootstrap/src/utils/change_tracker.rs`.
"""

[mentions."src/bootstrap/defaults/config.compiler.toml"]
message = "This PR changes src/bootstrap/defaults/config.compiler.toml. If appropriate, please also update `config.codegen.toml` so the defaults are in sync."
[mentions."src/bootstrap/defaults/config.codegen.toml"]
message = "This PR changes src/bootstrap/defaults/config.codegen.toml. If appropriate, please also update `config.compiler.toml` so the defaults are in sync."

[mentions."src/bootstrap/src/core/build_steps/llvm.rs"]
message = "This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp."

Expand Down

0 comments on commit 9e68d89

Please sign in to comment.