Add custom_insts for our own OpExtInsts, and use it for some debuginfo.

CHANGELOG.md

@@ -29,6 +29,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added ⭐
+- [PR#1064](https://github.com/EmbarkStudios/rust-gpu/pull/1064) added a Rust-GPU-private
+  "extended instruction set" (to allow us to have custom `OpExtInst`s), with the
+  initial custom `OpExtInst`s being used to improve debuginfo source locations
+  (using ranges instead of just the starting position, and tracking inlined calls)
+
 ### Changed 🛠
 - [PR#1038](https://github.com/EmbarkStudios/rust-gpu/pull/1038) relaxed `glam` version requirements (from only `0.22`, to `>=0.22, <=0.24`)
 

crates/rustc_codegen_spirv/src/builder/builder_methods.rs

@@ -1,6 +1,7 @@
 use super::Builder;
 use crate::abi::ConvSpirvType;
 use crate::builder_spirv::{BuilderCursor, SpirvConst, SpirvValue, SpirvValueExt, SpirvValueKind};
+use crate::custom_insts::{CustomInst, CustomOp};
 use crate::rustc_codegen_ssa::traits::BaseTypeMethods;
 use crate::spirv_type::SpirvType;
 use rspirv::dr::{InsertPoint, Instruction, Operand};
@@ -656,15 +657,55 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
     }
 
     fn set_span(&mut self, span: Span) {
-        self.current_span = Some(span);
+        let old_span = self.current_span.replace(span);
+
+        // FIXME(eddyb) enable this once cross-block interactions are figured out
+        // (in particular, every block starts off with no debuginfo active).
+        if false {
+            // Avoid redundant debuginfo.
+            if old_span == Some(span) {
+                return;
+            }
+        }
 
-        // We may not always have valid spans.
-        // FIXME(eddyb) reduce the sources of this as much as possible.
-        if span.is_dummy() {
-            self.emit().no_line();
+        // HACK(eddyb) this is only to aid testing (and to not remove the old code).
+        let use_custom_insts = true;
+
+        if use_custom_insts {
+            // FIXME(eddyb) this should be cached more efficiently.
+            let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self);
+
+            // We may not always have valid spans.
+            // FIXME(eddyb) reduce the sources of this as much as possible.
+            if span.is_dummy() {
+                self.custom_inst(void_ty, CustomInst::ClearDebugSrcLoc);
+            } else {
+                let (file, line_col_range) = self.builder.file_line_col_range_for_debuginfo(span);
+                let ((line_start, col_start), (line_end, col_end)) =
+                    (line_col_range.start, line_col_range.end);
+
+                self.custom_inst(
+                    void_ty,
+                    CustomInst::SetDebugSrcLoc {
+                        file: Operand::IdRef(file.file_name_op_string_id),
+                        line_start: Operand::IdRef(self.const_u32(line_start).def(self)),
+                        line_end: Operand::IdRef(self.const_u32(line_end).def(self)),
+                        col_start: Operand::IdRef(self.const_u32(col_start).def(self)),
+                        col_end: Operand::IdRef(self.const_u32(col_end).def(self)),
+                    },
+                );
+            }
         } else {
-            let (file, line, col) = self.builder.file_line_col_for_op_line(span);
-            self.emit().line(file.file_name_op_string_id, line, col);
+            // We may not always have valid spans.
+            // FIXME(eddyb) reduce the sources of this as much as possible.
+            if span.is_dummy() {
+                self.emit().no_line();
+            } else {
+                let (file, line_col_range) = self.builder.file_line_col_range_for_debuginfo(span);
+                let (line, col) = line_col_range.start;
+
+                self.emit().line(file.file_name_op_string_id, line, col);
+            }
         }
     }
 
@@ -2359,6 +2400,8 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
                     _ => return None,
                 };
 
+                let custom_ext_inst_set_import = self.ext_inst.borrow_mut().import_custom(self);
+
                 // HACK(eddyb) we can remove SSA instructions even when they have
                 // side-effects, *as long as* they are "local" enough and cannot
                 // be observed from outside this current invocation - because the
@@ -2372,7 +2415,14 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
                     .instructions
                     .iter()
                     .enumerate()
-                    .filter(|(_, inst)| ![Op::Line, Op::NoLine].contains(&inst.class.opcode));
+                    .filter(|(_, inst)| {
+                        let is_standard_debug = [Op::Line, Op::NoLine].contains(&inst.class.opcode);
+                        let is_custom_debug = inst.class.opcode == Op::ExtInst
+                            && inst.operands[0].unwrap_id_ref() == custom_ext_inst_set_import
+                            && [CustomOp::SetDebugSrcLoc, CustomOp::ClearDebugSrcLoc]
+                                .contains(&CustomOp::decode_from_ext_inst(inst));
+                        !(is_standard_debug || is_custom_debug)
+                    });
                 let mut relevant_insts_next_back = |expected_op| {
                     non_debug_insts
                         .next_back()

crates/rustc_codegen_spirv/src/builder/ext_inst.rs

@@ -1,5 +1,6 @@
 use super::Builder;
 use crate::builder_spirv::{SpirvValue, SpirvValueExt};
+use crate::custom_insts;
 use rspirv::spirv::{GLOp, Word};
 use rspirv::{dr::Operand, spirv::Capability};
 
@@ -8,11 +9,26 @@ const GLSL_STD_450: &str = "GLSL.std.450";
 /// Manager for OpExtInst/OpExtImport instructions
 #[derive(Default)]
 pub struct ExtInst {
+    /// See `crate::custom_insts` for more details on what this entails.
+    custom: Option<Word>,
+
     glsl: Option<Word>,
     integer_functions_2_intel: bool,
 }
 
 impl ExtInst {
+    pub fn import_custom(&mut self, bx: &Builder<'_, '_>) -> Word {
+        if let Some(id) = self.custom {
+            id
+        } else {
+            let id = bx
+                .emit_global()
+                .ext_inst_import(custom_insts::CUSTOM_EXT_INST_SET.clone());
+            self.custom = Some(id);
+            id
+        }
+    }
+
     pub fn import_glsl(&mut self, bx: &Builder<'_, '_>) -> Word {
         if let Some(id) = self.glsl {
             id
@@ -46,6 +62,24 @@ impl ExtInst {
 }
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
+    pub fn custom_inst(
+        &mut self,
+        result_type: Word,
+        inst: custom_insts::CustomInst<Operand>,
+    ) -> SpirvValue {
+        let custom_ext_inst_set = self.ext_inst.borrow_mut().import_custom(self);
+        self.emit()
+            .ext_inst(
+                result_type,
+                None,
+                custom_ext_inst_set,
+                inst.op() as u32,
+                inst.into_operands(),
+            )
+            .unwrap()
+            .with_type(result_type)
+    }
+
     pub fn gl_op(
         &mut self,
         op: GLOp,

crates/rustc_codegen_spirv/src/builder_spirv.rs

@@ -21,6 +21,7 @@ use std::assert_matches::assert_matches;
 use std::cell::{RefCell, RefMut};
 use std::hash::{Hash, Hasher};
 use std::iter;
+use std::ops::Range;
 use std::str;
 use std::{fs::File, io::Write, path::Path};
 
@@ -678,13 +679,28 @@ impl<'tcx> BuilderSpirv<'tcx> {
         }
     }
 
-    pub fn file_line_col_for_op_line(&self, span: Span) -> (DebugFileSpirv<'tcx>, u32, u32) {
-        let loc = self.source_map.lookup_char_pos(span.lo());
-        (
-            self.def_debug_file(loc.file),
-            loc.line as u32,
-            loc.col_display as u32,
-        )
+    pub fn file_line_col_range_for_debuginfo(
+        &self,
+        span: Span,
+    ) -> (DebugFileSpirv<'tcx>, Range<(u32, u32)>) {
+        let (lo, hi) = (span.lo(), span.hi());
+
+        let lo_loc = self.source_map.lookup_char_pos(lo);
+        let lo_line_col = (lo_loc.line as u32, lo_loc.col_display as u32);
+
+        // Only use `hi` if the span is actually a range within a file.
+        let hi_line_col = if lo <= hi {
+            let hi_loc = self.source_map.lookup_char_pos(hi);
+            if lo_loc.file.start_pos == hi_loc.file.start_pos {
+                (hi_loc.line as u32, hi_loc.col_display as u32)
+            } else {
+                lo_line_col
+            }
+        } else {
+            lo_line_col
+        };
+
+        (self.def_debug_file(lo_loc.file), lo_line_col..hi_line_col)
     }
 
     fn def_debug_file(&self, sf: Lrc<SourceFile>) -> DebugFileSpirv<'tcx> {

crates/rustc_codegen_spirv/src/codegen_cx/declare.rs

@@ -2,7 +2,7 @@ use super::CodegenCx;
 use crate::abi::ConvSpirvType;
 use crate::attr::AggregatedSpirvAttributes;
 use crate::builder_spirv::{SpirvConst, SpirvValue, SpirvValueExt};
-use crate::decorations::{CustomDecoration, SrcLocDecoration};
+use crate::custom_decorations::{CustomDecoration, SrcLocDecoration};
 use crate::spirv_type::SpirvType;
 use rspirv::spirv::{FunctionControl, LinkageType, StorageClass, Word};
 use rustc_attr::InlineAttr;

crates/rustc_codegen_spirv/src/codegen_cx/entry.rs

@@ -63,7 +63,10 @@ impl<'tcx> CodegenCx<'tcx> {
         name: String,
         entry: Entry,
     ) {
-        let span = self.tcx.def_span(instance.def_id());
+        let span = self
+            .tcx
+            .def_ident_span(instance.def_id())
+            .unwrap_or_else(|| self.tcx.def_span(instance.def_id()));
         let hir_params = {
             let fn_local_def_id = if let Some(id) = instance.def_id().as_local() {
                 id

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

@@ -5,7 +5,7 @@ mod type_;
 
 use crate::builder::{ExtInst, InstructionTable};
 use crate::builder_spirv::{BuilderCursor, BuilderSpirv, SpirvConst, SpirvValue, SpirvValueKind};
-use crate::decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
+use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
 use crate::spirv_type::{SpirvType, SpirvTypePrinter, TypeCache};
 use crate::symbols::Symbols;
 use crate::target::SpirvTarget;
@@ -369,6 +369,11 @@ impl CodegenArgs {
                 "dump the SPIR-T module across passes, to a (pair of) file(s) in DIR",
                 "DIR",
             );
+            opts.optflag(
+                "",
+                "spirt-keep-custom-debuginfo-in-dumps",
+                "keep custom debuginfo when dumping SPIR-T (instead of lossily prettifying it)",
+            );
             opts.optflag(
                 "",
                 "specializer-debug",
@@ -534,6 +539,8 @@ 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"),
             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"),