From 2c141a454249ea0edbab2f4697eb634063e8b032 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:22:15 +1000 Subject: [PATCH 1/7] Update old comment referring to `libcompiler_builtins` --- library/profiler_builtins/build.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index c1e0e5c1c8975..b57f1187f8356 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -1,6 +1,8 @@ //! Compiles the profiler part of the `compiler-rt` library. //! -//! See the build.rs for libcompiler_builtins crate for details. +//! Loosely based on: +//! - LLVM's `compiler-rt/lib/profile/CMakeLists.txt` +//! - . use std::env; use std::path::PathBuf; From fcce75e9a666c60d106a04cbffc365c01364f1fd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 14:46:24 +1000 Subject: [PATCH 2/7] Use helper functions to read environment variables This also migrates from legacy `cargo:` directives to the newer `cargo::` prefix. --- library/profiler_builtins/build.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index b57f1187f8356..b9aaf512d5a17 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -8,9 +8,8 @@ use std::env; use std::path::PathBuf; fn main() { - println!("cargo:rerun-if-env-changed=LLVM_PROFILER_RT_LIB"); - if let Ok(rt) = env::var("LLVM_PROFILER_RT_LIB") { - println!("cargo:rustc-link-lib=static:+verbatim={rt}"); + if let Ok(rt) = tracked_env_var("LLVM_PROFILER_RT_LIB") { + println!("cargo::rustc-link-lib=static:+verbatim={rt}"); return; } @@ -82,12 +81,10 @@ fn main() { } // Get the LLVM `compiler-rt` directory from bootstrap. - println!("cargo:rerun-if-env-changed=RUST_COMPILER_RT_FOR_PROFILER"); - let root = PathBuf::from(env::var("RUST_COMPILER_RT_FOR_PROFILER").unwrap_or_else(|_| { - let path = "../../src/llvm-project/compiler-rt"; - println!("RUST_COMPILER_RT_FOR_PROFILER was not set; falling back to {path:?}"); - path.to_owned() - })); + let root = PathBuf::from(tracked_env_var_or_fallback( + "RUST_COMPILER_RT_FOR_PROFILER", + "../../src/llvm-project/compiler-rt", + )); let src_root = root.join("lib").join("profile"); assert!(src_root.exists(), "profiler runtime source directory not found: {src_root:?}"); @@ -105,3 +102,14 @@ fn main() { cfg.warnings(false); cfg.compile("profiler-rt"); } + +fn tracked_env_var(key: &str) -> Result { + println!("cargo::rerun-if-env-changed={key}"); + env::var(key) +} +fn tracked_env_var_or_fallback(key: &str, fallback: &str) -> String { + tracked_env_var(key).unwrap_or_else(|_| { + println!("cargo::warning={key} was not set; falling back to {fallback:?}"); + fallback.to_owned() + }) +} From b6dba995b4efddfae2b567f7bce8d5217a0e7477 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:04:10 +1000 Subject: [PATCH 3/7] Remove `InstrProfilingBiasVar.c` from the list of source files This file was introduced in LLVM 11, but was then removed in LLVM 13. --- library/profiler_builtins/build.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index b9aaf512d5a17..7dcbcc444ba4d 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -40,7 +40,6 @@ fn main() { "InstrProfilingWriter.c", // These files were added in LLVM 11. "InstrProfilingInternal.c", - "InstrProfilingBiasVar.c", ]; if target_env == "msvc" { From 25ca8a283f757b95929da385c4493b45e3012e78 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:12:51 +1000 Subject: [PATCH 4/7] Sort the list of source files --- library/profiler_builtins/build.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index 7dcbcc444ba4d..6089937e0b7a2 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -20,10 +20,12 @@ fn main() { // FIXME: `rerun-if-changed` directives are not currently emitted and the build script // will not rerun on changes in these source files or headers included into them. let mut profile_sources = vec![ + // tidy-alphabetical-start "GCDAProfiling.c", "InstrProfiling.c", "InstrProfilingBuffer.c", "InstrProfilingFile.c", + "InstrProfilingInternal.c", "InstrProfilingMerge.c", "InstrProfilingMergeFile.c", "InstrProfilingNameVar.c", @@ -38,8 +40,7 @@ fn main() { "InstrProfilingValue.c", "InstrProfilingVersionVar.c", "InstrProfilingWriter.c", - // These files were added in LLVM 11. - "InstrProfilingInternal.c", + // tidy-alphabetical-end ]; if target_env == "msvc" { From 842cda6865ca297135f53083bd013aa91abb5587 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:15:08 +1000 Subject: [PATCH 5/7] Always include `WindowsMMap.c` in the list of source files The whole file is surrounded by `#if defined(_WIN32)`, so there's no need to have separate logic to exclue it from non-Windows builds. --- library/profiler_builtins/build.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index 6089937e0b7a2..93048a6dec75c 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -19,7 +19,7 @@ fn main() { // FIXME: `rerun-if-changed` directives are not currently emitted and the build script // will not rerun on changes in these source files or headers included into them. - let mut profile_sources = vec![ + let profile_sources = vec![ // tidy-alphabetical-start "GCDAProfiling.c", "InstrProfiling.c", @@ -40,13 +40,13 @@ fn main() { "InstrProfilingValue.c", "InstrProfilingVersionVar.c", "InstrProfilingWriter.c", + "WindowsMMap.c", // tidy-alphabetical-end ]; if target_env == "msvc" { // Don't pull in extra libraries on MSVC cfg.flag("/Zl"); - profile_sources.push("WindowsMMap.c"); cfg.define("strdup", Some("_strdup")); cfg.define("open", Some("_open")); cfg.define("fdopen", Some("_fdopen")); @@ -61,8 +61,6 @@ fn main() { if target_os != "windows" { cfg.flag("-fvisibility=hidden"); cfg.define("COMPILER_RT_HAS_UNAME", Some("1")); - } else { - profile_sources.push("WindowsMMap.c"); } } From 7f90aa5538454c226b664ed882729e5f24251bd4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:38:26 +1000 Subject: [PATCH 6/7] Add `cargo::rerun-if-changed` directives for source directories --- library/profiler_builtins/build.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index 93048a6dec75c..bf4a1749322de 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -17,8 +17,6 @@ fn main() { let target_env = env::var("CARGO_CFG_TARGET_ENV").expect("CARGO_CFG_TARGET_ENV was not set"); let cfg = &mut cc::Build::new(); - // FIXME: `rerun-if-changed` directives are not currently emitted and the build script - // will not rerun on changes in these source files or headers included into them. let profile_sources = vec![ // tidy-alphabetical-start "GCDAProfiling.c", @@ -86,6 +84,7 @@ fn main() { let src_root = root.join("lib").join("profile"); assert!(src_root.exists(), "profiler runtime source directory not found: {src_root:?}"); + println!("cargo::rerun-if-changed={}", src_root.display()); let mut n_sources_found = 0u32; for src in profile_sources { let path = src_root.join(src); @@ -96,7 +95,10 @@ fn main() { } assert!(n_sources_found > 0, "couldn't find any profiler runtime source files in {src_root:?}"); - cfg.include(root.join("include")); + let include = root.join("include"); + println!("cargo::rerun-if-changed={}", include.display()); + cfg.include(include); + cfg.warnings(false); cfg.compile("profiler-rt"); } From 5b6ff4fe18e4294ac2180325cdb46494a051706c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 27 Aug 2024 15:41:46 +1000 Subject: [PATCH 7/7] Don't skip nonexistent source files This behaviour was introduced during the upgrade to LLVM 11. Now that the list of source files has been cleaned up, we can reasonably expect _all_ of the listed source files to be present. --- library/profiler_builtins/build.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/library/profiler_builtins/build.rs b/library/profiler_builtins/build.rs index bf4a1749322de..dd85239fa8cfd 100644 --- a/library/profiler_builtins/build.rs +++ b/library/profiler_builtins/build.rs @@ -85,15 +85,9 @@ fn main() { let src_root = root.join("lib").join("profile"); assert!(src_root.exists(), "profiler runtime source directory not found: {src_root:?}"); println!("cargo::rerun-if-changed={}", src_root.display()); - let mut n_sources_found = 0u32; - for src in profile_sources { - let path = src_root.join(src); - if path.exists() { - cfg.file(path); - n_sources_found += 1; - } + for file in profile_sources { + cfg.file(src_root.join(file)); } - assert!(n_sources_found > 0, "couldn't find any profiler runtime source files in {src_root:?}"); let include = root.join("include"); println!("cargo::rerun-if-changed={}", include.display());