Skip to content

Commit

Permalink
Rollup merge of rust-lang#137080 - jieyouxu:more-tracing, r=onur-ozkan
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
GuillaumeGomez authored Feb 16, 2025
2 parents 54fa2f8 + 05ba1a4 commit 51c6826
Show file tree
Hide file tree
Showing 11 changed files with 340 additions and 31 deletions.
192 changes: 181 additions & 11 deletions src/bootstrap/src/core/build_steps/compile.rs

Large diffs are not rendered by default.

47 changes: 43 additions & 4 deletions src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::{env, fs};

use object::BinaryFormat;
use object::read::archive::ArchiveFile;
#[cfg(feature = "tracing")]
use tracing::instrument;

use crate::core::build_steps::doc::DocumentationFormat;
use crate::core::build_steps::tool::{self, Tool};
Expand All @@ -30,7 +32,7 @@ use crate::utils::helpers::{
exe, is_dylib, move_file, t, target_supports_cranelift_backend, timeit,
};
use crate::utils::tarball::{GeneratedTarball, OverlayKind, Tarball};
use crate::{Compiler, DependencyType, LLVM_TOOLS, Mode};
use crate::{Compiler, DependencyType, LLVM_TOOLS, Mode, trace};

pub fn pkgname(builder: &Builder<'_>, component: &str) -> String {
format!("{}-{}", component, builder.rust_package_vers())
Expand Down Expand Up @@ -582,7 +584,7 @@ impl Step for DebuggerScripts {
fn skip_host_target_lib(builder: &Builder<'_>, compiler: Compiler) -> bool {
// The only true set of target libraries came from the build triple, so
// let's reduce redundant work by only producing archives from that host.
if !builder.is_builder_target(&compiler.host) {
if !builder.is_builder_target(compiler.host) {
builder.info("\tskipping, not a build host");
true
} else {
Expand Down Expand Up @@ -637,7 +639,7 @@ fn copy_target_libs(
for (path, dependency_type) in builder.read_stamp_file(stamp) {
if dependency_type == DependencyType::TargetSelfContained {
builder.copy_link(&path, &self_contained_dst.join(path.file_name().unwrap()));
} else if dependency_type == DependencyType::Target || builder.is_builder_target(&target) {
} else if dependency_type == DependencyType::Target || builder.is_builder_target(target) {
builder.copy_link(&path, &dst.join(path.file_name().unwrap()));
}
}
Expand Down Expand Up @@ -786,7 +788,7 @@ impl Step for Analysis {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
if !builder.is_builder_target(&compiler.host) {
if !builder.is_builder_target(compiler.host) {
return None;
}

Expand Down Expand Up @@ -2029,6 +2031,15 @@ fn install_llvm_file(
/// Maybe add LLVM object files to the given destination lib-dir. Allows either static or dynamic linking.
///
/// Returns whether the files were actually copied.
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "maybe_install_llvm",
skip_all,
fields(target = ?target, dst_libdir = ?dst_libdir, install_symlink = install_symlink),
),
)]
fn maybe_install_llvm(
builder: &Builder<'_>,
target: TargetSelection,
Expand All @@ -2052,6 +2063,7 @@ fn maybe_install_llvm(
// If the LLVM is coming from ourselves (just from CI) though, we
// still want to install it, as it otherwise won't be available.
if builder.is_system_llvm(target) {
trace!("system LLVM requested, no install");
return false;
}

Expand All @@ -2070,6 +2082,7 @@ fn maybe_install_llvm(
} else if let llvm::LlvmBuildStatus::AlreadyBuilt(llvm::LlvmResult { llvm_config, .. }) =
llvm::prebuilt_llvm_config(builder, target, true)
{
trace!("LLVM already built, installing LLVM files");
let mut cmd = command(llvm_config);
cmd.arg("--libfiles");
builder.verbose(|| println!("running {cmd:?}"));
Expand All @@ -2092,6 +2105,19 @@ fn maybe_install_llvm(
}

/// Maybe add libLLVM.so to the target lib-dir for linking.
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "maybe_install_llvm_target",
skip_all,
fields(
llvm_link_shared = ?builder.llvm_link_shared(),
target = ?target,
sysroot = ?sysroot,
),
),
)]
pub fn maybe_install_llvm_target(builder: &Builder<'_>, target: TargetSelection, sysroot: &Path) {
let dst_libdir = sysroot.join("lib/rustlib").join(target).join("lib");
// We do not need to copy LLVM files into the sysroot if it is not
Expand All @@ -2103,6 +2129,19 @@ pub fn maybe_install_llvm_target(builder: &Builder<'_>, target: TargetSelection,
}

/// Maybe add libLLVM.so to the runtime lib-dir for rustc itself.
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "maybe_install_llvm_runtime",
skip_all,
fields(
llvm_link_shared = ?builder.llvm_link_shared(),
target = ?target,
sysroot = ?sysroot,
),
),
)]
pub fn maybe_install_llvm_runtime(builder: &Builder<'_>, target: TargetSelection, sysroot: &Path) {
let dst_libdir =
sysroot.join(builder.sysroot_libdir_relative(Compiler { stage: 1, host: target }));
Expand Down
23 changes: 19 additions & 4 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::{env, fs};

use build_helper::ci::CiEnv;
use build_helper::git::get_closest_merge_commit;
#[cfg(feature = "tracing")]
use tracing::instrument;

use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::core::config::{Config, TargetSelection};
Expand All @@ -24,7 +26,7 @@ use crate::utils::exec::command;
use crate::utils::helpers::{
self, exe, get_clang_cl_resource_dir, t, unhashed_basename, up_to_date,
};
use crate::{CLang, GitRepo, Kind};
use crate::{CLang, GitRepo, Kind, trace};

#[derive(Clone)]
pub struct LlvmResult {
Expand Down Expand Up @@ -516,7 +518,7 @@ impl Step for Llvm {
}

// https://llvm.org/docs/HowToCrossCompileLLVM.html
if !builder.is_builder_target(&target) {
if !builder.is_builder_target(target) {
let LlvmResult { llvm_config, .. } =
builder.ensure(Llvm { target: builder.config.build });
if !builder.config.dry_run() {
Expand Down Expand Up @@ -668,7 +670,7 @@ fn configure_cmake(
}
cfg.target(&target.triple).host(&builder.config.build.triple);

if !builder.is_builder_target(&target) {
if !builder.is_builder_target(target) {
cfg.define("CMAKE_CROSSCOMPILING", "True");

if target.contains("netbsd") {
Expand Down Expand Up @@ -934,6 +936,15 @@ impl Step for Enzyme {
}

/// Compile Enzyme for `target`.
#[cfg_attr(
feature = "tracing",
instrument(
level = "debug",
name = "Enzyme::run",
skip_all,
fields(target = ?self.target),
),
)]
fn run(self, builder: &Builder<'_>) -> PathBuf {
builder.require_submodule(
"src/tools/enzyme",
Expand All @@ -959,7 +970,9 @@ impl Step for Enzyme {
let out_dir = builder.enzyme_out(target);
let stamp = BuildStamp::new(&out_dir).with_prefix("enzyme").add_stamp(smart_stamp_hash);

trace!("checking build stamp to see if we need to rebuild enzyme artifacts");
if stamp.is_up_to_date() {
trace!(?out_dir, "enzyme build artifacts are up to date");
if stamp.stamp().is_empty() {
builder.info(
"Could not determine the Enzyme submodule commit hash. \
Expand All @@ -973,6 +986,7 @@ impl Step for Enzyme {
return out_dir;
}

trace!(?target, "(re)building enzyme artifacts");
builder.info(&format!("Building Enzyme for {}", target));
t!(stamp.remove());
let _time = helpers::timeit(builder);
Expand All @@ -994,6 +1008,7 @@ impl Step for Enzyme {
(true, false) => "Release",
(true, true) => "RelWithDebInfo",
};
trace!(?profile);

cfg.out_dir(&out_dir)
.profile(profile)
Expand Down Expand Up @@ -1118,7 +1133,7 @@ impl Step for Lld {
.define("LLVM_CMAKE_DIR", llvm_cmake_dir)
.define("LLVM_INCLUDE_TESTS", "OFF");

if !builder.is_builder_target(&target) {
if !builder.is_builder_target(target) {
// Use the host llvm-tblgen binary.
cfg.define(
"LLVM_TABLEGEN_EXE",
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2743,7 +2743,7 @@ impl Step for Crate {
cargo
} else {
// Also prepare a sysroot for the target.
if !builder.is_builder_target(&target) {
if !builder.is_builder_target(target) {
builder.ensure(compile::Std::new(compiler, target).force_recompile(true));
builder.ensure(RemoteCopyLibs { compiler, target });
}
Expand Down
24 changes: 24 additions & 0 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::path::PathBuf;
use std::{env, fs};

#[cfg(feature = "tracing")]
use tracing::instrument;

use crate::core::build_steps::compile::is_lto_stage;
use crate::core::build_steps::toolstate::ToolState;
use crate::core::build_steps::{compile, llvm};
Expand Down Expand Up @@ -304,6 +307,14 @@ macro_rules! bootstrap_tool {
});
}

#[cfg_attr(
feature = "tracing",
instrument(
level = "debug",
name = $tool_name,
skip_all,
),
)]
fn run(self, builder: &Builder<'_>) -> PathBuf {
$(
for submodule in $submodules {
Expand Down Expand Up @@ -758,6 +769,15 @@ impl Step for LldWrapper {
run.never()
}

#[cfg_attr(
feature = "tracing",
instrument(
level = "debug",
name = "LldWrapper::run",
skip_all,
fields(build_compiler = ?self.build_compiler, target_compiler = ?self.target_compiler),
),
)]
fn run(self, builder: &Builder<'_>) {
if builder.config.dry_run() {
return;
Expand Down Expand Up @@ -914,6 +934,10 @@ impl Step for LlvmBitcodeLinker {
});
}

#[cfg_attr(
feature = "tracing",
instrument(level = "debug", name = "LlvmBitcodeLinker::run", skip_all)
)]
fn run(self, builder: &Builder<'_>) -> PathBuf {
let bin_name = "llvm-bitcode-linker";

Expand Down
41 changes: 38 additions & 3 deletions src/bootstrap/src/core/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use std::time::{Duration, Instant};
use std::{env, fs};

use clap::ValueEnum;
#[cfg(feature = "tracing")]
use tracing::instrument;

pub use self::cargo::{Cargo, cargo_profile_var};
pub use crate::Compiler;
Expand All @@ -21,7 +23,7 @@ use crate::core::config::{DryRun, TargetSelection};
use crate::utils::cache::Cache;
use crate::utils::exec::{BootstrapCommand, command};
use crate::utils::helpers::{self, LldThreads, add_dylib_path, exe, libdir, linker_args, t};
use crate::{Build, Crate};
use crate::{Build, Crate, trace};

mod cargo;

Expand Down Expand Up @@ -1214,6 +1216,19 @@ impl<'a> Builder<'a> {
/// compiler will run on, *not* the target it will build code for). Explicitly does not take
/// `Compiler` since all `Compiler` instances are meant to be obtained through this function,
/// since it ensures that they are valid (i.e., built and assembled).
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "Builder::compiler",
target = "COMPILER",
skip_all,
fields(
stage = stage,
host = ?host,
),
),
)]
pub fn compiler(&self, stage: u32, host: TargetSelection) -> Compiler {
self.ensure(compile::Assemble { target_compiler: Compiler { stage, host } })
}
Expand All @@ -1229,19 +1244,39 @@ impl<'a> Builder<'a> {
/// sysroot.
///
/// See `force_use_stage1` and `force_use_stage2` for documentation on what each argument is.
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "Builder::compiler_for",
target = "COMPILER_FOR",
skip_all,
fields(
stage = stage,
host = ?host,
target = ?target,
),
),
)]
pub fn compiler_for(
&self,
stage: u32,
host: TargetSelection,
target: TargetSelection,
) -> Compiler {
if self.build.force_use_stage2(stage) {
#![allow(clippy::let_and_return)]
let resolved_compiler = if self.build.force_use_stage2(stage) {
trace!(target: "COMPILER_FOR", ?stage, "force_use_stage2");
self.compiler(2, self.config.build)
} else if self.build.force_use_stage1(stage, target) {
trace!(target: "COMPILER_FOR", ?stage, "force_use_stage1");
self.compiler(1, self.config.build)
} else {
trace!(target: "COMPILER_FOR", ?stage, ?host, "no force, fallback to `compiler()`");
self.compiler(stage, host)
}
};
trace!(target: "COMPILER_FOR", ?resolved_compiler);
resolved_compiler
}

pub fn sysroot(&self, compiler: Compiler) -> PathBuf {
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ fn test_is_builder_target() {
let build = Build::new(config);
let builder = Builder::new(&build);

assert!(builder.is_builder_target(&target1));
assert!(!builder.is_builder_target(&target2));
assert!(builder.is_builder_target(target1));
assert!(!builder.is_builder_target(target2));
}
}
9 changes: 9 additions & 0 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2747,6 +2747,15 @@ impl Config {
/// tarball). Typically [`crate::Build::require_submodule`] should be
/// used instead to provide a nice error to the user if the submodule is
/// missing.
#[cfg_attr(
feature = "tracing",
instrument(
level = "trace",
name = "Config::update_submodule",
skip_all,
fields(relative_path = ?relative_path),
),
)]
pub(crate) fn update_submodule(&self, relative_path: &str) {
if !self.submodules() {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ than building it.
if target.contains("musl") && !target.contains("unikraft") {
// If this is a native target (host is also musl) and no musl-root is given,
// fall back to the system toolchain in /usr before giving up
if build.musl_root(*target).is_none() && build.is_builder_target(target) {
if build.musl_root(*target).is_none() && build.is_builder_target(*target) {
let target = build.config.target_config.entry(*target).or_default();
target.musl_root = Some("/usr".into());
}
Expand Down
Loading

0 comments on commit 51c6826

Please sign in to comment.