From 30c65028fcfb3d230e85c27df8d21a284d0b6650 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Sun, 27 Apr 2025 12:52:52 +0200 Subject: [PATCH 01/26] compile_codegen: add compile_codegen feature --- crates/spirv-builder/Cargo.toml | 9 ++++++--- crates/spirv-builder/src/lib.rs | 25 ------------------------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index 125bcbe72de..bf7585efc60 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -18,10 +18,13 @@ no-default-features = true # that optional dependency, from being automatically created by Cargo, see: # https://doc.rust-lang.org/cargo/reference/features.html#optional-dependencies [features] -# See `rustc_codegen_spirv/Cargo.toml` for details on these features. default = ["use-compiled-tools"] -use-installed-tools = ["dep:rustc_codegen_spirv", "rustc_codegen_spirv?/use-installed-tools"] -use-compiled-tools = ["dep:rustc_codegen_spirv", "rustc_codegen_spirv?/use-compiled-tools"] +# Compile `rustc_codegen_spirv`, allows constructing SpirvBuilder without +# explicitly passing in a path to a compiled `rustc_codegen_spirv.so` (or dll) +compile_codegen = ["dep:rustc_codegen_spirv"] +# See `rustc_codegen_spirv/Cargo.toml` for details on these features. +use-installed-tools = ["compile_codegen", "rustc_codegen_spirv?/use-installed-tools"] +use-compiled-tools = ["compile_codegen", "rustc_codegen_spirv?/use-compiled-tools"] skip-toolchain-check = ["rustc_codegen_spirv?/skip-toolchain-check"] watch = ["dep:notify"] diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 488f71508ef..c7116de7780 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -72,31 +72,6 @@ // #![allow()] #![doc = include_str!("../README.md")] -// HACK(eddyb) try to catch misuse of Cargo package features very early on -// (see `spirv-builder/Cargo.toml` for why we go through all of this). -#[cfg(all( - not(any(feature = "use-compiled-tools", feature = "use-installed-tools")), - not(doc) -))] -compile_error!( - "at least one of `use-compiled-tools` or `use-installed-tools` features must be enabled -(outside of documentation builds, which require disabling both to build on stable)" -); - -#[cfg(doc)] -fn _ensure_cfg_doc_means_rustdoc() { - // HACK(eddyb) this relies on specific `rustdoc` behavior (i.e. it skips - // type-checking function bodies, so we trigger a compile-time `panic! from - // a type) to check that we're in fact under `rustdoc`, not just `--cfg doc`. - #[rustfmt::skip] - let _: [(); panic!(" - - `--cfg doc` was set outside of `rustdoc` - (if you are running `rustdoc` or `cargo doc`, please file an issue) - -")]; -} - mod depfile; #[cfg(feature = "watch")] mod watch; From a30c580f10354336da2ce9daf05c6748c67411fa Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Sun, 27 Apr 2025 12:59:10 +0200 Subject: [PATCH 02/26] compile_codegen: panic when `rustc_codegen_spirv_location` is not set --- crates/spirv-builder/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index c7116de7780..e5fe749e74d 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -606,6 +606,7 @@ fn dylib_path() -> Vec { } } +#[cfg(feature = "compile_codegen")] fn find_rustc_codegen_spirv() -> PathBuf { let filename = format!( "{}rustc_codegen_spirv{}", @@ -621,6 +622,13 @@ fn find_rustc_codegen_spirv() -> PathBuf { panic!("Could not find {filename} in library path"); } +#[cfg(not(feature = "compile_codegen"))] +fn find_rustc_codegen_spirv() -> PathBuf { + panic!( + "Without feature `compile_codegen`, you need to set the path of the codegen dylib using `rustc_codegen_spirv_location(...)`" + ); +} + /// Joins strings together while ensuring none of the strings contain the separator. // NOTE(eddyb) this intentionally consumes the `Vec` to limit accidental misuse. fn join_checking_for_separators(strings: Vec>, sep: &str) -> String { From dd056a3a678111594997f89aa2b58262b452fef0 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Sun, 27 Apr 2025 14:01:46 +0200 Subject: [PATCH 03/26] gitignore: .idea --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 410633c8d72..07ba091a65e 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ target/ tests/Cargo.lock .github/install-spirv-tools/Cargo.lock rustc-ice-*.txt +.idea From 7e3a7b50680cdbd7f19688cae48c873f799dc09e Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Mon, 28 Apr 2025 19:09:27 +0200 Subject: [PATCH 04/26] compile_codegen: make SpirvBuilderError use thiserror --- Cargo.lock | 43 ++++++++++++++------ crates/spirv-builder/Cargo.toml | 1 + crates/spirv-builder/src/lib.rs | 69 ++++++++------------------------- 3 files changed, 49 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bb709cad86f..faa5093fb3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -70,7 +70,7 @@ dependencies = [ "ndk-context", "ndk-sys 0.6.0+11769913", "num_enum", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -319,7 +319,7 @@ dependencies = [ "polling", "rustix", "slab", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -1107,7 +1107,7 @@ checksum = "c151a2a5ef800297b4e79efa4f4bec035c5f51d5ae587287c9b952bdf734cacd" dependencies = [ "log", "presser", - "thiserror", + "thiserror 1.0.69", "windows", ] @@ -1260,7 +1260,7 @@ dependencies = [ "combine", "jni-sys", "log", - "thiserror", + "thiserror 1.0.69", "walkdir", "windows-sys 0.45.0", ] @@ -1546,7 +1546,7 @@ dependencies = [ "rustc-hash", "spirv", "termcolor", - "thiserror", + "thiserror 1.0.69", "unicode-xid", ] @@ -1563,7 +1563,7 @@ dependencies = [ "num_enum", "raw-window-handle 0.5.2", "raw-window-handle 0.6.2", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -2231,7 +2231,7 @@ checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" dependencies = [ "getrandom", "libredox", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -2594,7 +2594,7 @@ dependencies = [ "log", "memmap2", "rustix", - "thiserror", + "thiserror 1.0.69", "wayland-backend", "wayland-client 0.31.7", "wayland-csd-frame", @@ -2655,6 +2655,7 @@ dependencies = [ "rustc_codegen_spirv-types", "serde", "serde_json", + "thiserror 2.0.12", ] [[package]] @@ -2813,7 +2814,16 @@ version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.69", +] + +[[package]] +name = "thiserror" +version = "2.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "567b8a2dae586314f7be2a752ec7474332959c6460e02bde30d702a66d488708" +dependencies = [ + "thiserror-impl 2.0.12", ] [[package]] @@ -2827,6 +2837,17 @@ dependencies = [ "syn", ] +[[package]] +name = "thiserror-impl" +version = "2.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "thorin-dwp" version = "0.8.0" @@ -3412,7 +3433,7 @@ dependencies = [ "raw-window-handle 0.6.2", "rustc-hash", "smallvec", - "thiserror", + "thiserror 1.0.69", "wgpu-hal", "wgpu-types", ] @@ -3454,7 +3475,7 @@ dependencies = [ "renderdoc-sys", "rustc-hash", "smallvec", - "thiserror", + "thiserror 1.0.69", "wasm-bindgen", "web-sys", "wgpu-types", diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index bf7585efc60..55bf55f1207 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -41,5 +41,6 @@ memchr = "2.4" raw-string = "0.3.5" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +thiserror = "2.0.12" notify = { version = "7.0", optional = true } diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index e5fe749e74d..9422bccdd5f 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -81,79 +81,42 @@ use serde::Deserialize; use std::borrow::Borrow; use std::collections::HashMap; use std::env; -use std::error::Error; -use std::fmt; use std::fs::File; use std::io::BufReader; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use thiserror::Error; pub use rustc_codegen_spirv_types::Capability; pub use rustc_codegen_spirv_types::{CompileResult, ModuleResult}; -#[derive(Debug)] +#[derive(Debug, Error)] #[non_exhaustive] pub enum SpirvBuilderError { + #[error("`target` must be set, for example `spirv-unknown-vulkan1.2`")] + NoTargetSet, + #[error("expected `{SPIRV_TARGET_PREFIX}...` target, found `{target}`")] NonSpirvTarget { target: String }, + #[error("SPIR-V target `{SPIRV_TARGET_PREFIX}-{target_env}` is not supported")] UnsupportedSpirvTargetEnv { target_env: String }, + #[error("`path_to_crate` must be set")] + NoCratePathSet, + #[error("crate path {0} does not exist")] CratePathDoesntExist(PathBuf), + #[error("build failed")] BuildFailed, + #[error("multi-module build cannot be used with print_metadata = MetadataPrintout::Full")] MultiModuleWithPrintMetadata, + #[error("watching within build scripts will prevent build completion")] WatchWithPrintMetadata, - MetadataFileMissing(std::io::Error), - MetadataFileMalformed(serde_json::Error), + #[error("multi-module metadata file missing")] + MetadataFileMissing(#[from] std::io::Error), + #[error("unable to parse multi-module metadata file")] + MetadataFileMalformed(#[from] serde_json::Error), } const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; -impl fmt::Display for SpirvBuilderError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::NonSpirvTarget { target } => { - write!( - f, - "expected `{SPIRV_TARGET_PREFIX}...` target, found `{target}`" - ) - } - Self::UnsupportedSpirvTargetEnv { target_env } if target_env.starts_with("opencl") => { - write!( - f, - "OpenCL targets like `{SPIRV_TARGET_PREFIX}-{target_env}` are not supported" - ) - } - Self::UnsupportedSpirvTargetEnv { target_env } if target_env.starts_with("webgpu") => { - write!( - f, - "WebGPU targets like `{SPIRV_TARGET_PREFIX}-{target_env}` are not supported, \ - consider using `{SPIRV_TARGET_PREFIX}-vulkan1.0` instead" - ) - } - Self::UnsupportedSpirvTargetEnv { target_env } => { - write!( - f, - "SPIR-V target `{SPIRV_TARGET_PREFIX}-{target_env}` is not supported" - ) - } - Self::CratePathDoesntExist(path) => { - write!(f, "crate path {} does not exist", path.display()) - } - Self::BuildFailed => f.write_str("build failed"), - Self::MultiModuleWithPrintMetadata => f.write_str( - "multi-module build cannot be used with print_metadata = MetadataPrintout::Full", - ), - Self::WatchWithPrintMetadata => { - f.write_str("watching within build scripts will prevent build completion") - } - Self::MetadataFileMissing(_) => f.write_str("multi-module metadata file missing"), - Self::MetadataFileMalformed(_) => { - f.write_str("unable to parse multi-module metadata file") - } - } - } -} - -impl Error for SpirvBuilderError {} - #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum MetadataPrintout { /// Print no cargo metadata. From 49a962be27ca202c3d189c0c8dedffff41101a1a Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Sun, 27 Apr 2025 15:14:57 +0200 Subject: [PATCH 05/26] compile_codegen: make SpirvBuilder fields pub, reuse spirv-tools structs --- crates/spirv-builder/src/lib.rs | 323 ++++++++++++++++++++++---------- 1 file changed, 220 insertions(+), 103 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 9422bccdd5f..23be70da4f0 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -94,13 +94,13 @@ pub use rustc_codegen_spirv_types::{CompileResult, ModuleResult}; #[non_exhaustive] pub enum SpirvBuilderError { #[error("`target` must be set, for example `spirv-unknown-vulkan1.2`")] - NoTargetSet, + MissingTarget, #[error("expected `{SPIRV_TARGET_PREFIX}...` target, found `{target}`")] NonSpirvTarget { target: String }, #[error("SPIR-V target `{SPIRV_TARGET_PREFIX}-{target_env}` is not supported")] UnsupportedSpirvTargetEnv { target_env: String }, #[error("`path_to_crate` must be set")] - NoCratePathSet, + MissingCratePath, #[error("crate path {0} does not exist")] CratePathDoesntExist(PathBuf), #[error("build failed")] @@ -117,7 +117,7 @@ pub enum SpirvBuilderError { const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] pub enum MetadataPrintout { /// Print no cargo metadata. None, @@ -126,12 +126,14 @@ pub enum MetadataPrintout { /// Print all cargo metadata. /// /// Includes dependency information and spirv environment variable. + #[default] Full, } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] pub enum SpirvMetadata { /// Strip all names and other debug information from SPIR-V output. + #[default] None, /// Only include `OpName`s for public interface variables (uniforms and the like), to allow /// shader reflection. @@ -141,13 +143,14 @@ pub enum SpirvMetadata { } /// Strategy used to handle Rust `panic!`s in shaders compiled to SPIR-V. -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] pub enum ShaderPanicStrategy { /// Return from shader entry-point with no side-effects **(default)**. /// /// While similar to the standard SPIR-V `OpTerminateInvocation`, this is /// *not* limited to fragment shaders, and instead supports all shaders /// (as it's handled via control-flow rewriting, instead of SPIR-V features). + #[default] SilentExit, /// Like `SilentExit`, but also using `debugPrintf` to report the panic in @@ -221,56 +224,162 @@ pub enum ShaderPanicStrategy { UNSOUND_DO_NOT_USE_UndefinedBehaviorViaUnreachable, } +/// Options for specifying the behavior of the validator +/// Copied from `spirv-tools/src/val.rs` struct `ValidatorOptions`, with some fields disabled. +#[derive(Default, Clone)] +pub struct ValidatorOptions { + /// Record whether or not the validator should relax the rules on types for + /// stores to structs. When relaxed, it will allow a type mismatch as long as + /// the types are structs with the same layout. Two structs have the same layout + /// if + /// + /// 1) the members of the structs are either the same type or are structs with + /// same layout, and + /// + /// 2) the decorations that affect the memory layout are identical for both + /// types. Other decorations are not relevant. + pub relax_struct_store: bool, + /// Records whether or not the validator should relax the rules on pointer usage + /// in logical addressing mode. + /// + /// When relaxed, it will allow the following usage cases of pointers: + /// 1) `OpVariable` allocating an object whose type is a pointer type + /// 2) `OpReturnValue` returning a pointer value + pub relax_logical_pointer: bool, + // /// Records whether or not the validator should relax the rules because it is + // /// expected that the optimizations will make the code legal. + // /// + // /// When relaxed, it will allow the following: + // /// 1) It will allow relaxed logical pointers. Setting this option will also + // /// set that option. + // /// 2) Pointers that are pass as parameters to function calls do not have to + // /// match the storage class of the formal parameter. + // /// 3) Pointers that are actaul parameters on function calls do not have to point + // /// to the same type pointed as the formal parameter. The types just need to + // /// logically match. + // pub before_legalization: bool, + /// Records whether the validator should use "relaxed" block layout rules. + /// Relaxed layout rules are described by Vulkan extension + /// `VK_KHR_relaxed_block_layout`, and they affect uniform blocks, storage blocks, + /// and push constants. + /// + /// This is enabled by default when targeting Vulkan 1.1 or later. + /// Relaxed layout is more permissive than the default rules in Vulkan 1.0. + pub relax_block_layout: Option, + /// Records whether the validator should use standard block layout rules for + /// uniform blocks. + pub uniform_buffer_standard_layout: bool, + /// Records whether the validator should use "scalar" block layout rules. + /// Scalar layout rules are more permissive than relaxed block layout. + /// + /// See Vulkan extnesion `VK_EXT_scalar_block_layout`. The scalar alignment is + /// defined as follows: + /// - scalar alignment of a scalar is the scalar size + /// - scalar alignment of a vector is the scalar alignment of its component + /// - scalar alignment of a matrix is the scalar alignment of its component + /// - scalar alignment of an array is the scalar alignment of its element + /// - scalar alignment of a struct is the max scalar alignment among its + /// members + /// + /// For a struct in Uniform, `StorageClass`, or `PushConstant`: + /// - a member Offset must be a multiple of the member's scalar alignment + /// - `ArrayStride` or `MatrixStride` must be a multiple of the array or matrix + /// scalar alignment + pub scalar_block_layout: bool, + /// Records whether or not the validator should skip validating standard + /// uniform/storage block layout. + pub skip_block_layout: bool, + // /// Applies a maximum to one or more Universal limits + // pub max_limits: Vec<(ValidatorLimits, u32)>, +} + +/// Options for specifying the behavior of the optimizer +/// Copied from `spirv-tools/src/opt.rs` struct `Options`, with some fields disabled. +#[derive(Default, Clone)] +pub struct OptimizerOptions { + // /// Records the validator options that should be passed to the validator, + // /// the validator will run with the options before optimizer. + // pub validator_options: Option, + // /// Records the maximum possible value for the id bound. + // pub max_id_bound: Option, + /// Records whether all bindings within the module should be preserved. + pub preserve_bindings: bool, + // /// Records whether all specialization constants within the module + // /// should be preserved. + // pub preserve_spec_constants: bool, +} + /// Cargo features specification for building the shader crate. #[derive(Default)] -struct ShaderCrateFeatures { - default_features: Option, - features: Vec, +pub struct ShaderCrateFeatures { + /// Set --default-features for the target shader crate. + pub default_features: Option, + /// Set --features for the target shader crate. + pub features: Vec, } +#[non_exhaustive] pub struct SpirvBuilder { - path_to_crate: PathBuf, - print_metadata: MetadataPrintout, - release: bool, - target: String, - shader_crate_features: ShaderCrateFeatures, - deny_warnings: bool, - multimodule: bool, - spirv_metadata: SpirvMetadata, - capabilities: Vec, - extensions: Vec, - extra_args: Vec, + pub path_to_crate: Option, + /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::Full`]. + pub print_metadata: MetadataPrintout, + /// Build in release. Defaults to true. + pub release: bool, + /// The target triple, eg. `spirv-unknown-vulkan1.2` + pub target: Option, + /// Cargo features specification for building the shader crate. + pub shader_crate_features: ShaderCrateFeatures, + /// Deny any warnings, as they may never be printed when building within a build script. Defaults to false. + pub deny_warnings: bool, + /// Splits the resulting SPIR-V file into one module per entry point. This is useful in cases + /// where ecosystem tooling has bugs around multiple entry points per module - having all entry + /// points bundled into a single file is the preferred system. + pub multimodule: bool, + /// Sets the level of metadata (primarily `OpName` and `OpLine`) included in the SPIR-V binary. + /// Including metadata significantly increases binary size. + pub spirv_metadata: SpirvMetadata, + /// Adds a capability to the SPIR-V module. Checking if a capability is enabled in code can be + /// done via `#[cfg(target_feature = "TheCapability")]`. + pub capabilities: Vec, + /// Adds an extension to the SPIR-V module. Checking if an extension is enabled in code can be + /// done via `#[cfg(target_feature = "ext:the_extension")]`. + pub extensions: Vec, + /// Set additional "codegen arg". Note: the `RUSTGPU_CODEGEN_ARGS` environment variable + /// takes precedence over any set arguments using this function. + pub extra_args: Vec, // Optional location of a known `rustc_codegen_spirv` dylib - rustc_codegen_spirv_location: Option, - // Optional location of a known "target-spec" file - path_to_target_spec: Option, - target_dir_path: Option, + pub rustc_codegen_spirv_location: Option, + + /// The path of the "target specification" file. + /// + /// For more info on "target specification" see + /// [this RFC](https://rust-lang.github.io/rfcs/0131-target-specification.html). + pub path_to_target_spec: Option, + /// Set the target dir path within `./target` to use for building shaders. Defaults to `spirv-builder`, resulting + /// in the path `./target/spirv-builder`. + pub target_dir_path: Option, // `rustc_codegen_spirv::linker` codegen args + /// Change the shader `panic!` handling strategy (see [`ShaderPanicStrategy`]). pub shader_panic_strategy: ShaderPanicStrategy, - // spirv-val flags - pub relax_struct_store: bool, - pub relax_logical_pointer: bool, - pub relax_block_layout: bool, - pub uniform_buffer_standard_layout: bool, - pub scalar_block_layout: bool, - pub skip_block_layout: bool, + /// spirv-val flags + pub validator: ValidatorOptions, - // spirv-opt flags - pub preserve_bindings: bool, + /// spirv-opt flags + pub optimizer: OptimizerOptions, } -impl SpirvBuilder { - pub fn new(path_to_crate: impl AsRef, target: impl Into) -> Self { +impl Default for SpirvBuilder { + fn default() -> Self { Self { - path_to_crate: path_to_crate.as_ref().to_owned(), - print_metadata: MetadataPrintout::Full, + path_to_crate: None, + print_metadata: MetadataPrintout::default(), release: true, - target: target.into(), + target: None, deny_warnings: false, multimodule: false, - spirv_metadata: SpirvMetadata::None, + spirv_metadata: SpirvMetadata::default(), capabilities: Vec::new(), extensions: Vec::new(), extra_args: Vec::new(), @@ -278,19 +387,22 @@ impl SpirvBuilder { path_to_target_spec: None, target_dir_path: None, - shader_panic_strategy: ShaderPanicStrategy::SilentExit, - - relax_struct_store: false, - relax_logical_pointer: false, - relax_block_layout: false, - uniform_buffer_standard_layout: false, - scalar_block_layout: false, - skip_block_layout: false, - - preserve_bindings: false, + shader_panic_strategy: ShaderPanicStrategy::default(), + validator: ValidatorOptions::default(), + optimizer: OptimizerOptions::default(), shader_crate_features: ShaderCrateFeatures::default(), } } +} + +impl SpirvBuilder { + pub fn new(path_to_crate: impl AsRef, target: impl Into) -> Self { + Self { + path_to_crate: Some(path_to_crate.as_ref().to_owned()), + target: Some(target.into()), + ..SpirvBuilder::default() + } + } /// Sets the path of the "target specification" file. /// @@ -365,7 +477,7 @@ impl SpirvBuilder { /// Allow store from one struct type to a different type with compatible layout and members. #[must_use] pub fn relax_struct_store(mut self, v: bool) -> Self { - self.relax_struct_store = v; + self.validator.relax_struct_store = v; self } @@ -373,7 +485,7 @@ impl SpirvBuilder { /// in logical addressing mode #[must_use] pub fn relax_logical_pointer(mut self, v: bool) -> Self { - self.relax_logical_pointer = v; + self.validator.relax_logical_pointer = v; self } @@ -381,7 +493,7 @@ impl SpirvBuilder { /// push constant layouts. This is the default when targeting Vulkan 1.1 or later. #[must_use] pub fn relax_block_layout(mut self, v: bool) -> Self { - self.relax_block_layout = v; + self.validator.relax_block_layout = Some(v); self } @@ -389,7 +501,7 @@ impl SpirvBuilder { /// layouts. #[must_use] pub fn uniform_buffer_standard_layout(mut self, v: bool) -> Self { - self.uniform_buffer_standard_layout = v; + self.validator.uniform_buffer_standard_layout = v; self } @@ -398,7 +510,7 @@ impl SpirvBuilder { /// in effect this will override the --relax-block-layout option. #[must_use] pub fn scalar_block_layout(mut self, v: bool) -> Self { - self.scalar_block_layout = v; + self.validator.scalar_block_layout = v; self } @@ -406,14 +518,14 @@ impl SpirvBuilder { /// --scalar-block-layout option. #[must_use] pub fn skip_block_layout(mut self, v: bool) -> Self { - self.skip_block_layout = v; + self.validator.skip_block_layout = v; self } /// Preserve unused descriptor bindings. Useful for reflection. #[must_use] pub fn preserve_bindings(mut self, v: bool) -> Self { - self.preserve_bindings = v; + self.optimizer.preserve_bindings = v; self } @@ -464,8 +576,7 @@ impl SpirvBuilder { /// Builds the module. If `print_metadata` is [`MetadataPrintout::Full`], you usually don't have to inspect the path /// in the result, as the environment variable for the path to the module will already be set. - pub fn build(mut self) -> Result { - self.validate_running_conditions()?; + pub fn build(self) -> Result { let metadata_file = invoke_rustc(&self)?; match self.print_metadata { MetadataPrintout::Full | MetadataPrintout::DependencyOnly => { @@ -482,43 +593,6 @@ impl SpirvBuilder { Ok(metadata) } - pub(crate) fn validate_running_conditions(&mut self) -> Result<(), SpirvBuilderError> { - let target_env = self - .target - .strip_prefix(SPIRV_TARGET_PREFIX) - .ok_or_else(|| SpirvBuilderError::NonSpirvTarget { - target: self.target.clone(), - })?; - // HACK(eddyb) used only to split the full list into groups. - #[allow(clippy::match_same_arms)] - match target_env { - // HACK(eddyb) hardcoded list to avoid checking if the JSON file - // for a particular target exists (and sanitizing strings for paths). - // - // FIXME(eddyb) consider moving this list, or even `target-specs`, - // into `rustc_codegen_spirv_types`'s code/source. - "spv1.0" | "spv1.1" | "spv1.2" | "spv1.3" | "spv1.4" | "spv1.5" => {} - "opengl4.0" | "opengl4.1" | "opengl4.2" | "opengl4.3" | "opengl4.5" => {} - "vulkan1.0" | "vulkan1.1" | "vulkan1.1spv1.4" | "vulkan1.2" => {} - - _ => { - return Err(SpirvBuilderError::UnsupportedSpirvTargetEnv { - target_env: target_env.into(), - }); - } - } - - if (self.print_metadata == MetadataPrintout::Full) && self.multimodule { - return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); - } - if !self.path_to_crate.is_dir() { - return Err(SpirvBuilderError::CratePathDoesntExist(std::mem::take( - &mut self.path_to_crate, - ))); - } - Ok(()) - } - pub(crate) fn parse_metadata_file( &self, at: &Path, @@ -604,6 +678,49 @@ fn join_checking_for_separators(strings: Vec>, sep: &str) -> St // Returns path to the metadata json. fn invoke_rustc(builder: &SpirvBuilder) -> Result { + let target = builder + .target + .as_ref() + .ok_or(SpirvBuilderError::MissingTarget)?; + let path_to_crate = builder + .path_to_crate + .as_ref() + .ok_or(SpirvBuilderError::MissingCratePath)?; + { + let target_env = target.strip_prefix(SPIRV_TARGET_PREFIX).ok_or_else(|| { + SpirvBuilderError::NonSpirvTarget { + target: target.clone(), + } + })?; + // HACK(eddyb) used only to split the full list into groups. + #[allow(clippy::match_same_arms)] + match target_env { + // HACK(eddyb) hardcoded list to avoid checking if the JSON file + // for a particular target exists (and sanitizing strings for paths). + // + // FIXME(eddyb) consider moving this list, or even `target-specs`, + // into `rustc_codegen_spirv_types`'s code/source. + "spv1.0" | "spv1.1" | "spv1.2" | "spv1.3" | "spv1.4" | "spv1.5" => {} + "opengl4.0" | "opengl4.1" | "opengl4.2" | "opengl4.3" | "opengl4.5" => {} + "vulkan1.0" | "vulkan1.1" | "vulkan1.1spv1.4" | "vulkan1.2" => {} + + _ => { + return Err(SpirvBuilderError::UnsupportedSpirvTargetEnv { + target_env: target_env.into(), + }); + } + } + + if (builder.print_metadata == MetadataPrintout::Full) && builder.multimodule { + return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); + } + if !path_to_crate.is_dir() { + return Err(SpirvBuilderError::CratePathDoesntExist( + path_to_crate.clone(), + )); + } + } + // Okay, this is a little bonkers: in a normal world, we'd have the user clone // rustc_codegen_spirv and pass in the path to it, and then we'd invoke cargo to build it, grab // the resulting .so, and pass it into -Z codegen-backend. But that's really gross: the user @@ -659,25 +776,25 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { } SpirvMetadata::Full => llvm_args.push("--spirv-metadata=full".to_string()), } - if builder.relax_struct_store { + if builder.validator.relax_struct_store { llvm_args.push("--relax-struct-store".to_string()); } - if builder.relax_logical_pointer { + if builder.validator.relax_logical_pointer { llvm_args.push("--relax-logical-pointer".to_string()); } - if builder.relax_block_layout { + if builder.validator.relax_block_layout.unwrap_or(false) { llvm_args.push("--relax-block-layout".to_string()); } - if builder.uniform_buffer_standard_layout { + if builder.validator.uniform_buffer_standard_layout { llvm_args.push("--uniform-buffer-standard-layout".to_string()); } - if builder.scalar_block_layout { + if builder.validator.scalar_block_layout { llvm_args.push("--scalar-block-layout".to_string()); } - if builder.skip_block_layout { + if builder.validator.skip_block_layout { llvm_args.push("--skip-block-layout".to_string()); } - if builder.preserve_bindings { + if builder.optimizer.preserve_bindings { llvm_args.push("--preserve-bindings".to_string()); } let mut target_features = vec![]; @@ -779,7 +896,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { .arg(builder.path_to_target_spec.clone().unwrap_or_else(|| { PathBuf::from(env!("CARGO_MANIFEST_DIR")) .join("target-specs") - .join(format!("{}.json", builder.target)) + .join(format!("{}.json", target)) })); if let Some(default_features) = builder.shader_crate_features.default_features { @@ -837,7 +954,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { let build = cargo .stderr(Stdio::inherit()) - .current_dir(&builder.path_to_crate) + .current_dir(&path_to_crate) .output() .expect("failed to execute cargo build"); From 1c0baee9bcdfac59458b947ebdf080b3b6cd8fae Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Mon, 28 Apr 2025 19:14:34 +0200 Subject: [PATCH 06/26] compile_codegen: forward missing codegen backend error --- crates/spirv-builder/src/lib.rs | 45 ++++++++++++++++----------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 23be70da4f0..b30512bb797 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -103,6 +103,10 @@ pub enum SpirvBuilderError { MissingCratePath, #[error("crate path {0} does not exist")] CratePathDoesntExist(PathBuf), + #[error( + "Without feature `compile_codegen`, you need to set the path of the codegen dylib using `rustc_codegen_spirv_location(...)`" + )] + MissingCodegenBackendDylib, #[error("build failed")] BuildFailed, #[error("multi-module build cannot be used with print_metadata = MetadataPrintout::Full")] @@ -643,27 +647,23 @@ fn dylib_path() -> Vec { } } -#[cfg(feature = "compile_codegen")] -fn find_rustc_codegen_spirv() -> PathBuf { - let filename = format!( - "{}rustc_codegen_spirv{}", - env::consts::DLL_PREFIX, - env::consts::DLL_SUFFIX - ); - for mut path in dylib_path() { - path.push(&filename); - if path.is_file() { - return path; +fn find_rustc_codegen_spirv() -> Result { + if cfg!(feature = "compile_codegen") { + let filename = format!( + "{}rustc_codegen_spirv{}", + env::consts::DLL_PREFIX, + env::consts::DLL_SUFFIX + ); + for mut path in dylib_path() { + path.push(&filename); + if path.is_file() { + return Ok(path); + } } + panic!("Could not find {filename} in library path"); + } else { + Err(SpirvBuilderError::MissingCodegenBackendDylib) } - panic!("Could not find {filename} in library path"); -} - -#[cfg(not(feature = "compile_codegen"))] -fn find_rustc_codegen_spirv() -> PathBuf { - panic!( - "Without feature `compile_codegen`, you need to set the path of the codegen dylib using `rustc_codegen_spirv_location(...)`" - ); } /// Joins strings together while ensuring none of the strings contain the separator. @@ -729,10 +729,9 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // alongside build.rs, and cargo will helpfully add it to LD_LIBRARY_PATH for us! However, // rustc expects a full path, instead of a filename looked up via LD_LIBRARY_PATH, so we need // to copy cargo's understanding of library lookup and find the library and its full path. - let rustc_codegen_spirv = builder - .rustc_codegen_spirv_location - .clone() - .unwrap_or_else(find_rustc_codegen_spirv); + let rustc_codegen_spirv = Ok(builder.rustc_codegen_spirv_location.clone()) + .transpose() + .unwrap_or_else(find_rustc_codegen_spirv)?; let mut rustflags = vec![ format!("-Zcodegen-backend={}", rustc_codegen_spirv.display()), From fe42c46fe014c2a73024dc31c1af1e9d00c9b7ca Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 15:19:41 +0200 Subject: [PATCH 07/26] compile_codegen: fix watch feature --- crates/spirv-builder/src/watch.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index 9c011fd6492..32a2393c193 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -12,10 +12,13 @@ impl SpirvBuilder { /// Returns the result of the first successful compilation, then calls /// `on_compilation_finishes` for each subsequent compilation. pub fn watch( - mut self, + self, mut on_compilation_finishes: impl FnMut(CompileResult) + Send + 'static, ) -> Result { - self.validate_running_conditions()?; + let path_to_crate = self + .path_to_crate + .as_ref() + .ok_or(SpirvBuilderError::MissingCratePath)?; if !matches!(self.print_metadata, crate::MetadataPrintout::None) { return Err(SpirvBuilderError::WatchWithPrintMetadata); } @@ -43,7 +46,7 @@ impl SpirvBuilder { .expect("Could create watcher"); // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, watcher - .watch(&self.path_to_crate, RecursiveMode::Recursive) + .watch(&path_to_crate, RecursiveMode::Recursive) .expect("Could watch crate root"); loop { rx.recv().expect("Watcher still alive"); From 674214b6b129d1a1c397239061b6652d89f8cd79 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 15:32:57 +0200 Subject: [PATCH 08/26] compile_codegen: fix lint --- crates/spirv-builder/src/lib.rs | 2 +- crates/spirv-builder/src/watch.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index b30512bb797..6de77014c07 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -953,7 +953,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { let build = cargo .stderr(Stdio::inherit()) - .current_dir(&path_to_crate) + .current_dir(path_to_crate) .output() .expect("failed to execute cargo build"); diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index 32a2393c193..fd1e16d956c 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -46,7 +46,7 @@ impl SpirvBuilder { .expect("Could create watcher"); // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, watcher - .watch(&path_to_crate, RecursiveMode::Recursive) + .watch(path_to_crate, RecursiveMode::Recursive) .expect("Could watch crate root"); loop { rx.recv().expect("Watcher still alive"); From 879338b3dadaf1168a0db388c9cb7a7167eade6f Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 15:25:30 +0200 Subject: [PATCH 09/26] compile_codegen: validate `rustc_codegen_spirv_location` on build --- crates/spirv-builder/src/lib.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 6de77014c07..48a2a206917 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -107,6 +107,8 @@ pub enum SpirvBuilderError { "Without feature `compile_codegen`, you need to set the path of the codegen dylib using `rustc_codegen_spirv_location(...)`" )] MissingCodegenBackendDylib, + #[error("`rustc_codegen_spirv_location` path '{0}' is not a file")] + CodegenBackendDylibDoesNotExist(PathBuf), #[error("build failed")] BuildFailed, #[error("multi-module build cannot be used with print_metadata = MetadataPrintout::Full")] @@ -556,17 +558,8 @@ impl SpirvBuilder { } #[must_use] - pub fn rustc_codegen_spirv_location( - mut self, - path_to_dylib: impl AsRef, - ) -> Self { - let path_to_dylib = path_to_dylib.as_ref().to_path_buf(); - assert!( - path_to_dylib.is_file(), - "Provided path to dylib '{}' is not a file", - path_to_dylib.display() - ); - self.rustc_codegen_spirv_location = Some(path_to_dylib); + pub fn rustc_codegen_spirv_location(mut self, path_to_dylib: impl AsRef) -> Self { + self.rustc_codegen_spirv_location = Some(path_to_dylib.as_ref().to_path_buf()); self } @@ -732,6 +725,11 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { let rustc_codegen_spirv = Ok(builder.rustc_codegen_spirv_location.clone()) .transpose() .unwrap_or_else(find_rustc_codegen_spirv)?; + if !rustc_codegen_spirv.is_file() { + return Err(SpirvBuilderError::CodegenBackendDylibDoesNotExist( + rustc_codegen_spirv, + )); + } let mut rustflags = vec![ format!("-Zcodegen-backend={}", rustc_codegen_spirv.display()), From bb5d2e0640ca966783f85f37be3de22c43eed56d Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 15:31:00 +0200 Subject: [PATCH 10/26] compile_codegen: rename feature from `compile_codegen` to `rustc_codegen_spirv` --- crates/spirv-builder/Cargo.toml | 6 +++--- crates/spirv-builder/src/lib.rs | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index 55bf55f1207..3ac3c5aa123 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -21,10 +21,10 @@ no-default-features = true default = ["use-compiled-tools"] # Compile `rustc_codegen_spirv`, allows constructing SpirvBuilder without # explicitly passing in a path to a compiled `rustc_codegen_spirv.so` (or dll) -compile_codegen = ["dep:rustc_codegen_spirv"] +rustc_codegen_spirv = ["dep:rustc_codegen_spirv"] # See `rustc_codegen_spirv/Cargo.toml` for details on these features. -use-installed-tools = ["compile_codegen", "rustc_codegen_spirv?/use-installed-tools"] -use-compiled-tools = ["compile_codegen", "rustc_codegen_spirv?/use-compiled-tools"] +use-installed-tools = ["rustc_codegen_spirv", "rustc_codegen_spirv?/use-installed-tools"] +use-compiled-tools = ["rustc_codegen_spirv", "rustc_codegen_spirv?/use-compiled-tools"] skip-toolchain-check = ["rustc_codegen_spirv?/skip-toolchain-check"] watch = ["dep:notify"] diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 48a2a206917..9f859482961 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -101,14 +101,14 @@ pub enum SpirvBuilderError { UnsupportedSpirvTargetEnv { target_env: String }, #[error("`path_to_crate` must be set")] MissingCratePath, - #[error("crate path {0} does not exist")] + #[error("crate path '{0}' does not exist")] CratePathDoesntExist(PathBuf), #[error( - "Without feature `compile_codegen`, you need to set the path of the codegen dylib using `rustc_codegen_spirv_location(...)`" + "Without feature `rustc_codegen_spirv`, you need to set the path of the dylib using `rustc_codegen_spirv_location(...)`" )] - MissingCodegenBackendDylib, + MissingRustcCodegenSpirvDylib, #[error("`rustc_codegen_spirv_location` path '{0}' is not a file")] - CodegenBackendDylibDoesNotExist(PathBuf), + RustcCodegenSpirvDylibDoesNotExist(PathBuf), #[error("build failed")] BuildFailed, #[error("multi-module build cannot be used with print_metadata = MetadataPrintout::Full")] @@ -353,7 +353,7 @@ pub struct SpirvBuilder { /// Set additional "codegen arg". Note: the `RUSTGPU_CODEGEN_ARGS` environment variable /// takes precedence over any set arguments using this function. pub extra_args: Vec, - // Optional location of a known `rustc_codegen_spirv` dylib + // Location of a known `rustc_codegen_spirv` dylib, only required without feature `rustc_codegen_spirv`. pub rustc_codegen_spirv_location: Option, /// The path of the "target specification" file. @@ -641,7 +641,7 @@ fn dylib_path() -> Vec { } fn find_rustc_codegen_spirv() -> Result { - if cfg!(feature = "compile_codegen") { + if cfg!(feature = "rustc_codegen_spirv") { let filename = format!( "{}rustc_codegen_spirv{}", env::consts::DLL_PREFIX, @@ -655,7 +655,7 @@ fn find_rustc_codegen_spirv() -> Result { } panic!("Could not find {filename} in library path"); } else { - Err(SpirvBuilderError::MissingCodegenBackendDylib) + Err(SpirvBuilderError::MissingRustcCodegenSpirvDylib) } } @@ -726,7 +726,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { .transpose() .unwrap_or_else(find_rustc_codegen_spirv)?; if !rustc_codegen_spirv.is_file() { - return Err(SpirvBuilderError::CodegenBackendDylibDoesNotExist( + return Err(SpirvBuilderError::RustcCodegenSpirvDylibDoesNotExist( rustc_codegen_spirv, )); } From 523516135e8c3d72ed1f341804d773e7e29ff0c3 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 15:40:46 +0200 Subject: [PATCH 11/26] compile_codegen: add serde derives --- Cargo.lock | 5 +++++ crates/rustc_codegen_spirv-types/Cargo.toml | 1 + crates/spirv-builder/src/lib.rs | 13 +++++++------ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index faa5093fb3f..d2f1624219f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -260,6 +260,9 @@ name = "bitflags" version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +dependencies = [ + "serde", +] [[package]] name = "block" @@ -2350,6 +2353,7 @@ dependencies = [ "rspirv", "serde", "serde_json", + "spirv", ] [[package]] @@ -2642,6 +2646,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eda41003dc44290527a59b13432d4a0379379fa074b70174882adfbdfd917844" dependencies = [ "bitflags 2.6.0", + "serde", ] [[package]] diff --git a/crates/rustc_codegen_spirv-types/Cargo.toml b/crates/rustc_codegen_spirv-types/Cargo.toml index e2d64d35cfb..36bd737f7e7 100644 --- a/crates/rustc_codegen_spirv-types/Cargo.toml +++ b/crates/rustc_codegen_spirv-types/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true repository.workspace = true [dependencies] +spirv = { version = "0.3.0", features = ["serialize", "deserialize"] } rspirv = "0.12" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 9f859482961..04f40114558 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -123,7 +123,7 @@ pub enum SpirvBuilderError { const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; -#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] pub enum MetadataPrintout { /// Print no cargo metadata. None, @@ -136,7 +136,7 @@ pub enum MetadataPrintout { Full, } -#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] pub enum SpirvMetadata { /// Strip all names and other debug information from SPIR-V output. #[default] @@ -149,7 +149,7 @@ pub enum SpirvMetadata { } /// Strategy used to handle Rust `panic!`s in shaders compiled to SPIR-V. -#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] pub enum ShaderPanicStrategy { /// Return from shader entry-point with no side-effects **(default)**. /// @@ -232,7 +232,7 @@ pub enum ShaderPanicStrategy { /// Options for specifying the behavior of the validator /// Copied from `spirv-tools/src/val.rs` struct `ValidatorOptions`, with some fields disabled. -#[derive(Default, Clone)] +#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] pub struct ValidatorOptions { /// Record whether or not the validator should relax the rules on types for /// stores to structs. When relaxed, it will allow a type mismatch as long as @@ -301,7 +301,7 @@ pub struct ValidatorOptions { /// Options for specifying the behavior of the optimizer /// Copied from `spirv-tools/src/opt.rs` struct `Options`, with some fields disabled. -#[derive(Default, Clone)] +#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] pub struct OptimizerOptions { // /// Records the validator options that should be passed to the validator, // /// the validator will run with the options before optimizer. @@ -316,7 +316,7 @@ pub struct OptimizerOptions { } /// Cargo features specification for building the shader crate. -#[derive(Default)] +#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] pub struct ShaderCrateFeatures { /// Set --default-features for the target shader crate. pub default_features: Option, @@ -325,6 +325,7 @@ pub struct ShaderCrateFeatures { } #[non_exhaustive] +#[derive(Clone, serde::Deserialize, serde::Serialize)] pub struct SpirvBuilder { pub path_to_crate: Option, /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::Full`]. From de2cef8b47ea2016c377ff6e2cbb51c46ac9ee81 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 16:18:33 +0200 Subject: [PATCH 12/26] compile_codegen: add clap feature, make SpirvBuilder args the same as cargo gpu --- Cargo.lock | 13 ++++--- crates/spirv-builder/Cargo.toml | 3 ++ crates/spirv-builder/src/lib.rs | 65 +++++++++++++++++++++++++++++---- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d2f1624219f..0c366481d7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -374,9 +374,9 @@ checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" [[package]] name = "clap" -version = "4.5.23" +version = "4.5.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3135e7ec2ef7b10c6ed8950f0f792ed96ee093fa088608f1c76e569722700c84" +checksum = "eccb054f56cbd38340b380d4a8e69ef1f02f1af43db2f0cc817a4774d80ae071" dependencies = [ "clap_builder", "clap_derive", @@ -384,9 +384,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.23" +version = "4.5.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30582fc632330df2bd26877bde0c1f4470d57c582bbc070376afcd04d8cb4838" +checksum = "efd9466fac8543255d3b1fcad4762c5e116ffe808c8a3043d4263cd4fd4862a2" dependencies = [ "anstream", "anstyle", @@ -396,9 +396,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.18" +version = "4.5.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ac6a0c7b1a9e9a5186361f67dfa1b88213572f427fb9ab038efb2bd8c582dab" +checksum = "09176aae279615badda0765c0c0b3f6ed53f4709118af73cf4655d85d1530cd7" dependencies = [ "heck", "proc-macro2", @@ -2653,6 +2653,7 @@ dependencies = [ name = "spirv-builder" version = "0.9.0" dependencies = [ + "clap", "memchr", "notify", "raw-string", diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index 3ac3c5aa123..1b3084da725 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -28,6 +28,7 @@ use-compiled-tools = ["rustc_codegen_spirv", "rustc_codegen_spirv?/use-compiled- skip-toolchain-check = ["rustc_codegen_spirv?/skip-toolchain-check"] watch = ["dep:notify"] +clap = ["dep:clap"] [dependencies] # See comment in `src/lib.rs` `invoke_rustc` regarding `rustc_codegen_spirv` dep. @@ -44,3 +45,5 @@ serde_json = "1.0" thiserror = "2.0.12" notify = { version = "7.0", optional = true } +# Pinning clap, as newer versions have raised min rustc version without being marked a breaking change +clap = { version = "=4.5.37", optional = true, features = ["derive"] } diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 04f40114558..d4318060ab9 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -124,6 +124,7 @@ pub enum SpirvBuilderError { const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; #[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] pub enum MetadataPrintout { /// Print no cargo metadata. None, @@ -137,6 +138,7 @@ pub enum MetadataPrintout { } #[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] pub enum SpirvMetadata { /// Strip all names and other debug information from SPIR-V output. #[default] @@ -150,6 +152,7 @@ pub enum SpirvMetadata { /// Strategy used to handle Rust `panic!`s in shaders compiled to SPIR-V. #[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] pub enum ShaderPanicStrategy { /// Return from shader entry-point with no side-effects **(default)**. /// @@ -202,6 +205,7 @@ pub enum ShaderPanicStrategy { /// their `debugPrintf` support can be done during instance creation /// * *optional*: integrating [`VK_EXT_debug_utils`](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_debug_utils.html) /// allows more reporting flexibility than `DEBUG_PRINTF_TO_STDOUT=1`) + #[cfg_attr(feature = "clap", clap(skip))] DebugPrintfThenExit { /// Whether to also print the entry-point inputs (excluding buffers/resources), /// which should uniquely identify the panicking shader invocation. @@ -233,6 +237,7 @@ pub enum ShaderPanicStrategy { /// Options for specifying the behavior of the validator /// Copied from `spirv-tools/src/val.rs` struct `ValidatorOptions`, with some fields disabled. #[derive(Default, Clone, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct ValidatorOptions { /// Record whether or not the validator should relax the rules on types for /// stores to structs. When relaxed, it will allow a type mismatch as long as @@ -244,6 +249,7 @@ pub struct ValidatorOptions { /// /// 2) the decorations that affect the memory layout are identical for both /// types. Other decorations are not relevant. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub relax_struct_store: bool, /// Records whether or not the validator should relax the rules on pointer usage /// in logical addressing mode. @@ -251,6 +257,7 @@ pub struct ValidatorOptions { /// When relaxed, it will allow the following usage cases of pointers: /// 1) `OpVariable` allocating an object whose type is a pointer type /// 2) `OpReturnValue` returning a pointer value + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub relax_logical_pointer: bool, // /// Records whether or not the validator should relax the rules because it is // /// expected that the optimizations will make the code legal. @@ -271,9 +278,11 @@ pub struct ValidatorOptions { /// /// This is enabled by default when targeting Vulkan 1.1 or later. /// Relaxed layout is more permissive than the default rules in Vulkan 1.0. + #[cfg_attr(feature = "clap", arg(long, default_value = "None"))] pub relax_block_layout: Option, /// Records whether the validator should use standard block layout rules for /// uniform blocks. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub uniform_buffer_standard_layout: bool, /// Records whether the validator should use "scalar" block layout rules. /// Scalar layout rules are more permissive than relaxed block layout. @@ -291,9 +300,11 @@ pub struct ValidatorOptions { /// - a member Offset must be a multiple of the member's scalar alignment /// - `ArrayStride` or `MatrixStride` must be a multiple of the array or matrix /// scalar alignment + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub scalar_block_layout: bool, /// Records whether or not the validator should skip validating standard /// uniform/storage block layout. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub skip_block_layout: bool, // /// Applies a maximum to one or more Universal limits // pub max_limits: Vec<(ValidatorLimits, u32)>, @@ -302,6 +313,7 @@ pub struct ValidatorOptions { /// Options for specifying the behavior of the optimizer /// Copied from `spirv-tools/src/opt.rs` struct `Options`, with some fields disabled. #[derive(Default, Clone, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct OptimizerOptions { // /// Records the validator options that should be passed to the validator, // /// the validator will run with the options before optimizer. @@ -309,6 +321,7 @@ pub struct OptimizerOptions { // /// Records the maximum possible value for the id bound. // pub max_id_bound: Option, /// Records whether all bindings within the module should be preserved. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub preserve_bindings: bool, // /// Records whether all specialization constants within the module // /// should be preserved. @@ -316,46 +329,70 @@ pub struct OptimizerOptions { } /// Cargo features specification for building the shader crate. -#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] +#[derive(Clone, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct ShaderCrateFeatures { /// Set --default-features for the target shader crate. - pub default_features: Option, + #[cfg_attr(feature = "clap", clap(long = "no-default-features", default_value = "true", action = clap::ArgAction::SetFalse))] + pub default_features: bool, /// Set --features for the target shader crate. + #[cfg_attr(feature = "clap", clap(long))] pub features: Vec, } +impl Default for ShaderCrateFeatures { + fn default() -> Self { + Self { + default_features: true, + features: Vec::new(), + } + } +} + #[non_exhaustive] #[derive(Clone, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct SpirvBuilder { pub path_to_crate: Option, /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::Full`]. pub print_metadata: MetadataPrintout, /// Build in release. Defaults to true. + #[cfg_attr(feature = "clap", clap(long = "debug", default_value = "true", action = clap::ArgAction::SetFalse))] pub release: bool, /// The target triple, eg. `spirv-unknown-vulkan1.2` + #[cfg_attr( + feature = "clap", + clap(long, default_value = "spirv-unknown-vulkan1.2") + )] pub target: Option, /// Cargo features specification for building the shader crate. + #[cfg_attr(feature = "clap", clap(flatten))] pub shader_crate_features: ShaderCrateFeatures, /// Deny any warnings, as they may never be printed when building within a build script. Defaults to false. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub deny_warnings: bool, /// Splits the resulting SPIR-V file into one module per entry point. This is useful in cases /// where ecosystem tooling has bugs around multiple entry points per module - having all entry /// points bundled into a single file is the preferred system. + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub multimodule: bool, /// Sets the level of metadata (primarily `OpName` and `OpLine`) included in the SPIR-V binary. /// Including metadata significantly increases binary size. + #[cfg_attr(feature = "clap", arg(long, default_value = "none"))] pub spirv_metadata: SpirvMetadata, /// Adds a capability to the SPIR-V module. Checking if a capability is enabled in code can be /// done via `#[cfg(target_feature = "TheCapability")]`. + #[cfg_attr(feature = "clap", arg(long, value_parser=Self::parse_spirv_capability))] pub capabilities: Vec, /// Adds an extension to the SPIR-V module. Checking if an extension is enabled in code can be /// done via `#[cfg(target_feature = "ext:the_extension")]`. + #[cfg_attr(feature = "clap", arg(long))] pub extensions: Vec, /// Set additional "codegen arg". Note: the `RUSTGPU_CODEGEN_ARGS` environment variable /// takes precedence over any set arguments using this function. pub extra_args: Vec, // Location of a known `rustc_codegen_spirv` dylib, only required without feature `rustc_codegen_spirv`. - pub rustc_codegen_spirv_location: Option, + pub rustc_codegen_spirv_location: Option, /// The path of the "target specification" file. /// @@ -371,12 +408,26 @@ pub struct SpirvBuilder { pub shader_panic_strategy: ShaderPanicStrategy, /// spirv-val flags + #[cfg_attr(feature = "clap", clap(flatten))] pub validator: ValidatorOptions, /// spirv-opt flags + #[cfg_attr(feature = "clap", clap(flatten))] pub optimizer: OptimizerOptions, } +#[cfg(feature = "clap")] +impl SpirvBuilder { + /// Clap value parser for `Capability`. + fn parse_spirv_capability(capability: &str) -> Result { + use core::str::FromStr; + Capability::from_str(capability).map_or_else( + |()| Err(clap::Error::new(clap::error::ErrorKind::InvalidValue)), + Ok, + ) + } +} + impl Default for SpirvBuilder { fn default() -> Self { Self { @@ -547,7 +598,7 @@ impl SpirvBuilder { /// Set --default-features for the target shader crate. #[must_use] pub fn shader_crate_default_features(mut self, default_features: bool) -> Self { - self.shader_crate_features.default_features = Some(default_features); + self.shader_crate_features.default_features = default_features; self } @@ -897,10 +948,8 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { .join(format!("{}.json", target)) })); - if let Some(default_features) = builder.shader_crate_features.default_features { - if !default_features { - cargo.arg("--no-default-features"); - } + if !builder.shader_crate_features.default_features { + cargo.arg("--no-default-features"); } if !builder.shader_crate_features.features.is_empty() { From d643a65a0ab32e84993715e6bbdd38635a24a0b3 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 17:23:03 +0200 Subject: [PATCH 13/26] compile_codegen: derive Debug for SpirvBuilder --- crates/spirv-builder/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index d4318060ab9..e19f98d8428 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -236,7 +236,7 @@ pub enum ShaderPanicStrategy { /// Options for specifying the behavior of the validator /// Copied from `spirv-tools/src/val.rs` struct `ValidatorOptions`, with some fields disabled. -#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] +#[derive(Default, Debug, Clone, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct ValidatorOptions { /// Record whether or not the validator should relax the rules on types for @@ -312,7 +312,7 @@ pub struct ValidatorOptions { /// Options for specifying the behavior of the optimizer /// Copied from `spirv-tools/src/opt.rs` struct `Options`, with some fields disabled. -#[derive(Default, Clone, serde::Deserialize, serde::Serialize)] +#[derive(Default, Debug, Clone, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct OptimizerOptions { // /// Records the validator options that should be passed to the validator, @@ -329,7 +329,7 @@ pub struct OptimizerOptions { } /// Cargo features specification for building the shader crate. -#[derive(Clone, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct ShaderCrateFeatures { /// Set --default-features for the target shader crate. @@ -350,7 +350,7 @@ impl Default for ShaderCrateFeatures { } #[non_exhaustive] -#[derive(Clone, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct SpirvBuilder { pub path_to_crate: Option, From 6a518ac291cc522c9be7dbcd736ac55e7bd115f2 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 18:42:17 +0200 Subject: [PATCH 14/26] compile_codegen: make clip skip variables not previously exposed --- crates/spirv-builder/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index e19f98d8428..8efe11dfa5d 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -278,7 +278,7 @@ pub struct ValidatorOptions { /// /// This is enabled by default when targeting Vulkan 1.1 or later. /// Relaxed layout is more permissive than the default rules in Vulkan 1.0. - #[cfg_attr(feature = "clap", arg(long, default_value = "None"))] + #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] pub relax_block_layout: Option, /// Records whether the validator should use standard block layout rules for /// uniform blocks. @@ -353,8 +353,10 @@ impl Default for ShaderCrateFeatures { #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] pub struct SpirvBuilder { + #[cfg_attr(feature = "clap", clap(skip))] pub path_to_crate: Option, /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::Full`]. + #[cfg_attr(feature = "clap", clap(skip))] pub print_metadata: MetadataPrintout, /// Build in release. Defaults to true. #[cfg_attr(feature = "clap", clap(long = "debug", default_value = "true", action = clap::ArgAction::SetFalse))] @@ -390,21 +392,26 @@ pub struct SpirvBuilder { pub extensions: Vec, /// Set additional "codegen arg". Note: the `RUSTGPU_CODEGEN_ARGS` environment variable /// takes precedence over any set arguments using this function. + #[cfg_attr(feature = "clap", clap(skip))] pub extra_args: Vec, // Location of a known `rustc_codegen_spirv` dylib, only required without feature `rustc_codegen_spirv`. + #[cfg_attr(feature = "clap", clap(skip))] pub rustc_codegen_spirv_location: Option, /// The path of the "target specification" file. /// /// For more info on "target specification" see /// [this RFC](https://rust-lang.github.io/rfcs/0131-target-specification.html). + #[cfg_attr(feature = "clap", clap(skip))] pub path_to_target_spec: Option, /// Set the target dir path within `./target` to use for building shaders. Defaults to `spirv-builder`, resulting /// in the path `./target/spirv-builder`. + #[cfg_attr(feature = "clap", clap(skip))] pub target_dir_path: Option, // `rustc_codegen_spirv::linker` codegen args /// Change the shader `panic!` handling strategy (see [`ShaderPanicStrategy`]). + #[cfg_attr(feature = "clap", clap(skip))] pub shader_panic_strategy: ShaderPanicStrategy, /// spirv-val flags From 744c844eb761edf23bcd379b5271d128128d7dfa Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Tue, 29 Apr 2025 18:45:00 +0200 Subject: [PATCH 15/26] compile_codegen: serde flatten --- crates/spirv-builder/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 8efe11dfa5d..40353ee0179 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -369,6 +369,7 @@ pub struct SpirvBuilder { pub target: Option, /// Cargo features specification for building the shader crate. #[cfg_attr(feature = "clap", clap(flatten))] + #[serde(flatten)] pub shader_crate_features: ShaderCrateFeatures, /// Deny any warnings, as they may never be printed when building within a build script. Defaults to false. #[cfg_attr(feature = "clap", arg(long, default_value = "false"))] @@ -416,10 +417,12 @@ pub struct SpirvBuilder { /// spirv-val flags #[cfg_attr(feature = "clap", clap(flatten))] + #[serde(flatten)] pub validator: ValidatorOptions, /// spirv-opt flags #[cfg_attr(feature = "clap", clap(flatten))] + #[serde(flatten)] pub optimizer: OptimizerOptions, } From 9a9ebc185121827b7901661ec2ec761e2d660e54 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 14:09:29 +0200 Subject: [PATCH 16/26] compile_codegen: add warning in build.rs about cargo gpu parsing --- crates/rustc_codegen_spirv/build.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/rustc_codegen_spirv/build.rs b/crates/rustc_codegen_spirv/build.rs index b82a0e31c6c..661b840de40 100644 --- a/crates/rustc_codegen_spirv/build.rs +++ b/crates/rustc_codegen_spirv/build.rs @@ -11,6 +11,9 @@ use std::process::{Command, ExitCode}; use std::{env, fs, mem}; /// Current `rust-toolchain.toml` file +/// WARNING!!! cargo-gpu is new relying on this being a string literal! It will +/// scan `build.rs` for any line starting with `channel = "..."` to figure out +/// which toolchain version to use! This also allows backwards compat. /// Unfortunately, directly including the actual workspace `rust-toolchain.toml` doesn't work together with /// `cargo publish`. We need to figure out a way to do this properly, but let's hardcode it for now :/ //const REQUIRED_RUST_TOOLCHAIN: &str = include_str!("../../rust-toolchain.toml"); From da624d1e9720e090b02f89a3a93a83c268372031 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 15:03:15 +0200 Subject: [PATCH 17/26] compile_codegen: make SpirvBuilder::build() no longer consume self --- crates/spirv-builder/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 40353ee0179..2ebe946864d 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -635,8 +635,8 @@ impl SpirvBuilder { /// Builds the module. If `print_metadata` is [`MetadataPrintout::Full`], you usually don't have to inspect the path /// in the result, as the environment variable for the path to the module will already be set. - pub fn build(self) -> Result { - let metadata_file = invoke_rustc(&self)?; + pub fn build(&self) -> Result { + let metadata_file = invoke_rustc(self)?; match self.print_metadata { MetadataPrintout::Full | MetadataPrintout::DependencyOnly => { leaf_deps(&metadata_file, |artifact| { From b619665a24eba00cc3d2d4b15af6d0fc356efef9 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 15:18:16 +0200 Subject: [PATCH 18/26] compile_codegen: add SpirvBuilder.toolchain_overwrite --- crates/spirv-builder/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 2ebe946864d..83d7f790c50 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -398,6 +398,9 @@ pub struct SpirvBuilder { // Location of a known `rustc_codegen_spirv` dylib, only required without feature `rustc_codegen_spirv`. #[cfg_attr(feature = "clap", clap(skip))] pub rustc_codegen_spirv_location: Option, + // Overwrite the toolchain like `cargo +nightly` + #[cfg_attr(feature = "clap", clap(skip))] + pub toolchain_overwrite: Option, /// The path of the "target specification" file. /// @@ -454,7 +457,7 @@ impl Default for SpirvBuilder { rustc_codegen_spirv_location: None, path_to_target_spec: None, target_dir_path: None, - + toolchain_overwrite: None, shader_panic_strategy: ShaderPanicStrategy::default(), validator: ValidatorOptions::default(), optimizer: OptimizerOptions::default(), @@ -934,6 +937,9 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { let profile = if builder.release { "release" } else { "dev" }; let mut cargo = Command::new("cargo"); + if let Some(toolchain) = &builder.toolchain_overwrite { + cargo.arg(format!("+{}", toolchain)); + } cargo.args([ "build", "--lib", From dcbdc524c0a5c51cd32eee9a7c0078186d683bff Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 20:06:53 +0200 Subject: [PATCH 19/26] compile_codegen: include_str! all target_spec jsons for cargo gpu --- crates/spirv-builder/src/lib.rs | 2 + crates/spirv-builder/src/target_specs.rs | 63 ++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 crates/spirv-builder/src/target_specs.rs diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 83d7f790c50..f4bb982d28e 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -73,6 +73,7 @@ #![doc = include_str!("../README.md")] mod depfile; +mod target_specs; #[cfg(feature = "watch")] mod watch; @@ -89,6 +90,7 @@ use thiserror::Error; pub use rustc_codegen_spirv_types::Capability; pub use rustc_codegen_spirv_types::{CompileResult, ModuleResult}; +pub use target_specs::TARGET_SPECS; #[derive(Debug, Error)] #[non_exhaustive] diff --git a/crates/spirv-builder/src/target_specs.rs b/crates/spirv-builder/src/target_specs.rs new file mode 100644 index 00000000000..7b52d6da3db --- /dev/null +++ b/crates/spirv-builder/src/target_specs.rs @@ -0,0 +1,63 @@ +/// Metadata for the compile targets supported by `rust-gpu` +pub const TARGET_SPECS: &[(&str, &str)] = &[ + ( + "spirv-unknown-opengl4.0.json", + include_str!("../target-specs/spirv-unknown-opengl4.0.json"), + ), + ( + "spirv-unknown-opengl4.1.json", + include_str!("../target-specs/spirv-unknown-opengl4.1.json"), + ), + ( + "spirv-unknown-opengl4.2.json", + include_str!("../target-specs/spirv-unknown-opengl4.2.json"), + ), + ( + "spirv-unknown-opengl4.3.json", + include_str!("../target-specs/spirv-unknown-opengl4.3.json"), + ), + ( + "spirv-unknown-opengl4.5.json", + include_str!("../target-specs/spirv-unknown-opengl4.5.json"), + ), + ( + "spirv-unknown-spv1.0.json", + include_str!("../target-specs/spirv-unknown-spv1.0.json"), + ), + ( + "spirv-unknown-spv1.1.json", + include_str!("../target-specs/spirv-unknown-spv1.1.json"), + ), + ( + "spirv-unknown-spv1.2.json", + include_str!("../target-specs/spirv-unknown-spv1.2.json"), + ), + ( + "spirv-unknown-spv1.3.json", + include_str!("../target-specs/spirv-unknown-spv1.3.json"), + ), + ( + "spirv-unknown-spv1.4.json", + include_str!("../target-specs/spirv-unknown-spv1.4.json"), + ), + ( + "spirv-unknown-spv1.5.json", + include_str!("../target-specs/spirv-unknown-spv1.5.json"), + ), + ( + "spirv-unknown-vulkan1.0.json", + include_str!("../target-specs/spirv-unknown-vulkan1.0.json"), + ), + ( + "spirv-unknown-vulkan1.1.json", + include_str!("../target-specs/spirv-unknown-vulkan1.1.json"), + ), + ( + "spirv-unknown-vulkan1.1spv1.4.json", + include_str!("../target-specs/spirv-unknown-vulkan1.1spv1.4.json"), + ), + ( + "spirv-unknown-vulkan1.2.json", + include_str!("../target-specs/spirv-unknown-vulkan1.2.json"), + ), +]; From e040ab9c296f2c0f1555051d4b574969329715ec Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 20:35:16 +0200 Subject: [PATCH 20/26] compile_codegen: SpirvBuilder: remove RUSTC env var before calling cargo --- crates/spirv-builder/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index f4bb982d28e..ea558f797d1 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -999,6 +999,12 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // so we turn off that caching with an env var, just to avoid any issues. cargo.env("CARGO_CACHE_RUSTC_INFO", "0"); + // NOTE(firestar99) If you call SpirvBuilder in a build script, it will + // set `RUSTC` before calling it. And if we were to propagate it to our + // cargo invocation, it will take precedence over the `+toolchain` we + // previously set. + cargo.env_remove("RUSTC"); + // 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. From b8f659beb33be8d1b01fbd4685b570f4b6bf432a Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 21:42:42 +0200 Subject: [PATCH 21/26] compile_codegen: SpirvBuilder: only use target_spec json when compiler is new enough --- Cargo.lock | 4 +++ crates/spirv-builder/Cargo.toml | 1 + crates/spirv-builder/src/lib.rs | 51 ++++++++++++++++++++++++++++----- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0c366481d7f..eb177ffe93e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2485,6 +2485,9 @@ name = "semver" version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3cb6eb87a131f756572d7fb904f6e7b68633f09cca868c5df1c4b8d1a694bbba" +dependencies = [ + "serde", +] [[package]] name = "serde" @@ -2659,6 +2662,7 @@ dependencies = [ "raw-string", "rustc_codegen_spirv", "rustc_codegen_spirv-types", + "semver", "serde", "serde_json", "thiserror 2.0.12", diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index 1b3084da725..545085c114e 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -43,6 +43,7 @@ raw-string = "0.3.5" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "2.0.12" +semver = { version = "1.0.24", features = ["serde"] } notify = { version = "7.0", optional = true } # Pinning clap, as newer versions have raised min rustc version without being marked a breaking change diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index ea558f797d1..2f3c7aa0bc6 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -78,6 +78,7 @@ mod target_specs; mod watch; use raw_string::{RawStr, RawString}; +use semver::Version; use serde::Deserialize; use std::borrow::Borrow; use std::collections::HashMap; @@ -403,6 +404,9 @@ pub struct SpirvBuilder { // Overwrite the toolchain like `cargo +nightly` #[cfg_attr(feature = "clap", clap(skip))] pub toolchain_overwrite: Option, + // Set the rustc version of the toolchain, used to adjust params to support older toolchains + #[cfg_attr(feature = "clap", clap(skip))] + pub toolchain_rustc_version: Option, /// The path of the "target specification" file. /// @@ -460,6 +464,7 @@ impl Default for SpirvBuilder { path_to_target_spec: None, target_dir_path: None, toolchain_overwrite: None, + toolchain_rustc_version: None, shader_panic_strategy: ShaderPanicStrategy::default(), validator: ValidatorOptions::default(), optimizer: OptimizerOptions::default(), @@ -781,6 +786,13 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { } } + let toolchain_rustc_version = + if let Some(toolchain_rustc_version) = &builder.toolchain_rustc_version { + toolchain_rustc_version.clone() + } else { + query_rustc_version(builder.toolchain_overwrite.as_deref())? + }; + // Okay, this is a little bonkers: in a normal world, we'd have the user clone // rustc_codegen_spirv and pass in the path to it, and then we'd invoke cargo to build it, grab // the resulting .so, and pass it into -Z codegen-backend. But that's really gross: the user @@ -958,13 +970,21 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // FIXME(eddyb) consider moving `target-specs` into `rustc_codegen_spirv_types`. // FIXME(eddyb) consider the `RUST_TARGET_PATH` env var alternative. - cargo - .arg("--target") - .arg(builder.path_to_target_spec.clone().unwrap_or_else(|| { - PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("target-specs") - .join(format!("{}.json", target)) - })); + + // NOTE(firestar99) rustc 1.76 has been tested to correctly parse modern + // target_spec jsons, some later version requires them, some earlier + // version fails with them (notably our 0.9.0 release) + if toolchain_rustc_version >= Version::new(1, 76, 0) { + cargo + .arg("--target") + .arg(builder.path_to_target_spec.clone().unwrap_or_else(|| { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("target-specs") + .join(format!("{}.json", target)) + })); + } else { + cargo.arg("--target").arg(target); + } if !builder.shader_crate_features.default_features { cargo.arg("--no-default-features"); @@ -1108,3 +1128,20 @@ fn leaf_deps(artifact: &Path, mut handle: impl FnMut(&RawStr)) -> std::io::Resul recurse(&deps_map, artifact.to_str().unwrap().into(), &mut handle); Ok(()) } + +pub fn query_rustc_version(toolchain: Option<&str>) -> std::io::Result { + let mut cmd = Command::new("rustc"); + if let Some(toolchain) = toolchain { + cmd.arg(format!("+{}", toolchain)); + } + cmd.arg("--version"); + + let parse = |stdout| { + let output = String::from_utf8(stdout).ok()?; + let output = output.strip_prefix("rustc ")?; + let version = &output[..output.find("-")?]; + Some(Version::parse(version).expect("invalid version")) + }; + + Ok(parse(cmd.output()?.stdout).expect("rustc --version parsing failed")) +} From eca83dfbfc0aae5c72f1e3f53081f5ccb72c27a4 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 30 Apr 2025 23:08:17 +0200 Subject: [PATCH 22/26] compile_codegen: SpirvBuilder: fix `query_rustc_version()` for stable rustc versions --- crates/spirv-builder/src/lib.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 2f3c7aa0bc6..db1b92f69de 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -1135,13 +1135,14 @@ pub fn query_rustc_version(toolchain: Option<&str>) -> std::io::Result cmd.arg(format!("+{}", toolchain)); } cmd.arg("--version"); + let output = cmd.output()?; - let parse = |stdout| { - let output = String::from_utf8(stdout).ok()?; + let stdout = String::from_utf8(output.stdout).expect("stdout must be utf-8"); + let parse = |output: &str| { let output = output.strip_prefix("rustc ")?; - let version = &output[..output.find("-")?]; - Some(Version::parse(version).expect("invalid version")) + let version = &output[..output.find(|c| !"0123456789.".contains(c))?]; + Version::parse(version).ok() }; - - Ok(parse(cmd.output()?.stdout).expect("rustc --version parsing failed")) + Ok(parse(&stdout) + .unwrap_or_else(|| panic!("failed parsing `rustc --version` output `{}`", stdout))) } From 5db761ecbe700a3e2be142c1f95e0b43ecbc4441 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Thu, 1 May 2025 13:35:32 +0200 Subject: [PATCH 23/26] compile_codegen: change default MetadataPrintout to None --- crates/spirv-builder/src/lib.rs | 2 +- examples/runners/wgpu/builder/src/main.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index db1b92f69de..ea017ed526a 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -130,13 +130,13 @@ const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; #[cfg_attr(feature = "clap", derive(clap::ValueEnum))] pub enum MetadataPrintout { /// Print no cargo metadata. + #[default] None, /// Print only dependency information (eg for multiple modules). DependencyOnly, /// Print all cargo metadata. /// /// Includes dependency information and spirv environment variable. - #[default] Full, } diff --git a/examples/runners/wgpu/builder/src/main.rs b/examples/runners/wgpu/builder/src/main.rs index bee3f074d00..7fb9735381c 100644 --- a/examples/runners/wgpu/builder/src/main.rs +++ b/examples/runners/wgpu/builder/src/main.rs @@ -1,4 +1,4 @@ -use spirv_builder::SpirvBuilder; +use spirv_builder::{MetadataPrintout, SpirvBuilder}; use std::env; use std::error::Error; use std::fs; @@ -7,7 +7,9 @@ use std::path::Path; fn build_shader(path_to_crate: &str, codegen_names: bool) -> Result<(), Box> { let builder_dir = &Path::new(env!("CARGO_MANIFEST_DIR")); let path_to_crate = builder_dir.join(path_to_crate); - let result = SpirvBuilder::new(path_to_crate, "spirv-unknown-vulkan1.1").build()?; + let result = SpirvBuilder::new(path_to_crate, "spirv-unknown-vulkan1.1") + .print_metadata(MetadataPrintout::Full) + .build()?; if codegen_names { let out_dir = env::var_os("OUT_DIR").unwrap(); let dest_path = Path::new(&out_dir).join("entry_points.rs"); From 3c9a7d7d0ab908b4d30dbe47bc9c4c9ee350bde5 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Thu, 1 May 2025 13:23:47 +0200 Subject: [PATCH 24/26] compile_codegen: refactor watcher to call on_compilation_finishes() even on first completion, code cleanup --- crates/spirv-builder/src/watch.rs | 147 +++++++++++++++--------------- 1 file changed, 74 insertions(+), 73 deletions(-) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index fd1e16d956c..bbf8ae15c02 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -1,20 +1,18 @@ -use std::{collections::HashSet, sync::mpsc::sync_channel}; - -use notify::{Event, RecursiveMode, Watcher}; -use rustc_codegen_spirv_types::CompileResult; - use crate::{SpirvBuilder, SpirvBuilderError, leaf_deps}; +use notify::{Event, RecommendedWatcher, RecursiveMode, Watcher as _}; +use rustc_codegen_spirv_types::CompileResult; +use std::path::{Path, PathBuf}; +use std::sync::mpsc::Receiver; +use std::{collections::HashSet, sync::mpsc::sync_channel}; impl SpirvBuilder { /// Watches the module for changes using [`notify`](https://crates.io/crates/notify), - /// and rebuild it upon changes - /// - /// Returns the result of the first successful compilation, then calls - /// `on_compilation_finishes` for each subsequent compilation. + /// and rebuild it upon changes. Calls `on_compilation_finishes` after each + /// successful compilation. pub fn watch( - self, + &self, mut on_compilation_finishes: impl FnMut(CompileResult) + Send + 'static, - ) -> Result { + ) -> Result<(), SpirvBuilderError> { let path_to_crate = self .path_to_crate .as_ref() @@ -22,93 +20,96 @@ impl SpirvBuilder { if !matches!(self.print_metadata, crate::MetadataPrintout::None) { return Err(SpirvBuilderError::WatchWithPrintMetadata); } - let metadata_result = crate::invoke_rustc(&self); + + let metadata_result = crate::invoke_rustc(self); // Load the dependencies of the thing let metadata_file = if let Ok(path) = metadata_result { path } else { - let (tx, rx) = sync_channel(0); // Fall back to watching from the crate root if the initial compilation fails - let mut watcher = - notify::recommended_watcher(move |event: notify::Result| match event { - Ok(e) => match e.kind { - notify::EventKind::Access(_) => (), - notify::EventKind::Any - | notify::EventKind::Create(_) - | notify::EventKind::Modify(_) - | notify::EventKind::Remove(_) - | notify::EventKind::Other => { - let _ = tx.try_send(()); - } - }, - Err(e) => println!("notify error: {e:?}"), - }) - .expect("Could create watcher"); // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, + let mut watcher = Watcher::new(); watcher + .watcher .watch(path_to_crate, RecursiveMode::Recursive) .expect("Could watch crate root"); loop { - rx.recv().expect("Watcher still alive"); - let metadata_file = crate::invoke_rustc(&self); + watcher.recv(); + let metadata_file = crate::invoke_rustc(self); if let Ok(f) = metadata_file { break f; } } }; let metadata = self.parse_metadata_file(&metadata_file)?; - let first_result = metadata; + on_compilation_finishes(metadata); + let builder = self.clone(); let thread = std::thread::spawn(move || { - let mut watched_paths = HashSet::new(); - let (tx, rx) = sync_channel(0); - let mut watcher = - notify::recommended_watcher(move |event: notify::Result| match event { - Ok(e) => match e.kind { - notify::EventKind::Access(_) => (), - notify::EventKind::Any - | notify::EventKind::Create(_) - | notify::EventKind::Modify(_) - | notify::EventKind::Remove(_) - | notify::EventKind::Other => { - let _ = tx.try_send(()); - } - }, - Err(e) => println!("notify error: {e:?}"), - }) - .expect("Could create watcher"); - leaf_deps(&metadata_file, |it| { - let path = it.to_path().unwrap(); - if watched_paths.insert(path.to_owned()) { - watcher - .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) - .expect("Cargo dependencies are valid files"); - } - }) - .expect("Could read dependencies file"); + let mut watcher = Watcher::new(); + watcher.watch_leaf_deps(&metadata_file); + loop { - rx.recv().expect("Watcher still alive"); - let metadata_result = crate::invoke_rustc(&self); + watcher.recv(); + let metadata_result = crate::invoke_rustc(&builder); if let Ok(file) = metadata_result { - let metadata = self + let metadata = builder .parse_metadata_file(&file) .expect("Metadata file is correct"); - - leaf_deps(&file, |it| { - let path = it.to_path().unwrap(); - if watched_paths.insert(path.to_owned()) { - watcher - .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) - .expect("Cargo dependencies are valid files"); - } - }) - .expect("Could read dependencies file"); - + watcher.watch_leaf_deps(&metadata_file); on_compilation_finishes(metadata); } } }); - std::mem::drop(thread); - Ok(first_result) + drop(thread); + Ok(()) + } +} + +struct Watcher { + watcher: RecommendedWatcher, + rx: Receiver<()>, + watched_paths: HashSet, +} + +impl Watcher { + fn new() -> Self { + let (tx, rx) = sync_channel(0); + let watcher = + notify::recommended_watcher(move |event: notify::Result| match event { + Ok(e) => match e.kind { + notify::EventKind::Access(_) => (), + notify::EventKind::Any + | notify::EventKind::Create(_) + | notify::EventKind::Modify(_) + | notify::EventKind::Remove(_) + | notify::EventKind::Other => { + let _ = tx.try_send(()); + } + }, + Err(e) => println!("notify error: {e:?}"), + }) + .expect("Could create watcher"); + Self { + watcher, + rx, + watched_paths: HashSet::new(), + } + } + + fn watch_leaf_deps(&mut self, metadata_file: &Path) { + leaf_deps(metadata_file, |it| { + let path = it.to_path().unwrap(); + if self.watched_paths.insert(path.to_owned()) { + self.watcher + .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) + .expect("Cargo dependencies are valid files"); + } + }) + .expect("Could read dependencies file"); + } + + fn recv(&self) { + self.rx.recv().expect("Watcher still alive"); } } From 74d1c66ceb855f60a995621652c5158a474cb2ee Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Thu, 1 May 2025 15:15:43 +0200 Subject: [PATCH 25/26] compile_codegen: allow SpirvBuilder watch to return something on first completion --- crates/spirv-builder/src/watch.rs | 31 ++++++++++++++++++++++++------- examples/runners/wgpu/src/lib.rs | 25 +++++++++++++++++-------- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index bbf8ae15c02..a1f68d62f39 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -8,11 +8,15 @@ use std::{collections::HashSet, sync::mpsc::sync_channel}; impl SpirvBuilder { /// Watches the module for changes using [`notify`](https://crates.io/crates/notify), /// and rebuild it upon changes. Calls `on_compilation_finishes` after each - /// successful compilation. - pub fn watch( + /// successful compilation. The second `Option>` + /// param allows you to return some `T` on the first compile, which is + /// then returned by this function (wrapped in Option). + pub fn watch( &self, - mut on_compilation_finishes: impl FnMut(CompileResult) + Send + 'static, - ) -> Result<(), SpirvBuilderError> { + mut on_compilation_finishes: impl FnMut(CompileResult, Option>) + + Send + + 'static, + ) -> Result, SpirvBuilderError> { let path_to_crate = self .path_to_crate .as_ref() @@ -42,7 +46,8 @@ impl SpirvBuilder { } }; let metadata = self.parse_metadata_file(&metadata_file)?; - on_compilation_finishes(metadata); + let mut out = None; + on_compilation_finishes(metadata, Some(AcceptFirstCompile(&mut out))); let builder = self.clone(); let thread = std::thread::spawn(move || { @@ -57,12 +62,24 @@ impl SpirvBuilder { .parse_metadata_file(&file) .expect("Metadata file is correct"); watcher.watch_leaf_deps(&metadata_file); - on_compilation_finishes(metadata); + on_compilation_finishes(metadata, None); } } }); drop(thread); - Ok(()) + Ok(out) + } +} + +pub struct AcceptFirstCompile<'a, T>(&'a mut Option); + +impl<'a, T> AcceptFirstCompile<'a, T> { + pub fn new(write: &'a mut Option) -> Self { + Self(write) + } + + pub fn submit(self, t: T) { + *self.0 = Some(t); } } diff --git a/examples/runners/wgpu/src/lib.rs b/examples/runners/wgpu/src/lib.rs index 82d0e758fb5..11cba07c39d 100644 --- a/examples/runners/wgpu/src/lib.rs +++ b/examples/runners/wgpu/src/lib.rs @@ -164,13 +164,7 @@ fn maybe_watch( // HACK(eddyb) needed because of `debugPrintf` instrumentation limitations // (see https://github.com/KhronosGroup/SPIRV-Tools/issues/4892). .multimodule(has_debug_printf); - let initial_result = if let Some(mut f) = on_watch { - builder - .watch(move |compile_result| f(handle_compile_result(compile_result))) - .expect("Configuration is correct for watching") - } else { - builder.build().unwrap() - }; + fn handle_compile_result(compile_result: CompileResult) -> CompiledShaderModules { let load_spv_module = |path| { let data = std::fs::read(path).unwrap(); @@ -194,7 +188,22 @@ fn maybe_watch( }, } } - handle_compile_result(initial_result) + + if let Some(mut f) = on_watch { + builder + .watch(move |compile_result, accept| { + let modules = handle_compile_result(compile_result); + if let Some(accept) = accept { + accept.submit(modules); + } else { + f(modules); + } + }) + .expect("Configuration is correct for watching") + .unwrap() + } else { + handle_compile_result(builder.build().unwrap()) + } } #[cfg(any(target_os = "android", target_arch = "wasm32"))] { From d8f03982a5b23510b13c337683ff6e6b646d1a5f Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Thu, 1 May 2025 15:43:45 +0200 Subject: [PATCH 26/26] compile_codegen: fix typo --- crates/rustc_codegen_spirv/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rustc_codegen_spirv/build.rs b/crates/rustc_codegen_spirv/build.rs index 661b840de40..c7cafd699f9 100644 --- a/crates/rustc_codegen_spirv/build.rs +++ b/crates/rustc_codegen_spirv/build.rs @@ -11,7 +11,7 @@ use std::process::{Command, ExitCode}; use std::{env, fs, mem}; /// Current `rust-toolchain.toml` file -/// WARNING!!! cargo-gpu is new relying on this being a string literal! It will +/// WARNING!!! cargo-gpu is now relying on this being a string literal! It will /// scan `build.rs` for any line starting with `channel = "..."` to figure out /// which toolchain version to use! This also allows backwards compat. /// Unfortunately, directly including the actual workspace `rust-toolchain.toml` doesn't work together with