From 2b24d24d78a3f0a9ac99673daa5f0b9f3b911b0d Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 15 Apr 2023 17:22:08 +0300 Subject: [PATCH 1/2] spirv-builder: clean up the Cargo args/env var setup order. --- crates/spirv-builder/src/lib.rs | 62 +++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index b76a05b2f1..38561261d1 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -510,27 +510,12 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { rustflags.extend(extra_rustflags.split_whitespace().map(|s| s.to_string())); } - let mut cargo = Command::new("cargo"); - cargo.args([ - "build", - "--lib", - "--message-format=json-render-diagnostics", - "-Zbuild-std=core", - "-Zbuild-std-features=compiler-builtins-mem", - "--target", - &*builder.target, - ]); - - if builder.release { - cargo.arg("--release"); - } - // If we're nested in `cargo` invocation, use a different `--target-dir`, // to avoid waiting on the same lock (which effectively dead-locks us). let outer_target_dir = match (env::var("PROFILE"), env::var_os("OUT_DIR")) { - (Ok(profile), Some(dir)) => { - // Strip `$profile/build/*/out`. - [&profile, "build", "*", "out"].iter().rev().try_fold( + (Ok(outer_profile), Some(dir)) => { + // Strip `$outer_profile/build/*/out`. + [&outer_profile, "build", "*", "out"].iter().rev().try_fold( PathBuf::from(dir), |mut dir, &filter| { if (filter == "*" || dir.ends_with(filter)) && dir.pop() { @@ -546,16 +531,30 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // FIXME(eddyb) use `crate metadata` to always be able to get the "outer" // (or "default") `--target-dir`, to append `/spirv-builder` to it. let target_dir = outer_target_dir.map(|outer| outer.join("spirv-builder")); + + let profile = if builder.release { "release" } else { "dev" }; + + let mut cargo = Command::new("cargo"); + cargo.args([ + "build", + "--lib", + "--message-format=json-render-diagnostics", + "-Zbuild-std=core", + "-Zbuild-std-features=compiler-builtins-mem", + "--profile", + profile, + "--target", + &*builder.target, + ]); + + // NOTE(eddyb) see above how this is computed and why it might be missing. if let Some(target_dir) = target_dir { cargo.arg("--target-dir").arg(target_dir); } - // NOTE(eddyb) Cargo caches some information it got from `rustc` in - // `.rustc_info.json`, and assumes it only depends on the `rustc` binary, - // but in our case, `rustc_codegen_spirv` changes are also relevant, - // so we turn off that caching with an env var, just to avoid any issues. - cargo.env("CARGO_CACHE_RUSTC_INFO", "0"); - + // Clear Cargo environment variables that we don't want to leak into the + // inner invocation of Cargo (because e.g. build scripts might read them), + // before we set any of our own below. for (key, _) in env::vars_os() { let remove = key.to_str().map_or(false, |s| { s.starts_with("CARGO_FEATURES_") || s.starts_with("CARGO_CFG_") @@ -565,12 +564,23 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { } } - let cargo_encoded_rustflags = join_checking_for_separators(rustflags, "\x1f"); + // NOTE(eddyb) Cargo caches some information it got from `rustc` in + // `.rustc_info.json`, and assumes it only depends on the `rustc` binary, + // but in our case, `rustc_codegen_spirv` changes are also relevant, + // so we turn off that caching with an env var, just to avoid any issues. + cargo.env("CARGO_CACHE_RUSTC_INFO", "0"); + + // NOTE(eddyb) this used to be just `RUSTFLAGS` but at some point Cargo + // added a separate environment variable using `\x1f` instead of spaces, + // which allows us to have spaces within individual `rustc` flags. + cargo.env( + "CARGO_ENCODED_RUSTFLAGS", + join_checking_for_separators(rustflags, "\x1f"), + ); let build = cargo .stderr(Stdio::inherit()) .current_dir(&builder.path_to_crate) - .env("CARGO_ENCODED_RUSTFLAGS", cargo_encoded_rustflags) .output() .expect("failed to execute cargo build"); From 7a482b06da77fc6adab308be6d7ef9e9fbeb3095 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 15 Apr 2023 18:28:32 +0300 Subject: [PATCH 2/2] spirv-builder: force a single CGU (codegen-unit). --- CHANGELOG.md | 5 ++++- crates/spirv-builder/src/lib.rs | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97feb4454a..dc37d7c34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,8 +29,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added ⭐ +- [PR#1031](https://github.com/EmbarkStudios/rust-gpu/pull/1031) added `Components` generic parameter to `Image` type, allowing images to return lower dimensional vectors and even scalars from the sampling API + ### Changed 🛠 -- [PR#1031](https://github.com/EmbarkStudios/rust-gpu/pull/1031) Added `Components` generic parameter to `Image` type, allowing images to return lower dimensional vectors and even scalars from the sampling API. +- [PR#1035](https://github.com/EmbarkStudios/rust-gpu/pull/1035) reduced the number of CGUs ("codegen units") used by `spirv-builder` to just `1` - [PR#1011](https://github.com/EmbarkStudios/rust-gpu/pull/1011) made `NonWritable` all read-only storage buffers (i.e. those typed `&T`, where `T` doesn't have interior mutability) - [PR#1029](https://github.com/EmbarkStudios/rust-gpu/pull/1029) fixed SampledImage::sample() fns being unnecessarily marked as unsafe diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 38561261d1..1da5627e5a 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -578,6 +578,16 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { join_checking_for_separators(rustflags, "\x1f"), ); + let profile_in_env_var = profile.replace('-', "_").to_ascii_uppercase(); + + // NOTE(eddyb) there's no parallelism to take advantage of multiple CGUs, + // and inter-CGU duplication can be wasteful, so this forces 1 CGU for now. + let num_cgus = 1; + cargo.env( + format!("CARGO_PROFILE_{profile_in_env_var}_CODEGEN_UNITS"), + num_cgus.to_string(), + ); + let build = cargo .stderr(Stdio::inherit()) .current_dir(&builder.path_to_crate)