Skip to content

Don't rebuild LLVM for BOLT optimization #107521

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 0 additions & 71 deletions src/bootstrap/bolt.rs

This file was deleted.

11 changes: 0 additions & 11 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,6 @@ pub struct Config {
pub llvm_profile_use: Option<String>,
pub llvm_profile_generate: bool,
pub llvm_libunwind_default: Option<LlvmLibunwind>,
pub llvm_bolt_profile_generate: bool,
pub llvm_bolt_profile_use: Option<String>,

pub build: TargetSelection,
pub hosts: Vec<TargetSelection>,
Expand Down Expand Up @@ -869,15 +867,6 @@ impl Config {
}
config.llvm_profile_use = flags.llvm_profile_use;
config.llvm_profile_generate = flags.llvm_profile_generate;
config.llvm_bolt_profile_generate = flags.llvm_bolt_profile_generate;
config.llvm_bolt_profile_use = flags.llvm_bolt_profile_use;

if config.llvm_bolt_profile_generate && config.llvm_bolt_profile_use.is_some() {
eprintln!(
"Cannot use both `llvm_bolt_profile_generate` and `llvm_bolt_profile_use` at the same time"
);
crate::detail_exit(1);
}

// Infer the rest of the configuration.

Expand Down
4 changes: 0 additions & 4 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2220,10 +2220,6 @@ impl Step for ReproducibleArtifacts {
tarball.add_file(path, ".", 0o644);
added_anything = true;
}
if let Some(path) = builder.config.llvm_bolt_profile_use.as_ref() {
Copy link
Contributor

@Kobzol Kobzol Feb 1, 2023

Choose a reason for hiding this comment

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

Another disadvantage of putting BOLT into Python is that we lose reproducibility of the profiles here :( I wonder if we could add a bootstrap step on top of LLVM that would create the BOLTed libraries in another place (in order not to interfere with the regular LLVM build step, and to sidestep its cache), and thus still keep BOLT as a first class citizen, with proper access to LLVM library paths, and with storage of profiles, while still avoiding the extra LLVM (re)build.

tarball.add_file(path, ".", 0o644);
added_anything = true;
}
if added_anything { Some(tarball.generate()) } else { None }
}
}
6 changes: 0 additions & 6 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ pub struct Flags {
//
// llvm_out/build/profiles/ is the location this writes to.
pub llvm_profile_generate: bool,
pub llvm_bolt_profile_generate: bool,
pub llvm_bolt_profile_use: Option<String>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -259,8 +257,6 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
opts.optmulti("D", "", "deny certain clippy lints", "OPT");
opts.optmulti("W", "", "warn about certain clippy lints", "OPT");
opts.optmulti("F", "", "forbid certain clippy lints", "OPT");
opts.optflag("", "llvm-bolt-profile-generate", "generate BOLT profile for LLVM build");
opts.optopt("", "llvm-bolt-profile-use", "use BOLT profile for LLVM build", "PROFILE");

// We can't use getopt to parse the options until we have completed specifying which
// options are valid, but under the current implementation, some options are conditional on
Expand Down Expand Up @@ -704,8 +700,6 @@ Arguments:
rust_profile_generate: matches.opt_str("rust-profile-generate"),
llvm_profile_use: matches.opt_str("llvm-profile-use"),
llvm_profile_generate: matches.opt_present("llvm-profile-generate"),
llvm_bolt_profile_generate: matches.opt_present("llvm-bolt-profile-generate"),
llvm_bolt_profile_use: matches.opt_str("llvm-bolt-profile-use"),
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ use crate::util::{
exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed,
};

mod bolt;
mod builder;
mod cache;
mod cc_detect;
Expand Down
29 changes: 0 additions & 29 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::io;
use std::path::{Path, PathBuf};
use std::process::Command;

use crate::bolt::{instrument_with_bolt_inplace, optimize_library_with_bolt_inplace};
use crate::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::channel;
use crate::config::{Config, TargetSelection};
Expand Down Expand Up @@ -345,12 +344,6 @@ impl Step for Llvm {
if let Some(path) = builder.config.llvm_profile_use.as_ref() {
cfg.define("LLVM_PROFDATA_FILE", &path);
}
if builder.config.llvm_bolt_profile_generate
|| builder.config.llvm_bolt_profile_use.is_some()
{
// Relocations are required for BOLT to work.
ldflags.push_all("-Wl,-q");
}

// Disable zstd to avoid a dependency on libzstd.so.
cfg.define("LLVM_ENABLE_ZSTD", "OFF");
Expand Down Expand Up @@ -520,34 +513,12 @@ impl Step for Llvm {
}
}

// After LLVM is built, we modify (instrument or optimize) the libLLVM.so library file
// in place. This is fine, because currently we do not support incrementally rebuilding
// LLVM after a configuration change, so to rebuild it the build files have to be removed,
// which will also remove these modified files.
if builder.config.llvm_bolt_profile_generate {
instrument_with_bolt_inplace(&get_built_llvm_lib_path(&res.llvm_config));
}
if let Some(path) = &builder.config.llvm_bolt_profile_use {
optimize_library_with_bolt_inplace(
&get_built_llvm_lib_path(&res.llvm_config),
&Path::new(path),
);
}

t!(stamp.write());

res
}
}

/// Returns path to a built LLVM library (libLLVM.so).
/// Assumes that we have built LLVM into a single library file.
fn get_built_llvm_lib_path(llvm_config_path: &Path) -> PathBuf {
let mut cmd = Command::new(llvm_config_path);
cmd.arg("--libfiles");
PathBuf::from(output(&mut cmd).trim())
}

fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) {
if !builder.config.llvm_version_check {
return;
Expand Down
Loading