Updated SPIR-T for better pretty-printing (esp. OpExtInst), and started deduplicating custom debuginfo.

CHANGELOG.md

@@ -46,6 +46,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   (see its documentation for more details about each available panic handling strategy)
 
 ### Changed 🛠
+- [PR#1083](https://github.com/EmbarkStudios/rust-gpu/pull/1083) updated SPIR-T to get pretty-printer
+  improvements (especially for `OpExtInst`, including Rust-GPU's custom ones), and started more
+  aggressively deduplicating custom debuginfo instructions (to make SPIR-T dumps more readable)
 - [PR#1079](https://github.com/EmbarkStudios/rust-gpu/pull/1079) revised `spirv-builder`'s `README.md`,
   and added a way for `docs.rs` to be able to build it (via `cargo +stable doc --no-default-features`)
 - [PR#1070](https://github.com/EmbarkStudios/rust-gpu/pull/1070) made panics (via the `abort` intrinsic)

Cargo.toml

@@ -52,3 +52,6 @@ codegen-units = 256
 opt-level = 3
 incremental = true
 codegen-units = 256
+
+[patch.crates-io]
+spirt = { git = "https://github.com/EmbarkStudios/spirt", branch = "main" }

crates/rustc_codegen_spirv/src/builder/builder_methods.rs

@@ -708,6 +708,23 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
                     },
                 );
             }
+
+            // HACK(eddyb) remove the previous instruction if made irrelevant.
+            let mut builder = self.emit();
+            if let (Some(func_idx), Some(block_idx)) =
+                (builder.selected_function(), builder.selected_block())
+            {
+                let block = &mut builder.module_mut().functions[func_idx].blocks[block_idx];
+                match &block.instructions[..] {
+                    [.., a, b]
+                        if a.class.opcode == b.class.opcode
+                            && a.operands[..2] == b.operands[..2] =>
+                    {
+                        block.instructions.remove(block.instructions.len() - 2);
+                    }
+                    _ => {}
+                }
+            }
         } else {
             // We may not always have valid spans.
             // FIXME(eddyb) reduce the sources of this as much as possible.
@@ -2878,11 +2895,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
 
             // HACK(eddyb) redirect any possible panic call to an abort, to avoid
             // needing to materialize `&core::panic::Location` or `format_args!`.
-            self.abort_with_message_and_debug_printf_args(
-                // HACK(eddyb) `|` is an ad-hoc convention of `linker::spirt_passes::controlflow`.
-                format!("panicked|{message}"),
-                debug_printf_args,
-            );
+            self.abort_with_kind_and_message_debug_printf("panic", message, debug_printf_args);
             self.undef(result_type)
         } else if let Some(mode) = buffer_load_intrinsic {
             self.codegen_buffer_load_intrinsic(result_type, args, mode)

crates/rustc_codegen_spirv/src/builder/intrinsics.rs

@@ -339,11 +339,7 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
     }
 
     fn abort(&mut self) {
-        // HACK(eddyb) `|` is an ad-hoc convention of `linker::spirt_passes::controlflow`.
-        self.abort_with_message_and_debug_printf_args(
-            "aborted|intrinsics::abort() called".into(),
-            [],
-        );
+        self.abort_with_kind_and_message_debug_printf("abort", "intrinsics::abort() called", []);
     }
 
     fn assume(&mut self, _val: Self::Value) {
@@ -378,24 +374,31 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
 }
 
 impl Builder<'_, '_> {
-    pub fn abort_with_message_and_debug_printf_args(
+    pub fn abort_with_kind_and_message_debug_printf(
         &mut self,
-        message: String,
-        debug_printf_args: impl IntoIterator<Item = SpirvValue>,
+        kind: &str,
+        message_debug_printf_fmt_str: impl Into<String>,
+        message_debug_printf_args: impl IntoIterator<Item = SpirvValue>,
     ) {
         // FIXME(eddyb) this should be cached more efficiently.
         let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self);
 
         // HACK(eddyb) there is no `abort` or `trap` instruction in SPIR-V,
         // so the best thing we can do is use our own custom instruction.
-        let message_id = self.emit().string(message);
+        let kind_id = self.emit().string(kind);
+        let message_debug_printf_fmt_str_id = self.emit().string(message_debug_printf_fmt_str);
         self.custom_inst(
             void_ty,
             CustomInst::Abort {
-                message: Operand::IdRef(message_id),
-                debug_printf_args: debug_printf_args
+                kind: Operand::IdRef(kind_id),
+                message_debug_printf: [message_debug_printf_fmt_str_id]
                     .into_iter()
-                    .map(|arg| Operand::IdRef(arg.def(self)))
+                    .chain(
+                        message_debug_printf_args
+                            .into_iter()
+                            .map(|arg| arg.def(self)),
+                    )
+                    .map(Operand::IdRef)
                     .collect(),
             },
         );

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

@@ -378,8 +378,13 @@ impl CodegenArgs {
             );
             opts.optflag(
                 "",
-                "spirt-keep-custom-debuginfo-in-dumps",
-                "keep custom debuginfo when dumping SPIR-T (instead of lossily prettifying it)",
+                "spirt-strip-custom-debuginfo-from-dumps",
+                "strip custom debuginfo instructions when dumping SPIR-T",
+            );
+            opts.optflag(
+                "",
+                "spirt-keep-debug-sources-in-dumps",
+                "keep file contents debuginfo when dumping SPIR-T",
             );
             opts.optflag(
                 "",
@@ -548,8 +553,10 @@ impl CodegenArgs {
             dump_post_merge: matches_opt_dump_dir_path("dump-post-merge"),
             dump_post_split: matches_opt_dump_dir_path("dump-post-split"),
             dump_spirt_passes: matches_opt_dump_dir_path("dump-spirt-passes"),
-            spirt_keep_custom_debuginfo_in_dumps: matches
-                .opt_present("spirt-keep-custom-debuginfo-in-dumps"),
+            spirt_strip_custom_debuginfo_from_dumps: matches
+                .opt_present("spirt-strip-custom-debuginfo-from-dumps"),
+            spirt_keep_debug_sources_in_dumps: matches
+                .opt_present("spirt-keep-debug-sources-in-dumps"),
             specializer_debug: matches.opt_present("specializer-debug"),
             specializer_dump_instances: matches_opt_path("specializer-dump-instances"),
             print_all_zombie: matches.opt_present("print-all-zombie"),

crates/rustc_codegen_spirv/src/custom_insts.rs

@@ -15,6 +15,18 @@ use smallvec::SmallVec;
 /// See `CUSTOM_EXT_INST_SET`'s docs for further constraints on the full name.
 pub const CUSTOM_EXT_INST_SET_PREFIX: &str = concat!("Rust.", env!("CARGO_PKG_NAME"), ".");
 
+macro_rules! join_cargo_pkg_version_major_minor_patch {
+    ($sep:literal) => {
+        concat!(
+            env!("CARGO_PKG_VERSION_MAJOR"),
+            $sep,
+            env!("CARGO_PKG_VERSION_MINOR"),
+            $sep,
+            env!("CARGO_PKG_VERSION_PATCH"),
+        )
+    };
+}
+
 lazy_static! {
     /// `OpExtInstImport` "instruction set" name for all Rust-GPU instructions.
     ///
@@ -30,10 +42,6 @@ lazy_static! {
     ///   if the definitions of the custom instructions have changed - this is
     ///   achieved by hashing the `SCHEMA` constant from `def_custom_insts!` below
     pub static ref CUSTOM_EXT_INST_SET: String = {
-        const VER_MAJOR: &str = env!("CARGO_PKG_VERSION_MAJOR");
-        const VER_MINOR: &str = env!("CARGO_PKG_VERSION_MINOR");
-        const VER_PATCH: &str = env!("CARGO_PKG_VERSION_PATCH");
-
         let schema_hash = {
             use rustc_data_structures::stable_hasher::StableHasher;
             use std::hash::Hash;
@@ -43,11 +51,47 @@ lazy_static! {
             let (lo, hi) = hasher.finalize();
             (lo as u128) | ((hi as u128) << 64)
         };
-
-        format!("{CUSTOM_EXT_INST_SET_PREFIX}{VER_MAJOR}_{VER_MINOR}_{VER_PATCH}.{schema_hash:x}")
+        let version = join_cargo_pkg_version_major_minor_patch!("_");
+        format!("{CUSTOM_EXT_INST_SET_PREFIX}{version}.{schema_hash:x}")
     };
 }
 
+pub fn register_to_spirt_context(cx: &spirt::Context) {
+    use spirt::spv::spec::{ExtInstSetDesc, ExtInstSetInstructionDesc};
+    cx.register_custom_ext_inst_set(
+        &CUSTOM_EXT_INST_SET,
+        ExtInstSetDesc {
+            // HACK(eddyb) this is the most compact form I've found, that isn't
+            // outright lossy by omitting "Rust vs Rust-GPU" or the version.
+            short_alias: Some(
+                concat!("Rust-GPU ", join_cargo_pkg_version_major_minor_patch!(".")).into(),
+            ),
+            instructions: SCHEMA
+                .iter()
+                .map(|&(i, name, operand_names)| {
+                    (
+                        i,
+                        ExtInstSetInstructionDesc {
+                            name: name.into(),
+                            operand_names: operand_names
+                                .iter()
+                                .map(|name| {
+                                    name.strip_prefix("..")
+                                        .unwrap_or(name)
+                                        .replace('_', " ")
+                                        .into()
+                                })
+                                .collect(),
+                            is_debuginfo: name.contains("Debug")
+                                || name.contains("InlinedCallFrame"),
+                        },
+                    )
+                })
+                .collect(),
+        },
+    );
+}
+
 macro_rules! def_custom_insts {
     ($($num:literal => $name:ident $({ $($field:ident),+ $(, ..$variadic_field:ident)? $(,)? })?),+ $(,)?) => {
         const SCHEMA: &[(u32, &str, &[&str])] = &[
@@ -151,9 +195,10 @@ def_custom_insts! {
     // users to do `catch_unwind` at the top-level of their shader to handle
     // panics specially (e.g. by appending to a custom buffer, or using some
     // specific color in a fragment shader, to indicate a panic happened).
-    // NOTE(eddyb) the `message` string follows `debugPrintf` rules, with remaining
-    // operands (collected into `debug_printf_args`) being its formatting arguments.
-    4 => Abort { message, ..debug_printf_args },
+    // NOTE(eddyb) `message_debug_printf` operands form a complete `debugPrintf`
+    // invocation (format string followed by inputs) for the "message", while
+    // `kind` only distinguishes broad categories like `"abort"` vs `"panic"`.
+    4 => Abort { kind, ..message_debug_printf },
 }
 
 impl CustomOp {

crates/rustc_codegen_spirv/src/link.rs

@@ -78,6 +78,7 @@ pub fn link(
                         crate_type,
                         &out_filename,
                         codegen_results,
+                        outputs,
                         &disambiguated_crate_name_for_dumps,
                     );
                 }
@@ -123,6 +124,7 @@ fn link_exe(
     crate_type: CrateType,
     out_filename: &Path,
     codegen_results: &CodegenResults,
+    outputs: &OutputFilenames,
     disambiguated_crate_name_for_dumps: &OsStr,
 ) {
     let mut objects = Vec::new();
@@ -152,6 +154,7 @@ fn link_exe(
         &cg_args,
         &objects,
         &rlibs,
+        outputs,
         disambiguated_crate_name_for_dumps,
     );
     let compile_result = match link_result {
@@ -517,6 +520,7 @@ fn do_link(
     cg_args: &CodegenArgs,
     objects: &[PathBuf],
     rlibs: &[PathBuf],
+    outputs: &OutputFilenames,
     disambiguated_crate_name_for_dumps: &OsStr,
 ) -> linker::LinkResult {
     let load_modules_timer = sess.timer("link_load_modules");
@@ -570,6 +574,7 @@ fn do_link(
         sess,
         modules,
         &cg_args.linker_opts,
+        outputs,
         disambiguated_crate_name_for_dumps,
     );