[DO NOT MERGE] Switch to Rust version of spirv-tools

Cargo.toml

@@ -46,7 +46,7 @@ spirv-builder = { path = "./crates/spirv-builder", version = "=0.9.0", default-f
 spirv-std = { path = "./crates/spirv-std", version = "=0.9.0" }
 spirv-std-types = { path = "./crates/spirv-std/shared", version = "=0.9.0" }
 spirv-std-macros = { path = "./crates/spirv-std/macros", version = "=0.9.0" }
-spirv-tools = { version = "0.13.0", default-features = false }
+spirv-tools = { git = "https://github.com/Rust-GPU/SPIRV-Tools", default-features = false }
 rustc_codegen_spirv = { path = "./crates/rustc_codegen_spirv", version = "=0.9.0", default-features = false }
 rustc_codegen_spirv-types = { path = "./crates/rustc_codegen_spirv-types", version = "=0.9.0" }
 
@@ -61,6 +61,12 @@ glam = { version = ">=0.30.8", default-features = false }
 libm = { version = "0.2.5", default-features = false }
 bytemuck = { version = "1.23", features = ["derive"] }
 
+# Patch rspirv/spirv to use newer versions with additional SPIR-V opcodes needed by spirv-tools.
+# TODO: Once a newer rspirv is published that includes these opcodes, switch to a crates.io version.
+[patch.crates-io]
+rspirv = { git = "https://github.com/gfx-rs/rspirv", branch = "sdk-update" }
+spirv = { git = "https://github.com/gfx-rs/rspirv", branch = "sdk-update", package = "spirv" }
+
 # Enable incremental by default in release mode.
 [profile.release]
 incremental = true

crates/rustc_codegen_spirv/Cargo.toml

@@ -12,20 +12,11 @@ repository.workspace = true
 crate-type = ["dylib"]
 
 [features]
-# By default, the use-compiled-tools is enabled, as doesn't require additional
-# setup steps for the user. This does however mean that you will need to disable
-# default features and explicitly enable `use-installed-tools` if you are using
-# this in an environment with spirv-tools in PATH, and you don't want to take
-# the compile time cost
-default = ["use-compiled-tools"]
-# If enabled, uses spirv-tools binaries installed in PATH, instead of
-# compiling and linking the spirv-tools C++ code
-use-installed-tools = ["spirv-tools/use-installed-tools"]
-# If enabled will compile and link the C++ code for the spirv tools, the compiled
-# version is preferred if both this and `use-installed-tools` are enabled
-use-compiled-tools = ["spirv-tools/use-compiled-tools"]
+# These features are no-ops kept for compatibility with existing build configurations.
+use-installed-tools = []
+use-compiled-tools = []
 # If enabled, this will not check whether the current rustc version is set to the
-# appropriate channel. rustc_cogeden_spirv requires a specific nightly version,
+# appropriate channel. rustc_codegen_spirv requires a specific nightly version,
 # and will likely produce compile errors when built against a different toolchain.
 # Enable this feature to be able to experiment with other versions.
 skip-toolchain-check = []

crates/rustc_codegen_spirv/src/builder/builder_methods.rs

@@ -341,7 +341,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn ordering_to_semantics_def(&mut self, ordering: AtomicOrdering) -> SpirvValue {
         let mut invalid_seq_cst = false;
         let semantics = match ordering {
-            AtomicOrdering::Relaxed => MemorySemantics::NONE,
+            AtomicOrdering::Relaxed => MemorySemantics::RELAXED,
             AtomicOrdering::Acquire => MemorySemantics::MAKE_VISIBLE | MemorySemantics::ACQUIRE,
             AtomicOrdering::Release => MemorySemantics::MAKE_AVAILABLE | MemorySemantics::RELEASE,
             AtomicOrdering::AcqRel => {

crates/rustc_codegen_spirv/src/builder/spirv_asm.rs

@@ -32,9 +32,23 @@ pub struct InstructionTable {
 
 impl InstructionTable {
     pub fn new() -> Self {
-        let table = rspirv::grammar::CoreInstructionTable::iter()
+        let mut table: FxHashMap<_, _> = rspirv::grammar::CoreInstructionTable::iter()
             .map(|inst| (inst.opname, inst))
             .collect();
+
+        // Add aliases for EXT/KHR instructions whose suffixes were removed
+        for inst in rspirv::grammar::CoreInstructionTable::iter() {
+            match inst.opname {
+                "DemoteToHelperInvocation" => {
+                    table.insert("DemoteToHelperInvocationEXT", inst);
+                }
+                "IsHelperInvocation" => {
+                    table.insert("IsHelperInvocationEXT", inst);
+                }
+                _ => {}
+            }
+        }
+
         Self { table }
     }
 }
@@ -1529,6 +1543,38 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> {
                 Ok(x) => inst.operands.push(dr::Operand::StoreCacheControl(x)),
                 Err(()) => self.err(format!("unknown StoreCacheControl {word}")),
             },
+            // New operand kinds added in newer SPIR-V versions - not yet supported in inline asm
+            (OperandKind::RawAccessChainOperands, Some(word)) => {
+                self.err(format!("RawAccessChainOperands not yet supported: {word}"));
+            }
+            (OperandKind::CooperativeMatrixReduce, Some(word)) => {
+                self.err(format!("CooperativeMatrixReduce not yet supported: {word}"));
+            }
+            (OperandKind::TensorClampMode, Some(word)) => {
+                self.err(format!("TensorClampMode not yet supported: {word}"));
+            }
+            (OperandKind::TensorAddressingOperands, Some(word)) => {
+                self.err(format!(
+                    "TensorAddressingOperands not yet supported: {word}"
+                ));
+            }
+            (OperandKind::FPEncoding, Some(word)) => {
+                self.err(format!("FPEncoding not yet supported: {word}"));
+            }
+            (OperandKind::NamedMaximumNumberOfRegisters, Some(word)) => {
+                self.err(format!(
+                    "NamedMaximumNumberOfRegisters not yet supported: {word}"
+                ));
+            }
+            (OperandKind::MatrixMultiplyAccumulateOperands, Some(word)) => {
+                self.err(format!(
+                    "MatrixMultiplyAccumulateOperands not yet supported: {word}"
+                ));
+            }
+            // Catch-all for any other new operand kinds
+            (kind, Some(word)) => {
+                self.err(format!("unsupported operand kind {kind:?}: {word}"));
+            }
             (kind, None) => match token {
                 Token::Word(_) => bug!(),
                 Token::String(_) => {
@@ -1563,16 +1609,13 @@ pub const IMAGE_OPERANDS: &[(&str, ImageOperands)] = &[
     ("Sample", ImageOperands::SAMPLE),
     ("MinLod", ImageOperands::MIN_LOD),
     ("MakeTexelAvailable", ImageOperands::MAKE_TEXEL_AVAILABLE),
-    (
-        "MakeTexelAvailableKHR",
-        ImageOperands::MAKE_TEXEL_AVAILABLE_KHR,
-    ),
+    ("MakeTexelAvailableKHR", ImageOperands::MAKE_TEXEL_AVAILABLE),
     ("MakeTexelVisible", ImageOperands::MAKE_TEXEL_VISIBLE),
-    ("MakeTexelVisibleKHR", ImageOperands::MAKE_TEXEL_VISIBLE_KHR),
+    ("MakeTexelVisibleKHR", ImageOperands::MAKE_TEXEL_VISIBLE),
     ("NonPrivateTexel", ImageOperands::NON_PRIVATE_TEXEL),
-    ("NonPrivateTexelKHR", ImageOperands::NON_PRIVATE_TEXEL_KHR),
+    ("NonPrivateTexelKHR", ImageOperands::NON_PRIVATE_TEXEL),
     ("VolatileTexel", ImageOperands::VOLATILE_TEXEL),
-    ("VolatileTexelKHR", ImageOperands::VOLATILE_TEXEL_KHR),
+    ("VolatileTexelKHR", ImageOperands::VOLATILE_TEXEL),
     ("SignExtend", ImageOperands::SIGN_EXTEND),
     ("ZeroExtend", ImageOperands::ZERO_EXTEND),
 ];
@@ -1610,7 +1653,7 @@ pub const FUNCTION_CONTROL: &[(&str, FunctionControl)] = &[
 ];
 pub const MEMORY_SEMANTICS: &[(&str, MemorySemantics)] = &[
     ("Relaxed", MemorySemantics::RELAXED),
-    ("None", MemorySemantics::NONE),
+    ("None", MemorySemantics::RELAXED),
     ("Acquire", MemorySemantics::ACQUIRE),
     ("Release", MemorySemantics::RELEASE),
     ("AcquireRelease", MemorySemantics::ACQUIRE_RELEASE),
@@ -1631,11 +1674,11 @@ pub const MEMORY_SEMANTICS: &[(&str, MemorySemantics)] = &[
     ),
     ("ImageMemory", MemorySemantics::IMAGE_MEMORY),
     ("OutputMemory", MemorySemantics::OUTPUT_MEMORY),
-    ("OutputMemoryKHR", MemorySemantics::OUTPUT_MEMORY_KHR),
+    ("OutputMemoryKHR", MemorySemantics::OUTPUT_MEMORY),
     ("MakeAvailable", MemorySemantics::MAKE_AVAILABLE),
-    ("MakeAvailableKHR", MemorySemantics::MAKE_AVAILABLE_KHR),
+    ("MakeAvailableKHR", MemorySemantics::MAKE_AVAILABLE),
     ("MakeVisible", MemorySemantics::MAKE_VISIBLE),
-    ("MakeVisibleKHR", MemorySemantics::MAKE_VISIBLE_KHR),
+    ("MakeVisibleKHR", MemorySemantics::MAKE_VISIBLE),
     ("Volatile", MemorySemantics::VOLATILE),
 ];
 pub const MEMORY_ACCESS: &[(&str, MemoryAccess)] = &[
@@ -1646,18 +1689,12 @@ pub const MEMORY_ACCESS: &[(&str, MemoryAccess)] = &[
     ("MakePointerAvailable", MemoryAccess::MAKE_POINTER_AVAILABLE),
     (
         "MakePointerAvailableKHR",
-        MemoryAccess::MAKE_POINTER_AVAILABLE_KHR,
+        MemoryAccess::MAKE_POINTER_AVAILABLE,
     ),
     ("MakePointerVisible", MemoryAccess::MAKE_POINTER_VISIBLE),
-    (
-        "MakePointerVisibleKHR",
-        MemoryAccess::MAKE_POINTER_VISIBLE_KHR,
-    ),
+    ("MakePointerVisibleKHR", MemoryAccess::MAKE_POINTER_VISIBLE),
     ("NonPrivatePointer", MemoryAccess::NON_PRIVATE_POINTER),
-    (
-        "NonPrivatePointerKHR",
-        MemoryAccess::NON_PRIVATE_POINTER_KHR,
-    ),
+    ("NonPrivatePointerKHR", MemoryAccess::NON_PRIVATE_POINTER),
 ];
 pub const KERNEL_PROFILING_INFO: &[(&str, KernelProfilingInfo)] = &[
     ("None", KernelProfilingInfo::NONE),

crates/rustc_codegen_spirv/src/codegen_cx/entry.rs

@@ -6,6 +6,7 @@ use crate::abi::ConvSpirvType;
 use crate::attr::{AggregatedSpirvAttributes, Entry, Spanned, SpecConstant};
 use crate::builder::Builder;
 use crate::builder_spirv::{SpirvFunctionCursor, SpirvValue, SpirvValueExt};
+use crate::custom_decorations::{CustomDecoration, SrcLocDecoration};
 use crate::spirv_type::SpirvType;
 use rspirv::dr::Operand;
 use rspirv::spirv::{
@@ -987,6 +988,14 @@ impl<'tcx> CodegenCx<'tcx> {
                 self.emit_global()
                     .variable(var_ptr_spirv_type, Some(var), storage_class, None);
 
+                // Add SrcLocDecoration to the variable for rich error messages.
+                let src_loc_inst = SrcLocDecoration::from_rustc_span(hir_param.span, &self.builder)
+                    .map(|src_loc| src_loc.encode_to_inst(var));
+                self.emit_global()
+                    .module_mut()
+                    .annotations
+                    .extend(src_loc_inst);
+
                 // Record this `OpVariable` as needing to be added (if applicable),
                 // to the *Interface* operands of the `OpEntryPoint` instruction.
                 if self.emit_global().version().unwrap() > (1, 3) {

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

@@ -313,6 +313,10 @@ pub struct CodegenArgs {
 
     pub run_spirv_opt: bool,
 
+    /// Preserve internal debug decorations (`SrcLocDecoration`, `ZombieDecoration`) in the output.
+    /// These are normally stripped after validation. Enable for debugging or testing.
+    pub preserve_debug_decorations: bool,
+
     /// All options pertinent to `rustc_codegen_spirv::linker` specifically.
     //
     // FIXME(eddyb) should these be handled as `-C linker-args="..."` instead?
@@ -384,6 +388,12 @@ impl CodegenArgs {
             "disables running spirv-opt on the final output",
         );
 
+        opts.optflag(
+            "",
+            "preserve-debug-decorations",
+            "preserve internal debug decorations (SrcLocDecoration, ZombieDecoration) in the output",
+        );
+
         opts.optflag(
             "",
             "preserve-bindings",
@@ -652,6 +662,8 @@ impl CodegenArgs {
 
             run_spirv_opt,
 
+            preserve_debug_decorations: matches.opt_present("preserve-debug-decorations"),
+
             linker_opts,
 
             // NOTE(eddyb) these are debugging options that used to be env vars

crates/rustc_codegen_spirv/src/custom_decorations.rs

@@ -18,7 +18,7 @@ use std::{fmt, iter, slice, str};
 
 /// Decorations not native to SPIR-V require some form of encoding into existing
 /// SPIR-V constructs, for which we use `OpDecorateString` with decoration type
-/// `UserTypeGOOGLE` and some encoded Rust value as the decoration string.
+/// `UserSemantic` and some encoded Rust value as the decoration string.
 ///
 /// Each decoration type has to implement this trait, and use a different
 /// `ENCODING_PREFIX` from any other decoration type, to disambiguate them.
@@ -27,7 +27,10 @@ use std::{fmt, iter, slice, str};
 /// ideally as soon as they're no longer needed, because no other tools
 /// processing the SPIR-V would understand them correctly.
 ///
-/// TODO: uses `non_semantic` instead of piggybacking off of `UserTypeGOOGLE`
+/// Custom decorations are encoded using `OpDecorateString` with `UserSemantic`,
+/// which is a standard SPIR-V decoration that doesn't require any extension.
+///
+/// TODO: consider using `non_semantic` instead for even cleaner separation
 /// <https://htmlpreview.github.io/?https://github.com/KhronosGroup/SPIRV-Registry/blob/master/extensions/KHR/SPV_KHR_non_semantic_info.html>
 pub trait CustomDecoration<'a>: Sized {
     const ENCODING_PREFIX: &'static str;
@@ -45,15 +48,15 @@ pub trait CustomDecoration<'a>: Sized {
             None,
             vec![
                 Operand::IdRef(id),
-                Operand::Decoration(Decoration::UserTypeGOOGLE),
+                Operand::Decoration(Decoration::UserSemantic),
                 Operand::LiteralString(encoded),
             ],
         )
     }
 
     fn try_decode_from_inst(inst: &Instruction) -> Option<(Word, LazilyDecoded<'_, Self>)> {
         if inst.class.opcode == Op::DecorateString
-            && inst.operands[1].unwrap_decoration() == Decoration::UserTypeGOOGLE
+            && inst.operands[1].unwrap_decoration() == Decoration::UserSemantic
         {
             let id = inst.operands[0].unwrap_id_ref();
             let prefixed_encoded = inst.operands[2].unwrap_literal_string();

crates/rustc_codegen_spirv/src/lib.rs

@@ -138,6 +138,7 @@ mod spirv_type_constraints;
 mod symbols;
 mod target;
 mod target_feature;
+mod validation_err;
 
 use builder::Builder;
 use codegen_cx::CodegenCx;

crates/rustc_codegen_spirv/src/link.rs

@@ -253,8 +253,26 @@ fn post_link_single_module(
     out_filename: &Path,
     dump_prefix: Option<&OsStr>,
 ) {
-    cg_args.do_disassemble(&module);
-    let spv_binary = module.assemble();
+    use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
+
+    // Strip internal debug decorations (SrcLocDecoration, ZombieDecoration) BEFORE
+    // disassembly/validation/optimization unless preserve_debug_decorations is set.
+    // These decorations are only valid on certain targets (variables), but rust-gpu
+    // also places them on functions for span tracking which is invalid SPIR-V.
+    // Keep the original module for span lookup in error messages.
+    let (module_for_output, original_module_for_spans) = if cg_args.preserve_debug_decorations {
+        // When preserving decorations, use the same module for both
+        (module.clone(), module)
+    } else {
+        let mut stripped = module.clone();
+        SrcLocDecoration::remove_all(&mut stripped);
+        ZombieDecoration::remove_all(&mut stripped);
+        (stripped, module)
+    };
+
+    // Disassemble the stripped module (unless preserving decorations)
+    cg_args.do_disassemble(&module_for_output);
+    let spv_binary = module_for_output.assemble();
 
     if let Some(dir) = &cg_args.dump_post_link {
         // FIXME(eddyb) rename `filename` with `file_path` to make this less confusing.
@@ -305,7 +323,13 @@ fn post_link_single_module(
     };
 
     if cg_args.run_spirv_val {
-        do_spirv_val(sess, &spv_binary, out_filename, val_options);
+        do_spirv_val(
+            sess,
+            &spv_binary,
+            &original_module_for_spans,
+            out_filename,
+            val_options,
+        );
     }
 
     {
@@ -393,18 +417,20 @@ fn do_spirv_opt(
 fn do_spirv_val(
     sess: &Session,
     spv_binary: &[u32],
+    original_module_for_spans: &Module,
     filename: &Path,
     options: spirv_tools::val::ValidatorOptions,
 ) {
+    use crate::validation_err::ValidationErrorContext;
     use spirv_tools::val::{self, Validator};
 
     let validator = val::create(sess.target.options.env.parse().ok());
 
     if let Err(e) = validator.validate(spv_binary, Some(options)) {
-        let mut err = sess.dcx().struct_err(e.to_string());
-        err.note("spirv-val failed");
-        err.note(format!("module `{}`", filename.display()));
-        err.emit();
+        // Use the ORIGINAL module (with SrcLocDecoration) to look up source spans
+        // Note: We validate the stripped binary, but look up spans in the original
+        let mut ctx = ValidationErrorContext::new(sess, Some(original_module_for_spans), filename);
+        ctx.emit_error(&e);
     }
 }
 

crates/rustc_codegen_spirv/src/linker/inline.rs

@@ -298,8 +298,8 @@ impl LegalGlobal {
             let global = match inst.class.opcode {
                 Op::TypePointer => Self::TypePointer(inst.operands[0].unwrap_storage_class()),
                 Op::Variable => Self::Variable,
-                op if rspirv::grammar::reflect::is_type(op) => Self::TypeNonPointer,
-                op if rspirv::grammar::reflect::is_constant(op) => Self::Const,
+                op if op.is_type() => Self::TypeNonPointer,
+                op if op.is_constant() => Self::Const,
 
                 // FIXME(eddyb) should this be `unreachable!()`?
                 _ => continue,

crates/rustc_codegen_spirv/src/linker/mod.rs

@@ -19,7 +19,6 @@ mod zombies;
 use std::borrow::Cow;
 
 use crate::codegen_cx::{ModuleOutputType, SpirvMetadata};
-use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
 use crate::custom_insts;
 use either::Either;
 use rspirv::binary::Assemble;
@@ -768,13 +767,10 @@ pub fn link(
             output.header.as_mut().unwrap().bound = simple_passes::compact_ids(output);
         };
 
-        // FIXME(eddyb) convert these into actual `OpLine`s with a SPIR-T pass,
-        // but that'd require keeping the modules in SPIR-T form (once lowered),
-        // and never loading them back into `rspirv` once lifted back to SPIR-V.
-        SrcLocDecoration::remove_all(output);
-
-        // FIXME(eddyb) might make more sense to rewrite these away on SPIR-T.
-        ZombieDecoration::remove_all(output);
+        // NOTE: We keep SrcLocDecoration and ZombieDecoration in the module
+        // so that they can be used for rich error messages during validation.
+        // These are encoded as OpDecorateString with UserSemantic, which is
+        // valid SPIR-V and should not cause validation failures.
     }
 
     Ok(output)