Skip to content

Commit

Permalink
Remove llvm.skip-rebuild option
Browse files Browse the repository at this point in the history
This was added to in 2019 to speed up rebuild times when LLVM was
modified. Now that download-ci-llvm exists, I don't think it makes sense
to support an unsound option like this that can lead to miscompiles; and
the code cleanup is nice too.
  • Loading branch information
jyn514 committed Mar 1, 2023
1 parent 6209e6c commit daf99a4
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 38 deletions.
6 changes: 0 additions & 6 deletions config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ changelog-seen = 2
# Defaults to "if-available" when `channel = "dev"` and "false" otherwise.
#download-ci-llvm = "if-available"

# Indicates whether LLVM rebuild should be skipped when running bootstrap. If
# this is `false` then the compiler's LLVM will be rebuilt whenever the built
# version doesn't have the correct hash. If it is `true` then LLVM will never
# be rebuilt. The default value is `false`.
#skip-rebuild = false

# Indicates whether the LLVM build is a Release or Debug build
#optimize = true

Expand Down
9 changes: 0 additions & 9 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ pub struct Config {
pub backtrace_on_ice: bool,

// llvm codegen options
pub llvm_skip_rebuild: bool,
pub llvm_assertions: bool,
pub llvm_tests: bool,
pub llvm_plugins: bool,
Expand Down Expand Up @@ -664,7 +663,6 @@ define_config! {
define_config! {
/// TOML representation of how the LLVM build is configured.
struct Llvm {
skip_rebuild: Option<bool> = "skip-rebuild",
optimize: Option<bool> = "optimize",
thin_lto: Option<bool> = "thin-lto",
release_debuginfo: Option<bool> = "release-debuginfo",
Expand Down Expand Up @@ -1057,11 +1055,6 @@ impl Config {
config.mandir = install.mandir.map(PathBuf::from);
}

// We want the llvm-skip-rebuild flag to take precedence over the
// skip-rebuild config.toml option so we store it separately
// so that we can infer the right value
let mut llvm_skip_rebuild = flags.llvm_skip_rebuild;

// Store off these values as options because if they're not provided
// we'll infer default values for them later
let mut llvm_assertions = None;
Expand Down Expand Up @@ -1166,7 +1159,6 @@ impl Config {
llvm_assertions = llvm.assertions;
llvm_tests = llvm.tests;
llvm_plugins = llvm.plugins;
llvm_skip_rebuild = llvm_skip_rebuild.or(llvm.skip_rebuild);
set(&mut config.llvm_optimize, llvm.optimize);
set(&mut config.llvm_thin_lto, llvm.thin_lto);
set(&mut config.llvm_release_debuginfo, llvm.release_debuginfo);
Expand Down Expand Up @@ -1329,7 +1321,6 @@ impl Config {
// Now that we've reached the end of our configuration, infer the
// default values for all options that we haven't otherwise stored yet.

config.llvm_skip_rebuild = llvm_skip_rebuild.unwrap_or(false);
config.llvm_assertions = llvm_assertions.unwrap_or(false);
config.llvm_tests = llvm_tests.unwrap_or(false);
config.llvm_plugins = llvm_plugins.unwrap_or(false);
Expand Down
13 changes: 0 additions & 13 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ pub struct Flags {
// true => deny, false => warn
pub deny_warnings: Option<bool>,

pub llvm_skip_rebuild: Option<bool>,

pub rust_profile_use: Option<String>,
pub rust_profile_generate: Option<String>,

Expand Down Expand Up @@ -249,14 +247,6 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
opts.optopt("", "error-format", "rustc error format", "FORMAT");
opts.optflag("", "json-output", "use message-format=json");
opts.optopt("", "color", "whether to use color in cargo and rustc output", "STYLE");
opts.optopt(
"",
"llvm-skip-rebuild",
"whether rebuilding llvm should be skipped \
a VALUE of TRUE indicates that llvm will not be rebuilt \
VALUE overrides the skip-rebuild option in config.toml.",
"VALUE",
);
opts.optopt(
"",
"rust-profile-generate",
Expand Down Expand Up @@ -707,9 +697,6 @@ Arguments:
.collect::<Vec<_>>(),
include_default_paths: matches.opt_present("include-default-paths"),
deny_warnings: parse_deny_warnings(&matches),
llvm_skip_rebuild: matches.opt_str("llvm-skip-rebuild").map(|s| s.to_lowercase()).map(
|s| s.parse::<bool>().expect("`llvm-skip-rebuild` should be either true or false"),
),
color: matches
.opt_get_default("color", Color::Auto)
.expect("`color` should be `always`, `never`, or `auto`"),
Expand Down
9 changes: 0 additions & 9 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,6 @@ pub fn prebuilt_llvm_config(
let stamp = out_dir.join("llvm-finished-building");
let stamp = HashStamp::new(stamp, builder.in_tree_llvm_info.sha());

if builder.config.llvm_skip_rebuild && stamp.path.exists() {
builder.info(
"Warning: \
Using a potentially stale build of LLVM; \
This may not behave well.",
);
return Ok(res);
}

if stamp.is_done() {
if stamp.hash.is_none() {
builder.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ Therefore, you can build Rust with support for the target by adding it to the ta
```toml
[llvm]
download-ci-llvm = false
skip-rebuild = true
optimize = true
ninja = true
targets = "ARM;X86"
Expand Down

0 comments on commit daf99a4

Please sign in to comment.