Skip to content
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

bootstrap: add more tracing to compiler/std/llvm flows #137080

Merged
merged 3 commits into from
Feb 17, 2025
Merged
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
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
Loading