Skip to content

Commit

Permalink
Bootstrap: Don't duplicate cc-rs flags
Browse files Browse the repository at this point in the history
Commands that end up invoking cc-rs, i.e. Cargo (through build scripts)
and cmake-rs don't need the CFLAGS from cc-rs itself, as they will just
end up as duplicates.

We do still choose to pass them in certain places, but now it's at least
clear which flags are default, and which flags are extra flags added on.
  • Loading branch information
madsmtm committed Jan 31, 2025
1 parent f85c6de commit e790b92
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 17 deletions.
3 changes: 2 additions & 1 deletion src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,8 @@ pub fn compiler_file(
return PathBuf::new();
}
let mut cmd = command(compiler);
cmd.args(builder.cflags(target, GitRepo::Rustc, c));
cmd.args(builder.default_cflags(target, c));
cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c));
cmd.arg(format!("-print-file-name={file}"));
let out = cmd.run_capture_stdout(builder).stdout();
PathBuf::from(out.trim())
Expand Down
11 changes: 9 additions & 2 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,9 +757,15 @@ fn configure_cmake(
}

cfg.build_arg("-j").build_arg(builder.jobs().to_string());
// FIXME(madsmtm): Allow `cmake-rs` to select flags by itself by passing
// our flags via `.cflag`/`.cxxflag` instead.
//
// Needs `suppressed_compiler_flag_prefixes` to be gone, and hence
// https://github.com/llvm/llvm-project/issues/88780 to be fixed.
let mut cflags: OsString = builder
.cflags(target, GitRepo::Llvm, CLang::C)
.default_cflags(target, CLang::C)
.into_iter()
.chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::C))
.filter(|flag| {
!suppressed_compiler_flag_prefixes
.iter()
Expand All @@ -778,8 +784,9 @@ fn configure_cmake(
}
cfg.define("CMAKE_C_FLAGS", cflags);
let mut cxxflags: OsString = builder
.cflags(target, GitRepo::Llvm, CLang::Cxx)
.default_cflags(target, CLang::Cxx)
.into_iter()
.chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx))
.filter(|flag| {
!suppressed_compiler_flag_prefixes
.iter()
Expand Down
8 changes: 6 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2005,14 +2005,18 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// Only pass correct values for these flags for the `run-make` suite as it
// requires that a C++ compiler was configured which isn't always the case.
if !builder.config.dry_run() && mode == "run-make" {
let mut cflags = builder.default_cflags(target, CLang::C);
cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C));
let mut cxxflags = builder.default_cflags(target, CLang::Cxx);
cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx));
cmd.arg("--cc")
.arg(builder.cc(target))
.arg("--cxx")
.arg(builder.cxx(target).unwrap())
.arg("--cflags")
.arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "))
.arg(cflags.join(" "))
.arg("--cxxflags")
.arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "));
.arg(cxxflags.join(" "));
copts_passed = true;
if let Some(ar) = builder.ar(target) {
cmd.arg("--ar").arg(ar);
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ impl Cargo {
let cc = ccacheify(&builder.cc(target));
self.command.env(format!("CC_{triple_underscored}"), &cc);

let cflags = builder.cflags(target, GitRepo::Rustc, CLang::C).join(" ");
let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" ");
self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags);

if let Some(ar) = builder.ar(target) {
Expand All @@ -329,7 +329,7 @@ impl Cargo {

if let Ok(cxx) = builder.cxx(target) {
let cxx = ccacheify(&cxx);
let cxxflags = builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ");
let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ");
self.command
.env(format!("CXX_{triple_underscored}"), &cxx)
.env(format!("CXXFLAGS_{triple_underscored}"), cxxflags);
Expand Down
21 changes: 13 additions & 8 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ impl Mode {
}
}

#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum CLang {
C,
Cxx,
Expand Down Expand Up @@ -1139,9 +1140,9 @@ Executed at: {executed_at}"#,
self.cc.borrow()[&target].path().into()
}

/// Returns a list of flags to pass to the C compiler for the target
/// specified.
fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
/// Returns C flags that `cc-rs` thinks should be enabled for the
/// specified target by default.
fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec<String> {
if self.config.dry_run() {
return Vec::new();
}
Expand All @@ -1150,14 +1151,18 @@ Executed at: {executed_at}"#,
CLang::Cxx => self.cxx.borrow()[&target].clone(),
};

// Filter out -O and /O (the optimization flags) that we picked up from
// cc-rs because the build scripts will determine that for themselves.
let mut base = base
.args()
// Filter out -O and /O (the optimization flags) that we picked up
// from cc-rs, that's up to the caller to figure out.
base.args()
.iter()
.map(|s| s.to_string_lossy().into_owned())
.filter(|s| !s.starts_with("-O") && !s.starts_with("/O"))
.collect::<Vec<String>>();
.collect::<Vec<String>>()
}

/// Returns extra C flags that `cc-rs` doesn't handle.
fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
let mut base = Vec::new();

// If we're compiling C++ on macOS then we add a flag indicating that
// we want libc++ (more filled out than libstdc++), ensuring that
Expand Down
6 changes: 4 additions & 2 deletions src/bootstrap/src/utils/cc_detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ pub fn find_target(build: &Build, target: TargetSelection) {
};

build.cc.borrow_mut().insert(target, compiler.clone());
let cflags = build.cflags(target, GitRepo::Rustc, CLang::C);
let mut cflags = build.default_cflags(target, CLang::C);
cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C));

// If we use llvm-libunwind, we will need a C++ compiler as well for all targets
// We'll need one anyways if the target triple is also a host triple
Expand All @@ -168,7 +169,8 @@ pub fn find_target(build: &Build, target: TargetSelection) {
build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target)));
build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple));
if let Ok(cxx) = build.cxx(target) {
let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx);
let mut cxxflags = build.default_cflags(target, CLang::Cxx);
cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx));
build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple));
build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple));
}
Expand Down

0 comments on commit e790b92

Please sign in to comment.