diff --git a/CHANGELOG.md b/CHANGELOG.md index 863ec8cefd..49d7eabf98 100644 --- a/CHANGELOG.md +++ b/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`) diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index b591bd3f41..72c0107fa5 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/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() diff --git a/crates/rustc_codegen_spirv/src/builder/ext_inst.rs b/crates/rustc_codegen_spirv/src/builder/ext_inst.rs index 7648da7466..e0dec8cf2d 100644 --- a/crates/rustc_codegen_spirv/src/builder/ext_inst.rs +++ b/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, + glsl: Option, 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, + ) -> 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, diff --git a/crates/rustc_codegen_spirv/src/builder_spirv.rs b/crates/rustc_codegen_spirv/src/builder_spirv.rs index 51b9901898..ee55c01851 100644 --- a/crates/rustc_codegen_spirv/src/builder_spirv.rs +++ b/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) -> DebugFileSpirv<'tcx> { diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/declare.rs b/crates/rustc_codegen_spirv/src/codegen_cx/declare.rs index b155d535bc..f062bbdcf0 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/declare.rs +++ b/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; diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs b/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs index 3d3a62f876..6b6bee6550 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs +++ b/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 diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index 918321d70a..7a23248627 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/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"), diff --git a/crates/rustc_codegen_spirv/src/decorations.rs b/crates/rustc_codegen_spirv/src/custom_decorations.rs similarity index 62% rename from crates/rustc_codegen_spirv/src/decorations.rs rename to crates/rustc_codegen_spirv/src/custom_decorations.rs index c3cbb843a0..6d82afebac 100644 --- a/crates/rustc_codegen_spirv/src/decorations.rs +++ b/crates/rustc_codegen_spirv/src/custom_decorations.rs @@ -2,6 +2,7 @@ //! the original codegen of a crate, and consumed by the `linker`. use crate::builder_spirv::BuilderSpirv; +use crate::custom_insts::{self, CustomInst}; use either::Either; use itertools::Itertools; use rspirv::dr::{Instruction, Module, Operand}; @@ -123,18 +124,20 @@ impl<'a> CustomDecoration<'a> for ZombieDecoration<'a> { } } -/// Equivalent of `OpLine`, for the places where `rspirv` currently doesn't let -/// us actually emit a real `OpLine`, the way generating SPIR-T directly might. +/// Equivalent of `CustomInst::SetDebugSrcLoc` (see `crate::custom_insts`), +/// for global definitions (i.e. outside functions), where limitations of +/// `rspirv`/`spirt` prevent us from using anything other than decorations. // -// NOTE(eddyb) by keeping `line`+`col`, we mimick `OpLine` limitations -// (which could be lifted in the future using custom SPIR-T debuginfo). -// NOTE(eddyb) `NonSemantic.Shader.DebugInfo`'s `DebugLine` has both start & end, -// might be good to invest in SPIR-T being able to use NonSemantic debuginfo. +// NOTE(eddyb) `CustomInst::SetDebugSrcLoc` is modelled after `DebugLine` from +// `NonSemantic.Shader.DebugInfo`, might be good to invest in SPIR-T being able +// to use `NonSemantic.Shader.DebugInfo` directly, in all situations. #[derive(Copy, Clone)] pub struct SrcLocDecoration<'a> { pub file_name: &'a str, - pub line: u32, - pub col: u32, + pub line_start: u32, + pub line_end: u32, + pub col_start: u32, + pub col_end: u32, } impl<'a> CustomDecoration<'a> for SrcLocDecoration<'a> { @@ -143,24 +146,33 @@ impl<'a> CustomDecoration<'a> for SrcLocDecoration<'a> { fn encode(self, w: &mut impl fmt::Write) -> fmt::Result { let Self { file_name, - line, - col, + line_start, + line_end, + col_start, + col_end, } = self; - write!(w, "{file_name}:{line}:{col}") + write!( + w, + "{file_name}:{line_start}:{col_start}-{line_end}:{col_end}" + ) } fn decode(s: &'a str) -> Self { #[derive(Copy, Clone, Debug)] struct InvalidSrcLoc<'a>(&'a str); let err = InvalidSrcLoc(s); - let (s, col) = s.rsplit_once(':').ok_or(err).unwrap(); - let (s, line) = s.rsplit_once(':').ok_or(err).unwrap(); + let (s, col_end) = s.rsplit_once(':').ok_or(err).unwrap(); + let (s, line_end) = s.rsplit_once('-').ok_or(err).unwrap(); + let (s, col_start) = s.rsplit_once(':').ok_or(err).unwrap(); + let (s, line_start) = s.rsplit_once(':').ok_or(err).unwrap(); let file_name = s; Self { file_name, - line: line.parse().unwrap(), - col: col.parse().unwrap(), + line_start: line_start.parse().unwrap(), + line_end: line_end.parse().unwrap(), + col_start: col_start.parse().unwrap(), + col_end: col_end.parse().unwrap(), } } } @@ -173,12 +185,16 @@ impl<'tcx> SrcLocDecoration<'tcx> { return None; } - let (file, line, col) = builder.file_line_col_for_op_line(span); + let (file, line_col_range) = 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); Some(Self { file_name: file.file_name, - line, - col, + line_start, + line_end, + col_start, + col_end, }) } } @@ -201,6 +217,14 @@ pub struct SpanRegenerator<'a> { #[derive(Default)] struct SpvDebugInfo<'a> { + /// ID of `OpExtInstImport` for our custom "extended instruction set", + /// if present (see `crate::custom_insts` for more details). + custom_ext_inst_set_import: Option, + + // HACK(eddyb) this is only needed because `OpExtInst`s can't have immediates, + // and must resort to referencing `OpConstant`s instead. + id_to_op_constant_operand: FxIndexMap, + id_to_op_string: FxIndexMap, files: FxIndexMap<&'a str, SpvDebugFile<'a>>, } @@ -235,6 +259,24 @@ impl<'a> SpvDebugInfo<'a> { } }; + // FIXME(eddyb) avoid repeating this across different passes/helpers. + this.custom_ext_inst_set_import = module + .ext_inst_imports + .iter() + .find(|inst| { + assert_eq!(inst.class.opcode, Op::ExtInstImport); + inst.operands[0].unwrap_literal_string() == &custom_insts::CUSTOM_EXT_INST_SET[..] + }) + .map(|inst| inst.result_id.unwrap()); + + this.id_to_op_constant_operand.extend( + module + .types_global_values + .iter() + .filter(|inst| inst.class.opcode == Op::Constant) + .map(|inst| (inst.result_id.unwrap(), &inst.operands[0])), + ); + let mut insts = module.debug_string_source.iter().peekable(); while let Some(inst) = insts.next() { match inst.class.opcode { @@ -326,20 +368,62 @@ impl<'a> SpanRegenerator<'a> { .map(|zombie| zombie.decode()) } - pub fn src_loc_from_op_line( - &mut self, - file_id: Word, - line: u32, - col: u32, - ) -> Option> { - self.spv_debug_info - .get_or_insert_with(|| SpvDebugInfo::collect(self.module)) + /// Extract the equivalent `SrcLocDecoration` from a debug instruction that + /// specifies some source location (both the standard `OpLine`, and our own + /// custom instruction, i.e. `CustomInst::SetDebugSrcLoc`, are supported). + pub fn src_loc_from_debug_inst(&mut self, inst: &Instruction) -> Option> { + let spv_debug_info = self + .spv_debug_info + .get_or_insert_with(|| SpvDebugInfo::collect(self.module)); + + let (file_id, line_start, line_end, col_start, col_end) = match inst.class.opcode { + Op::Line => { + let file = inst.operands[0].unwrap_id_ref(); + let line = inst.operands[1].unwrap_literal_int32(); + let col = inst.operands[2].unwrap_literal_int32(); + (file, line, line, col, col) + } + Op::ExtInst + if Some(inst.operands[0].unwrap_id_ref()) + == spv_debug_info.custom_ext_inst_set_import => + { + match CustomInst::decode(inst) { + CustomInst::SetDebugSrcLoc { + file, + line_start, + line_end, + col_start, + col_end, + } => { + let const_u32 = |operand: Operand| { + spv_debug_info.id_to_op_constant_operand[&operand.unwrap_id_ref()] + .unwrap_literal_int32() + }; + ( + file.unwrap_id_ref(), + const_u32(line_start), + const_u32(line_end), + const_u32(col_start), + const_u32(col_end), + ) + } + custom_inst => { + unreachable!("src_loc_from_debug_inst({inst:?} => {custom_inst:?})") + } + } + } + _ => unreachable!("src_loc_from_debug_inst({inst:?})"), + }; + + spv_debug_info .id_to_op_string .get(&file_id) .map(|&file_name| SrcLocDecoration { file_name, - line, - col, + line_start, + line_end, + col_start, + col_end, }) } @@ -401,66 +485,77 @@ impl<'a> SpanRegenerator<'a> { pub fn src_loc_to_rustc(&mut self, src_loc: SrcLocDecoration<'_>) -> Option { let SrcLocDecoration { file_name, - line, - col, + line_start, + line_end, + col_start, + col_end, } = src_loc; let file = self.regenerate_rustc_source_file(file_name)?; - let line_bpos_range = file.line_bounds(line.checked_sub(1)? as usize); - - // Find the special cases (`MultiByteChar`s/`NonNarrowChar`s) in the line. - let multibyte_chars = { - let find = |bpos| { - file.multibyte_chars - .binary_search_by_key(&bpos, |mbc| mbc.pos) - .unwrap_or_else(|x| x) + // FIXME(eddyb) avoid some of the duplicated work when this closure is + // called with `line`/`col` values that are near eachother - thankfully, + // this code should only be hit on the error reporting path anyway. + let line_col_to_bpos = |line: u32, col: u32| { + let line_bpos_range = file.line_bounds(line.checked_sub(1)? as usize); + + // Find the special cases (`MultiByteChar`s/`NonNarrowChar`s) in the line. + let multibyte_chars = { + let find = |bpos| { + file.multibyte_chars + .binary_search_by_key(&bpos, |mbc| mbc.pos) + .unwrap_or_else(|x| x) + }; + let Range { start, end } = line_bpos_range; + file.multibyte_chars[find(start)..find(end)].iter() }; - let Range { start, end } = line_bpos_range; - file.multibyte_chars[find(start)..find(end)].iter() - }; - let non_narrow_chars = { - let find = |bpos| { - file.non_narrow_chars - .binary_search_by_key(&bpos, |nnc| nnc.pos()) - .unwrap_or_else(|x| x) + let non_narrow_chars = { + let find = |bpos| { + file.non_narrow_chars + .binary_search_by_key(&bpos, |nnc| nnc.pos()) + .unwrap_or_else(|x| x) + }; + let Range { start, end } = line_bpos_range; + file.non_narrow_chars[find(start)..find(end)].iter() }; - let Range { start, end } = line_bpos_range; - file.non_narrow_chars[find(start)..find(end)].iter() - }; - let mut special_chars = multibyte_chars - .merge_join_by(non_narrow_chars, |mbc, nnc| mbc.pos.cmp(&nnc.pos())) - .peekable(); - - // Increment the `BytePos` until we reach the right `col_display`, using - // `MultiByteChar`s/`NonNarrowChar`s to track non-trivial contributions - // (this may look inefficient, but lines tend to be short, and `rustc` - // itself is even worse than this, when it comes to `BytePos` lookups). - let (mut cur_bpos, mut cur_col_display) = (line_bpos_range.start, 0); - while cur_bpos < line_bpos_range.end && cur_col_display < col { - let next_special_bpos = special_chars.peek().map(|special| { - special - .as_ref() - .map_any(|mbc| mbc.pos, |nnc| nnc.pos()) - .reduce(|x, _| x) - }); + let mut special_chars = multibyte_chars + .merge_join_by(non_narrow_chars, |mbc, nnc| mbc.pos.cmp(&nnc.pos())) + .peekable(); + + // Increment the `BytePos` until we reach the right `col_display`, using + // `MultiByteChar`s/`NonNarrowChar`s to track non-trivial contributions + // (this may look inefficient, but lines tend to be short, and `rustc` + // itself is even worse than this, when it comes to `BytePos` lookups). + let (mut cur_bpos, mut cur_col_display) = (line_bpos_range.start, 0); + while cur_bpos < line_bpos_range.end && cur_col_display < col { + let next_special_bpos = special_chars.peek().map(|special| { + special + .as_ref() + .map_any(|mbc| mbc.pos, |nnc| nnc.pos()) + .reduce(|x, _| x) + }); + + // Batch trivial chars (i.e. chars 1:1 wrt `BytePos` vs `col_display`). + let following_trivial_chars = + next_special_bpos.unwrap_or(line_bpos_range.end).0 - cur_bpos.0; + if following_trivial_chars > 0 { + let wanted_trivial_chars = following_trivial_chars.min(col - cur_col_display); + cur_bpos.0 += wanted_trivial_chars; + cur_col_display += wanted_trivial_chars; + continue; + } - // Batch trivial chars (i.e. chars 1:1 wrt `BytePos` vs `col_display`). - let following_trivial_chars = - next_special_bpos.unwrap_or(line_bpos_range.end).0 - cur_bpos.0; - if following_trivial_chars > 0 { - let wanted_trivial_chars = following_trivial_chars.min(col - cur_col_display); - cur_bpos.0 += wanted_trivial_chars; - cur_col_display += wanted_trivial_chars; - continue; + // Add a special char's `BytePos` and `col_display` contributions. + let mbc_nnc = special_chars.next().unwrap(); + cur_bpos.0 += mbc_nnc.as_ref().left().map_or(1, |mbc| mbc.bytes as u32); + cur_col_display += mbc_nnc.as_ref().right().map_or(1, |nnc| nnc.width() as u32); } + Some(cur_bpos) + }; - // Add a special char's `BytePos` and `col_display` contributions. - let mbc_nnc = special_chars.next().unwrap(); - cur_bpos.0 += mbc_nnc.as_ref().left().map_or(1, |mbc| mbc.bytes as u32); - cur_col_display += mbc_nnc.as_ref().right().map_or(1, |nnc| nnc.width() as u32); - } - - Some(Span::with_root_ctxt(cur_bpos, cur_bpos)) + Some(Span::with_root_ctxt( + line_col_to_bpos(line_start, col_start)?, + line_col_to_bpos(line_end, col_end)?, + )) } } diff --git a/crates/rustc_codegen_spirv/src/custom_insts.rs b/crates/rustc_codegen_spirv/src/custom_insts.rs new file mode 100644 index 0000000000..9427b69323 --- /dev/null +++ b/crates/rustc_codegen_spirv/src/custom_insts.rs @@ -0,0 +1,128 @@ +//! SPIR-V (extended) instructions specific to `rustc_codegen_spirv`, produced +//! during the original codegen of a crate, and consumed by the `linker`. + +use lazy_static::lazy_static; +use rspirv::dr::{Instruction, Operand}; +use rspirv::spirv::Op; +use smallvec::SmallVec; + +/// Prefix for `CUSTOM_EXT_INST_SET` (`OpExtInstImport` "instruction set" name), +/// without any of the disambiguating suffixes added for specific revisions. +/// +/// This **should not** be changed (if possible), to ensure version mismatches +/// can be detected (i.e. starting with this prefix, but the full name differs). +/// +/// 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"), "."); + +lazy_static! { + /// `OpExtInstImport` "instruction set" name for all Rust-GPU instructions. + /// + /// These considerations are relevant to the specific choice of name: + /// * does *not* start with `NonSemantic.`, as: + /// * some custom instructions may need to be semantic + /// * these custom instructions are not meant for the final SPIR-V + /// (so no third-party support is *technically* required for them) + /// * `NonSemantic.` requires SPIR-V 1.6 (or `SPV_KHR_non_semantic_info`) + /// * always starts with `CUSTOM_EXT_INST_SET_PREFIX` (see also its docs), + /// regardless of Rust-GPU version or custom instruction set definition + /// * contains enough disambiguating information to avoid misinterpretation + /// 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; + + let mut hasher = StableHasher::new(); + SCHEMA.hash(&mut hasher); + 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}") + }; +} + +macro_rules! def_custom_insts { + ($($num:literal => $name:ident $({ $($field:ident),+ $(,)? })?),+ $(,)?) => { + const SCHEMA: &[(u32, &str, &[&str])] = &[ + $(($num, stringify!($name), &[$($(stringify!($field)),+)?])),+ + ]; + + #[repr(u32)] + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + pub enum CustomOp { $($name = $num),+ } + + impl CustomOp { + pub fn decode(i: u32) -> Self { + match i { + $($num => Self::$name,)+ + _ => unreachable!("{i} is not a valid custom instruction number"), + } + } + + pub fn decode_from_ext_inst(inst: &Instruction) -> Self { + assert_eq!(inst.class.opcode, Op::ExtInst); + Self::decode(inst.operands[1].unwrap_literal_ext_inst_integer()) + } + + pub fn with_operands(self, operands: &[T]) -> CustomInst { + match self { + $(Self::$name => match operands { + [$($($field),+)?] => CustomInst::$name $({ $($field: $field.clone()),+ })?, + _ => unreachable!("{self:?} does not have the right number of operands"), + }),+ + } + } + } + + #[derive(Copy, Clone, Debug)] + pub enum CustomInst { + $($name $({ $($field: T),+ })?),+ + } + + impl CustomInst { + pub fn op(&self) -> CustomOp { + match *self { + $(Self::$name { .. } => CustomOp::$name),+ + } + } + + // HACK(eddyb) this should return an iterator, but that's too much effort. + pub fn into_operands(self) -> SmallVec<[T; 8]> { + match self { + $(Self::$name $({ $($field),+ })? => [$($($field),+)?].into_iter().collect()),+ + } + } + } + + impl CustomInst { + pub fn decode(inst: &Instruction) -> Self { + CustomOp::decode_from_ext_inst(inst).with_operands(&inst.operands[2..]) + } + } + } +} + +// NOTE(eddyb) several of these are similar to `NonSemantic.Shader.DebugInfo.100` +// instructions, but simpler (to aid implementation, for now). +def_custom_insts! { + // Like `DebugLine` (from `NonSemantic.Shader.DebugInfo.100`) or `OpLine`. + 0 => SetDebugSrcLoc { file, line_start, line_end, col_start, col_end }, + // Like `DebugNoLine` (from `NonSemantic.Shader.DebugInfo.100`) or `OpNoLine`. + 1 => ClearDebugSrcLoc, + + // Similar to `DebugInlinedAt` (from `NonSemantic.Shader.DebugInfo.100`), + // but simpler: there are no "scope objects", the location of the inlined + // callsite is given by other debuginfo (`SetDebugSrcLoc`/`OpLine`) active + // before this instruction, and only the name of the callee is recorded. + 2 => PushInlinedCallFrame { callee_name }, + // Leave the most recent inlined call frame entered by a `PushInlinedCallFrame` + // (i.e. the inlined call frames form a virtual call stack in debuginfo). + 3 => PopInlinedCallFrame, +} diff --git a/crates/rustc_codegen_spirv/src/lib.rs b/crates/rustc_codegen_spirv/src/lib.rs index 705a69386f..c966120bfb 100644 --- a/crates/rustc_codegen_spirv/src/lib.rs +++ b/crates/rustc_codegen_spirv/src/lib.rs @@ -72,7 +72,8 @@ mod attr; mod builder; mod builder_spirv; mod codegen_cx; -mod decorations; +mod custom_decorations; +mod custom_insts; mod link; mod linker; mod spirv_type; diff --git a/crates/rustc_codegen_spirv/src/linker/dce.rs b/crates/rustc_codegen_spirv/src/linker/dce.rs index c5d9b9b126..ac9bf3c7fd 100644 --- a/crates/rustc_codegen_spirv/src/linker/dce.rs +++ b/crates/rustc_codegen_spirv/src/linker/dce.rs @@ -9,7 +9,7 @@ use rspirv::dr::{Function, Instruction, Module, Operand}; use rspirv::spirv::{Decoration, LinkageType, Op, StorageClass, Word}; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::FxIndexSet; pub fn dce(module: &mut Module) { let mut rooted = collect_roots(module); @@ -17,8 +17,8 @@ pub fn dce(module: &mut Module) { kill_unrooted(module, &rooted); } -pub fn collect_roots(module: &Module) -> FxHashSet { - let mut rooted = FxHashSet::default(); +pub fn collect_roots(module: &Module) -> FxIndexSet { + let mut rooted = FxIndexSet::default(); for inst in &module.entry_points { root(inst, &mut rooted); @@ -53,7 +53,7 @@ fn all_inst_iter(func: &Function) -> impl DoubleEndedIterator) -> bool { +fn spread_roots(module: &Module, rooted: &mut FxIndexSet) -> bool { let mut any = false; for inst in module.global_inst_iter() { if let Some(id) = inst.result_id { @@ -82,7 +82,7 @@ fn spread_roots(module: &Module, rooted: &mut FxHashSet) -> bool { any } -fn root(inst: &Instruction, rooted: &mut FxHashSet) -> bool { +fn root(inst: &Instruction, rooted: &mut FxIndexSet) -> bool { let mut any = false; if let Some(id) = inst.result_type { any |= rooted.insert(id); @@ -95,7 +95,7 @@ fn root(inst: &Instruction, rooted: &mut FxHashSet) -> bool { any } -fn is_rooted(inst: &Instruction, rooted: &FxHashSet) -> bool { +fn is_rooted(inst: &Instruction, rooted: &FxIndexSet) -> bool { if let Some(result_id) = inst.result_id { rooted.contains(&result_id) } else { @@ -107,7 +107,7 @@ fn is_rooted(inst: &Instruction, rooted: &FxHashSet) -> bool { } } -fn kill_unrooted(module: &mut Module, rooted: &FxHashSet) { +fn kill_unrooted(module: &mut Module, rooted: &FxIndexSet) { module .ext_inst_imports .retain(|inst| is_rooted(inst, rooted)); @@ -138,7 +138,7 @@ fn kill_unrooted(module: &mut Module, rooted: &FxHashSet) { } pub fn dce_phi(func: &mut Function) { - let mut used = FxHashSet::default(); + let mut used = FxIndexSet::default(); loop { let mut changed = false; for inst in func.all_inst_iter() { diff --git a/crates/rustc_codegen_spirv/src/linker/import_export_link.rs b/crates/rustc_codegen_spirv/src/linker/import_export_link.rs index cf40f72d2c..0d5ae69c0d 100644 --- a/crates/rustc_codegen_spirv/src/linker/import_export_link.rs +++ b/crates/rustc_codegen_spirv/src/linker/import_export_link.rs @@ -1,5 +1,5 @@ use super::Result; -use crate::decorations::{CustomDecoration, ZombieDecoration}; +use crate::custom_decorations::{CustomDecoration, ZombieDecoration}; use rspirv::dr::{Instruction, Module}; use rspirv::spirv::{Capability, Decoration, LinkageType, Op, Word}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 223137b542..da816c60fa 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -7,6 +7,7 @@ use super::apply_rewrite_rules; use super::simple_passes::outgoing_edges; use super::{get_name, get_names}; +use crate::custom_insts::{self, CustomInst, CustomOp}; use rspirv::dr::{Block, Function, Instruction, Module, ModuleHeader, Operand}; use rspirv::spirv::{FunctionControl, Op, StorageClass, Word}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -14,10 +15,17 @@ use rustc_errors::ErrorGuaranteed; use rustc_session::Session; use smallvec::SmallVec; use std::convert::TryInto; -use std::mem::take; +use std::mem::{self, take}; type FunctionMap = FxHashMap; +// FIXME(eddyb) this is a bit silly, but this keeps being repeated everywhere. +fn next_id(header: &mut ModuleHeader) -> Word { + let result = header.bound; + header.bound += 1; + result +} + pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { // This algorithm gets real sad if there's recursion - but, good news, SPIR-V bans recursion deny_recursion_in_module(sess, module)?; @@ -28,11 +36,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { .map(|f| (f.def_id().unwrap(), f.clone())) .collect(); let legal_globals = LegalGlobal::gather_from_module(module); - let void = module - .types_global_values - .iter() - .find(|inst| inst.class.opcode == Op::TypeVoid) - .map_or(0, |inst| inst.result_id.unwrap()); + // Drop all the functions we'll be inlining. (This also means we won't waste time processing // inlines in functions that will get inlined) let mut dropped_ids = FxHashSet::default(); @@ -50,6 +54,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { true } }); + if !inlined_to_legalize_dont_inlines.is_empty() { let names = get_names(module); for f in inlined_to_legalize_dont_inlines { @@ -61,18 +66,63 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { } } - // Drop OpName etc. for inlined functions - module.debug_names.retain(|inst| { - !inst.operands.iter().any(|op| { - op.id_ref_any() - .map_or(false, |id| dropped_ids.contains(&id)) - }) - }); + let header = module.header.as_mut().unwrap(); + // FIXME(eddyb) clippy false positive (seperate `map` required for borrowck). + #[allow(clippy::map_unwrap_or)] let mut inliner = Inliner { - header: module.header.as_mut().unwrap(), - types_global_values: &mut module.types_global_values, + op_type_void_id: module + .types_global_values + .iter() + .find(|inst| inst.class.opcode == Op::TypeVoid) + .map(|inst| inst.result_id.unwrap()) + .unwrap_or_else(|| { + let id = next_id(header); + let inst = Instruction::new(Op::TypeVoid, None, Some(id), vec![]); + module.types_global_values.push(inst); + id + }), + + custom_ext_inst_set_import: module + .ext_inst_imports + .iter() + .find(|inst| { + assert_eq!(inst.class.opcode, Op::ExtInstImport); + inst.operands[0].unwrap_literal_string() == &custom_insts::CUSTOM_EXT_INST_SET[..] + }) + .map(|inst| inst.result_id.unwrap()) + .unwrap_or_else(|| { + let id = next_id(header); + let inst = Instruction::new( + Op::ExtInstImport, + None, + Some(id), + vec![Operand::LiteralString( + custom_insts::CUSTOM_EXT_INST_SET.to_string(), + )], + ); + module.ext_inst_imports.push(inst); + id + }), + + id_to_name: module + .debug_names + .iter() + .filter(|inst| inst.class.opcode == Op::Name) + .map(|inst| { + ( + inst.operands[0].unwrap_id_ref(), + inst.operands[1].unwrap_literal_string(), + ) + }) + .collect(), + + cached_op_strings: FxHashMap::default(), + + header, + debug_string_source: &mut module.debug_string_source, annotations: &mut module.annotations, - void, + types_global_values: &mut module.types_global_values, + functions: &functions, legal_globals: &legal_globals, }; @@ -80,6 +130,15 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { inliner.inline_fn(function); fuse_trivial_branches(function); } + + // Drop OpName etc. for inlined functions + module.debug_names.retain(|inst| { + !inst.operands.iter().any(|op| { + op.id_ref_any() + .map_or(false, |id| dropped_ids.contains(&id)) + }) + }); + Ok(()) } @@ -310,10 +369,19 @@ fn should_inline( .parameters .iter() .chain( - // TODO: OpLine handling call_site.caller.blocks[0] .instructions .iter() + .filter(|caller_inst| { + // HACK(eddyb) this only avoids scanning the + // whole entry block for `OpVariable`s, so + // it can overapproximate debuginfo insts. + let may_be_debuginfo = matches!( + caller_inst.class.opcode, + Op::Line | Op::NoLine | Op::ExtInst + ); + !may_be_debuginfo + }) .take_while(|caller_inst| caller_inst.class.opcode == Op::Variable), ) .map(|caller_inst| caller_inst.result_id.unwrap()); @@ -336,10 +404,28 @@ fn should_inline( // Insert blocks struct Inliner<'m, 'map> { + /// ID of `OpExtInstImport` for our custom "extended instruction set" + /// (see `crate::custom_insts` for more details). + custom_ext_inst_set_import: Word, + + op_type_void_id: Word, + + /// Pre-collected `OpName`s, that can be used to find any function's name + /// during inlining (to be able to generate debuginfo that uses names). + id_to_name: FxHashMap, + + /// `OpString` cache (for deduplicating `OpString`s for the same string). + // + // FIXME(eddyb) currently this doesn't reuse existing `OpString`s, but since + // this is mostly for inlined callee names, it's expected almost no overlap + // exists between existing `OpString`s and new ones, anyway. + cached_op_strings: FxHashMap<&'m str, Word>, + header: &'m mut ModuleHeader, - types_global_values: &'m mut Vec, + debug_string_source: &'m mut Vec, annotations: &'m mut Vec, - void: Word, + types_global_values: &'m mut Vec, + functions: &'map FunctionMap, legal_globals: &'map FxHashMap, // rewrite_rules: FxHashMap, @@ -347,24 +433,21 @@ struct Inliner<'m, 'map> { impl Inliner<'_, '_> { fn id(&mut self) -> Word { - let result = self.header.bound; - self.header.bound += 1; - result + next_id(self.header) } /// Applies all rewrite rules to the decorations in the header. fn apply_rewrite_for_decorations(&mut self, rewrite_rules: &FxHashMap) { - // NOTE(siebencorgie): We don't care *what* decoration we rewrite atm. AFAIK there is no case where rewriting - // the decoration on inline wouldn't be valid. + // NOTE(siebencorgie): We don't care *what* decoration we rewrite atm. + // AFAIK there is no case where keeping decorations on inline wouldn't be valid. for annotation_idx in 0..self.annotations.len() { - if self.annotations[annotation_idx].class.opcode == Op::Decorate { - if let Some(id) = self.annotations[annotation_idx].operands[0].id_ref_any_mut() { - if let Some(&rewrite) = rewrite_rules.get(id) { - // Copy decoration instruction and push it. - let mut instcpy = self.annotations[annotation_idx].clone(); - *instcpy.operands[0].id_ref_any_mut().unwrap() = rewrite; - self.annotations.push(instcpy); - } + let inst = &self.annotations[annotation_idx]; + if let [Operand::IdRef(target), ..] = inst.operands[..] { + if let Some(&rewritten_target) = rewrite_rules.get(&target) { + // Copy decoration instruction and push it. + let mut cloned_inst = inst.clone(); + cloned_inst.operands[0] = Operand::IdRef(rewritten_target); + self.annotations.push(cloned_inst); } } } @@ -437,13 +520,31 @@ impl Inliner<'_, '_> { }; let call_result_type = { let ty = call_inst.result_type.unwrap(); - if ty == self.void { + if ty == self.op_type_void_id { None } else { Some(ty) } }; let call_result_id = call_inst.result_id.unwrap(); + + // Get the debuginfo source location instruction that applies to the call. + let call_debug_src_loc_inst = caller.blocks[block_idx].instructions[..call_index] + .iter() + .rev() + .find(|inst| match inst.class.opcode { + Op::Line | Op::NoLine => true, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import => + { + matches!( + CustomOp::decode_from_ext_inst(inst), + CustomOp::SetDebugSrcLoc | CustomOp::ClearDebugSrcLoc + ) + } + _ => false, + }); + // Rewrite parameters to arguments let call_arguments = call_inst .operands @@ -463,7 +564,12 @@ impl Inliner<'_, '_> { }; let return_jump = self.id(); // Rewrite OpReturns of the callee. - let mut inlined_callee_blocks = get_inlined_blocks(callee, return_variable, return_jump); + let mut inlined_callee_blocks = self.get_inlined_blocks( + callee, + call_debug_src_loc_inst, + return_variable, + return_jump, + ); // Clone the IDs of the callee, because otherwise they'd be defined multiple times if the // fn is inlined multiple times. self.add_clone_id_rules(&mut rewrite_rules, &inlined_callee_blocks); @@ -487,10 +593,14 @@ impl Inliner<'_, '_> { if let Some(call_result_type) = call_result_type { // Generate the storage space for the return value: Do this *after* the split above, // because if block_idx=0, inserting a variable here shifts call_index. - insert_opvariable( + insert_opvariables( &mut caller.blocks[0], - self.ptr_ty(call_result_type), - return_variable.unwrap(), + [Instruction::new( + Op::Variable, + Some(self.ptr_ty(call_result_type)), + Some(return_variable.unwrap()), + vec![Operand::StorageClass(StorageClass::Function)], + )], ); } @@ -537,21 +647,49 @@ impl Inliner<'_, '_> { // Fuse the inlined callee entry block into the pre-call block. // This is okay because it's illegal to branch to the first BB in a function. { - // Take a prefix sequence of `OpVariable`s from the start of `insts`, - // and return it as a new `Vec` (while keeping the rest in `insts`). - let steal_vars = |insts: &mut Vec| { - let mut vars_and_non_vars = take(insts); - - // TODO: OpLine handling - let num_vars = vars_and_non_vars - .iter() - .position(|inst| inst.class.opcode != Op::Variable) - .unwrap_or(vars_and_non_vars.len()); - let (non_vars, vars) = (vars_and_non_vars.split_off(num_vars), vars_and_non_vars); - - // Keep non-`OpVariable`s in `insts` while returning `OpVariable`s. - *insts = non_vars; - vars + // Return the subsequence of `insts` made from `OpVariable`s, and any + // debuginfo instructions (which may apply to them), while removing + // *only* `OpVariable`s from `insts` (and keeping debuginfo in both). + let mut steal_vars = |insts: &mut Vec| { + let mut vars_and_debuginfo = vec![]; + insts.retain_mut(|inst| { + let is_debuginfo = match inst.class.opcode { + Op::Line | Op::NoLine => true, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() + == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc + | CustomOp::ClearDebugSrcLoc + | CustomOp::PushInlinedCallFrame + | CustomOp::PopInlinedCallFrame => true, + } + } + _ => false, + }; + if is_debuginfo { + // NOTE(eddyb) `OpExtInst`s have a result ID, + // even if unused, and it has to be unique. + let mut inst = inst.clone(); + if let Some(id) = &mut inst.result_id { + *id = self.id(); + } + vars_and_debuginfo.push(inst); + true + } else if inst.class.opcode == Op::Variable { + // HACK(eddyb) we're removing this `Instruction` from + // `inst`, so it doesn't really matter what we use here. + vars_and_debuginfo.push(mem::replace( + inst, + Instruction::new(Op::Nop, None, None, vec![]), + )); + false + } else { + true + } + }); + vars_and_debuginfo }; let [mut inlined_callee_entry_block]: [_; 1] = @@ -591,83 +729,132 @@ impl Inliner<'_, '_> { } } } -} -fn get_inlined_blocks( - function: &Function, - return_variable: Option, - return_jump: Word, -) -> Vec { - let mut blocks = function.blocks.clone(); - for block in &mut blocks { - let last = block.instructions.last().unwrap(); - if let Op::Return | Op::ReturnValue = last.class.opcode { - if Op::ReturnValue == last.class.opcode { - let return_value = last.operands[0].id_ref_any().unwrap(); - block.instructions.insert( - block.instructions.len() - 1, - Instruction::new( - Op::Store, - None, - None, - vec![ - Operand::IdRef(return_variable.unwrap()), - Operand::IdRef(return_value), - ], - ), + fn get_inlined_blocks( + &mut self, + callee: &Function, + call_debug_src_loc_inst: Option<&Instruction>, + return_variable: Option, + return_jump: Word, + ) -> Vec { + // Prepare the debuginfo insts to prepend/append to every block. + // FIXME(eddyb) this could be more efficient if we only used one pair of + // `{Push,Pop}InlinedCallFrame` for the whole inlined callee, but there + // is no way to hint the SPIR-T CFG (re)structurizer that it should keep + // the entire callee in one region - a SPIR-T inliner wouldn't have this + // issue, as it would require a fully structured callee. + let callee_name = self + .id_to_name + .get(&callee.def_id().unwrap()) + .copied() + .unwrap_or(""); + let callee_name_id = *self + .cached_op_strings + .entry(callee_name) + .or_insert_with(|| { + let id = next_id(self.header); + self.debug_string_source.push(Instruction::new( + Op::String, + None, + Some(id), + vec![Operand::LiteralString(callee_name.to_string())], + )); + id + }); + let mut mk_debuginfo_prefix_and_suffix = || { + // NOTE(eddyb) `OpExtInst`s have a result ID, even if unused, and + // it has to be unique (same goes for the other instructions below). + let instantiated_src_loc_inst = call_debug_src_loc_inst.map(|inst| { + let mut inst = inst.clone(); + if let Some(id) = &mut inst.result_id { + *id = self.id(); + } + inst + }); + let mut custom_inst_to_inst = |inst: CustomInst<_>| { + Instruction::new( + Op::ExtInst, + Some(self.op_type_void_id), + Some(self.id()), + [ + Operand::IdRef(self.custom_ext_inst_set_import), + Operand::LiteralExtInstInteger(inst.op() as u32), + ] + .into_iter() + .chain(inst.into_operands()) + .collect(), + ) + }; + ( + instantiated_src_loc_inst.into_iter().chain([{ + custom_inst_to_inst(CustomInst::PushInlinedCallFrame { + callee_name: Operand::IdRef(callee_name_id), + }) + }]), + [custom_inst_to_inst(CustomInst::PopInlinedCallFrame)].into_iter(), + ) + }; + + let mut blocks = callee.blocks.clone(); + for block in &mut blocks { + let last = block.instructions.last().unwrap(); + if let Op::Return | Op::ReturnValue = last.class.opcode { + if Op::ReturnValue == last.class.opcode { + let return_value = last.operands[0].id_ref_any().unwrap(); + block.instructions.insert( + block.instructions.len() - 1, + Instruction::new( + Op::Store, + None, + None, + vec![ + Operand::IdRef(return_variable.unwrap()), + Operand::IdRef(return_value), + ], + ), + ); + } else { + assert!(return_variable.is_none()); + } + *block.instructions.last_mut().unwrap() = + Instruction::new(Op::Branch, None, None, vec![Operand::IdRef(return_jump)]); + + let (debuginfo_prefix, debuginfo_suffix) = mk_debuginfo_prefix_and_suffix(); + block.instructions.splice( + // Insert the prefix debuginfo instructions after `OpPhi`s, + // which sadly can't be covered by them. + { + let i = block + .instructions + .iter() + .position(|inst| inst.class.opcode != Op::Phi) + .unwrap(); + i..i + }, + debuginfo_prefix, + ); + block.instructions.splice( + // Insert the suffix debuginfo instructions before the terminator, + // which sadly can't be covered by them. + { + let i = block.instructions.len() - 1; + i..i + }, + debuginfo_suffix, ); - } else { - assert!(return_variable.is_none()); } - *block.instructions.last_mut().unwrap() = - Instruction::new(Op::Branch, None, None, vec![Operand::IdRef(return_jump)]); } - } - blocks -} - -fn insert_opvariable(block: &mut Block, ptr_ty: Word, result_id: Word) { - let index = block - .instructions - .iter() - .enumerate() - .find_map(|(index, inst)| { - if inst.class.opcode != Op::Variable { - Some(index) - } else { - None - } - }); - let inst = Instruction::new( - Op::Variable, - Some(ptr_ty), - Some(result_id), - vec![Operand::StorageClass(StorageClass::Function)], - ); - match index { - Some(index) => block.instructions.insert(index, inst), - None => block.instructions.push(inst), + blocks } } -fn insert_opvariables(block: &mut Block, mut insts: Vec) { - let index = block +fn insert_opvariables(block: &mut Block, insts: impl IntoIterator) { + let first_non_variable = block .instructions .iter() - .enumerate() - .find_map(|(index, inst)| { - if inst.class.opcode != Op::Variable { - Some(index) - } else { - None - } - }); - match index { - Some(index) => { - block.instructions.splice(index..index, insts); - } - None => block.instructions.append(&mut insts), - } + .position(|inst| inst.class.opcode != Op::Variable); + let i = first_non_variable.unwrap_or(block.instructions.len()); + block.instructions.splice(i..i, insts); } fn fuse_trivial_branches(function: &mut Function) { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 6612e9b09a..bd182610e0 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -19,7 +19,8 @@ mod zombies; use std::borrow::Cow; use crate::codegen_cx::SpirvMetadata; -use crate::decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration}; +use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration}; +use crate::custom_insts; use either::Either; use rspirv::binary::{Assemble, Consumer}; use rspirv::dr::{Block, Instruction, Loader, Module, ModuleHeader, Operand}; @@ -56,6 +57,7 @@ pub struct Options { pub dump_post_merge: Option, pub dump_post_split: Option, pub dump_spirt_passes: Option, + pub spirt_keep_custom_debuginfo_in_dumps: bool, pub specializer_debug: bool, pub specializer_dump_instances: Option, pub print_all_zombie: bool, @@ -425,6 +427,15 @@ pub fn link( .join(disambiguated_crate_name_for_dumps) .with_extension("spirt"); + // HACK(eddyb) unless requested otherwise, clean up the pretty-printed + // SPIR-T output by converting our custom extended instructions, to + // standard SPIR-V debuginfo (which SPIR-T knows how to pretty-print). + if !opts.spirt_keep_custom_debuginfo_in_dumps { + for (_, module) in &mut per_pass_module_for_dumping { + spirt_passes::debuginfo::convert_custom_debuginfo_to_spv(module); + } + } + let plan = spirt::print::Plan::for_versions( &cx, per_pass_module_for_dumping @@ -446,10 +457,13 @@ pub fn link( } // NOTE(eddyb) this is late so that `--dump-spirt-passes` is processed, - // even/especially when error were reported, but lifting to SPIR-V is + // even/especially when errors were reported, but lifting to SPIR-V is // skipped (since it could very well fail due to reported errors). report_diagnostics_result?; + // Replace our custom debuginfo instructions just before lifting to SPIR-V. + spirt_passes::debuginfo::convert_custom_debuginfo_to_spv(&mut module); + let spv_words = { let _timer = sess.timer("spirt::Module::lift_to_spv_module_emitter"); module.lift_to_spv_module_emitter().unwrap().words @@ -462,6 +476,26 @@ pub fn link( }; } + // Ensure that no references remain, to our custom "extended instruction set". + for inst in &output.ext_inst_imports { + assert_eq!(inst.class.opcode, Op::ExtInstImport); + let ext_inst_set = inst.operands[0].unwrap_literal_string(); + if ext_inst_set.starts_with(custom_insts::CUSTOM_EXT_INST_SET_PREFIX) { + let expected = &custom_insts::CUSTOM_EXT_INST_SET[..]; + if ext_inst_set == expected { + return Err(sess.err(format!( + "`OpExtInstImport {ext_inst_set:?}` should not have been \ + left around after SPIR-T passes" + ))); + } else { + return Err(sess.err(format!( + "unsupported `OpExtInstImport {ext_inst_set:?}` + (expected {expected:?} name - version mismatch?)" + ))); + } + } + } + // FIXME(eddyb) rewrite these passes to SPIR-T ones, so we don't have to // parse the output of `spirt::spv::lift` back into `rspirv` - also, for // multi-module, it's much simpler with SPIR-T, just replace `module.exports` diff --git a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs index f76395ab33..7bd0ca285a 100644 --- a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs +++ b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs @@ -143,6 +143,7 @@ pub fn name_variables_pass(module: &mut Module) { module .debug_names .retain(|inst| variables.contains(&inst.operands[0].unwrap_id_ref())); + // FIXME(eddyb) why does this remove `OpLine` instructions? module .types_global_values .retain(|inst| inst.class.opcode != Op::Line); diff --git a/crates/rustc_codegen_spirv/src/linker/specializer.rs b/crates/rustc_codegen_spirv/src/linker/specializer.rs index 582daf1da9..d1cf59ba39 100644 --- a/crates/rustc_codegen_spirv/src/linker/specializer.rs +++ b/crates/rustc_codegen_spirv/src/linker/specializer.rs @@ -2379,7 +2379,7 @@ impl<'a, S: Specialization> Expander<'a, S> { let expanded_debug_names = expand_debug_or_annotation(debug_names); // Expand `Op(Member)Decorate* %target ...`, when `target` is "generic". - let expanded_annotations = expand_debug_or_annotation(annotations); + let mut expanded_annotations = expand_debug_or_annotation(annotations); // Expand "generic" globals (types, constants and module-scoped variables). let mut expanded_types_global_values = @@ -2440,6 +2440,8 @@ impl<'a, S: Specialization> Expander<'a, S> { let mut rewrite_rules = FxHashMap::default(); for func in newly_expanded_functions { + rewrite_rules.clear(); + rewrite_rules.extend(func.parameters.iter_mut().map(|param| { let old_id = param.result_id.unwrap(); let new_id = self.builder.id(); @@ -2460,6 +2462,18 @@ impl<'a, S: Specialization> Expander<'a, S> { ); super::apply_rewrite_rules(&rewrite_rules, &mut func.blocks); + + // HACK(eddyb) this duplicates similar logic from `inline`. + for annotation_idx in 0..expanded_annotations.len() { + let inst = &expanded_annotations[annotation_idx]; + if let [Operand::IdRef(target), ..] = inst.operands[..] { + if let Some(&rewritten_target) = rewrite_rules.get(&target) { + let mut expanded_inst = inst.clone(); + expanded_inst.operands[0] = Operand::IdRef(rewritten_target); + expanded_annotations.push(expanded_inst); + } + } + } } } diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs new file mode 100644 index 0000000000..f89d456ddf --- /dev/null +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs @@ -0,0 +1,161 @@ +//! SPIR-T passes related to debuginfo. + +use crate::custom_insts::{self, CustomInst, CustomOp}; +use rustc_data_structures::fx::FxIndexSet; +use smallvec::SmallVec; +use spirt::transform::{InnerInPlaceTransform, Transformer}; +use spirt::visit::InnerVisit; +use spirt::{ + spv, Attr, AttrSetDef, ConstCtor, Context, ControlNode, ControlNodeKind, DataInstKind, + InternedStr, Module, OrdAssertEq, Value, +}; + +/// Replace our custom extended instruction debuginfo with standard SPIR-V ones. +// +// FIXME(eddyb) also handle `SrcLocDecoration`s (when `rspirv` isn't used on the +// SPIR-V output of `spirt::spv::lift`, as it's lossy wrt `OpLine`). +pub fn convert_custom_debuginfo_to_spv(module: &mut Module) { + let cx = &module.cx(); + + // FIXME(eddyb) reuse this collection work in some kind of "pass manager". + let all_funcs = { + let mut collector = super::ReachableUseCollector { + cx, + module, + + seen_types: FxIndexSet::default(), + seen_consts: FxIndexSet::default(), + seen_global_vars: FxIndexSet::default(), + seen_funcs: FxIndexSet::default(), + }; + for (export_key, &exportee) in &module.exports { + export_key.inner_visit_with(&mut collector); + exportee.inner_visit_with(&mut collector); + } + collector.seen_funcs + }; + + let mut transformer = CustomDebuginfoToSpv { + cx, + wk: &super::SpvSpecWithExtras::get().well_known, + custom_ext_inst_set: cx.intern(&custom_insts::CUSTOM_EXT_INST_SET[..]), + }; + for func in all_funcs { + transformer.in_place_transform_func_decl(&mut module.funcs[func]); + } +} + +struct CustomDebuginfoToSpv<'a> { + cx: &'a Context, + wk: &'static super::SpvWellKnownWithExtras, + + /// Interned name for our custom "extended instruction set" + /// (see `crate::custom_insts` for more details). + custom_ext_inst_set: InternedStr, +} + +impl Transformer for CustomDebuginfoToSpv<'_> { + fn in_place_transform_control_node_def( + &mut self, + mut func_at_control_node: spirt::func_at::FuncAtMut<'_, ControlNode>, + ) { + // HACK(eddyb) this relies on the fact that `ControlNodeKind::Block` maps + // to one original SPIR-V block, which may not necessarily be true, and + // steps should be taken elsewhere to explicitly unset debuginfo, instead + // of relying on the end of a SPIR-V block implicitly unsetting it all. + // NOTE(eddyb) allowing debuginfo to apply *outside* of a `Block` could + // be useful in allowing *some* structured control-flow to have debuginfo, + // but that would likely require more work on the SPIR-T side. + if let ControlNodeKind::Block { mut insts } = func_at_control_node.reborrow().def().kind { + let mut current_file_line_col = None; + + // HACK(eddyb) buffering the `DataInst`s to remove from this block, + // as iterating and modifying a list at the same time isn't supported. + let mut insts_to_remove = SmallVec::<[_; 8]>::new(); + + let mut func_at_inst_iter = func_at_control_node.reborrow().at(insts).into_iter(); + while let Some(func_at_inst) = func_at_inst_iter.next() { + let inst = func_at_inst.position; + let data_inst_def = func_at_inst.def(); + + // FIXME(eddyb) deduplicate with `spirt_passes::diagnostics`. + if let DataInstKind::SpvExtInst { + ext_set, + inst: ext_inst, + } = data_inst_def.kind + { + if ext_set == self.custom_ext_inst_set { + match CustomOp::decode(ext_inst).with_operands(&data_inst_def.inputs) { + CustomInst::SetDebugSrcLoc { + file, + line_start: line, + line_end: _, + col_start: col, + col_end: _, + } => { + let const_ctor = |v: Value| match v { + Value::Const(ct) => &self.cx[ct].ctor, + _ => unreachable!(), + }; + let const_str = |v: Value| match const_ctor(v) { + &ConstCtor::SpvStringLiteralForExtInst(s) => s, + _ => unreachable!(), + }; + let const_u32 = |v: Value| match const_ctor(v) { + ConstCtor::SpvInst(spv_inst) => { + assert!(spv_inst.opcode == self.wk.OpConstant); + match spv_inst.imms[..] { + [spv::Imm::Short(_, x)] => x, + _ => unreachable!(), + } + } + _ => unreachable!(), + }; + current_file_line_col = + Some((const_str(file), const_u32(line), const_u32(col))); + insts_to_remove.push(inst); + continue; + } + CustomInst::ClearDebugSrcLoc => { + current_file_line_col = None; + insts_to_remove.push(inst); + continue; + } + CustomInst::PushInlinedCallFrame { .. } + | CustomInst::PopInlinedCallFrame => { + insts_to_remove.push(inst); + continue; + } + } + } + } + + // Add/remove the equivalent `Attr::SpvDebugLine` attribute. + // FIXME(eddyb) this could use more caching. + data_inst_def.attrs = self.cx.intern(AttrSetDef { + attrs: self.cx[data_inst_def.attrs] + .attrs + .iter() + .filter(|attr| !matches!(attr, Attr::SpvDebugLine { .. })) + .cloned() + .chain( + current_file_line_col.map(|(file, line, col)| Attr::SpvDebugLine { + file_path: OrdAssertEq(file), + line, + col, + }), + ) + .collect(), + }); + } + + // Finally remove the `DataInst`s buffered for removal earlier. + for inst in insts_to_remove { + insts.remove(inst, func_at_control_node.data_insts); + } + func_at_control_node.reborrow().def().kind = ControlNodeKind::Block { insts }; + } + + func_at_control_node.inner_in_place_transform_with(self); + } +} diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs index 1348b0cb20..c16b3d62bb 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs @@ -1,13 +1,18 @@ -use crate::decorations::{CustomDecoration, SpanRegenerator, SrcLocDecoration, ZombieDecoration}; +use crate::custom_decorations::{ + CustomDecoration, SpanRegenerator, SrcLocDecoration, ZombieDecoration, +}; +use crate::custom_insts::{self, CustomInst, CustomOp}; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::DiagnosticBuilder; use rustc_session::Session; use rustc_span::{Span, DUMMY_SP}; use smallvec::SmallVec; +use spirt::func_at::FuncAt; use spirt::visit::{InnerVisit, Visitor}; use spirt::{ - spv, Attr, AttrSet, AttrSetDef, Const, Context, DataInstDef, DataInstKind, Diag, DiagLevel, - ExportKey, Exportee, Func, GlobalVar, Module, Type, + spv, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind, + DataInstDef, DataInstKind, Diag, DiagLevel, ExportKey, Exportee, Func, FuncDecl, GlobalVar, + InternedStr, Module, Type, Value, }; use std::marker::PhantomData; use std::{mem, str}; @@ -24,6 +29,8 @@ pub(crate) fn report_diagnostics( linker_options, cx, + custom_ext_inst_set: cx.intern(&custom_insts::CUSTOM_EXT_INST_SET[..]), + module, seen_attrs: FxIndexSet::default(), @@ -37,21 +44,24 @@ pub(crate) fn report_diagnostics( overall_result: Ok(()), any_spirt_bugs: false, }; - for (export_key, &exportee) in &module.exports { + for (export_key, exportee) in &module.exports { assert_eq!(reporter.use_stack.len(), 0); - if let Exportee::Func(func) = exportee { + if let &Exportee::Func(func) = exportee { let func_decl = &module.funcs[func]; reporter.use_stack.push(UseOrigin::IntraFunc { func_attrs: func_decl.attrs, - func_export_key: Some(export_key), + special_func: Some(SpecialFunc::Exported(export_key)), + last_debug_src_loc_inst: None, inst_attrs: AttrSet::default(), origin: IntraFuncUseOrigin::Other, }); - export_key.inner_visit_with(&mut reporter); if reporter.seen_funcs.insert(func) { reporter.visit_func_decl(func_decl); } + // NOTE(eddyb) this is visited last, so that uses of the interface + // variables don't lack relevant context from the function body. + export_key.inner_visit_with(&mut reporter); reporter.use_stack.pop(); } export_key.inner_visit_with(&mut reporter); @@ -149,6 +159,11 @@ struct DiagnosticReporter<'a> { linker_options: &'a crate::linker::Options, cx: &'a Context, + + /// Interned name for our custom "extended instruction set" + /// (see `crate::custom_insts` for more details). + custom_ext_inst_set: InternedStr, + module: &'a Module, seen_attrs: FxIndexSet, @@ -170,50 +185,129 @@ enum UseOrigin<'a> { }, IntraFunc { func_attrs: AttrSet, - func_export_key: Option<&'a ExportKey>, + special_func: Option>, + + /// Active debug "source location" instruction at the time of the use, if any + /// (only `CustomInst::SetDebugSrcLoc` is supported). + last_debug_src_loc_inst: Option<&'a DataInstDef>, inst_attrs: AttrSet, origin: IntraFuncUseOrigin, }, } +#[derive(Copy, Clone)] +enum SpecialFunc<'a> { + /// This function is exported from the `Module` (likely an entry-point). + Exported(&'a ExportKey), + + /// This function doesn't have its own `FuncDecl`, but rather is an inlined + /// callee (i.e. instructions sandwiched by `{Push,Pop}InlinedCallFrame`). + Inlined { callee_name: InternedStr }, +} + enum IntraFuncUseOrigin { CallCallee, Other, } +impl SpanRegenerator<'_> { + fn spirt_attrs_to_rustc_span(&mut self, cx: &Context, attrs: AttrSet) -> Option { + let attrs_def = &cx[attrs]; + attrs_def + .attrs + .iter() + .find_map(|attr| match attr { + &Attr::SpvDebugLine { + file_path, + line, + col, + } => self.src_loc_to_rustc(SrcLocDecoration { + file_name: &cx[file_path.0], + line_start: line, + line_end: line, + col_start: col, + col_end: col, + }), + _ => None, + }) + .or_else(|| { + self.src_loc_to_rustc( + try_decode_custom_decoration::>(attrs_def)?.decode(), + ) + }) + } +} + impl UseOrigin<'_> { fn to_rustc_span(&self, cx: &Context, span_regen: &mut SpanRegenerator<'_>) -> Option { - let mut from_attrs = |attrs: AttrSet| { - let attrs_def = &cx[attrs]; - attrs_def - .attrs - .iter() - .find_map(|attr| match attr { - &Attr::SpvDebugLine { - file_path, - line, - col, - } => span_regen.src_loc_to_rustc(SrcLocDecoration { - file_name: &cx[file_path.0], - line, - col, - }), - _ => None, - }) - .or_else(|| { - span_regen.src_loc_to_rustc( - try_decode_custom_decoration::>(attrs_def)?.decode(), - ) - }) - }; match *self { - Self::Global { attrs, .. } => from_attrs(attrs), + Self::Global { attrs, .. } => span_regen.spirt_attrs_to_rustc_span(cx, attrs), Self::IntraFunc { func_attrs, + last_debug_src_loc_inst, inst_attrs, .. - } => from_attrs(inst_attrs).or_else(|| from_attrs(func_attrs)), + } => span_regen + .spirt_attrs_to_rustc_span(cx, inst_attrs) + .or_else(|| { + let debug_inst_def = last_debug_src_loc_inst?; + + let wk = &super::SpvSpecWithExtras::get().well_known; + + // FIXME(eddyb) deduplicate with `spirt_passes::diagnostics`. + let custom_op = match debug_inst_def.kind { + DataInstKind::SpvExtInst { + ext_set, + inst: ext_inst, + } => { + // FIXME(eddyb) inefficient (ideally the `InternedStr` + // shoudl be available), but this is the error case. + assert_eq!(&cx[ext_set], &custom_insts::CUSTOM_EXT_INST_SET[..]); + + CustomOp::decode(ext_inst) + } + _ => unreachable!(), + }; + let (file, line_start, line_end, col_start, col_end) = + match custom_op.with_operands(&debug_inst_def.inputs) { + CustomInst::SetDebugSrcLoc { + file, + line_start, + line_end, + col_start, + col_end, + } => (file, line_start, line_end, col_start, col_end), + _ => unreachable!(), + }; + let const_ctor = |v: Value| match v { + Value::Const(ct) => &cx[ct].ctor, + _ => unreachable!(), + }; + let const_str = |v: Value| match const_ctor(v) { + &ConstCtor::SpvStringLiteralForExtInst(s) => s, + _ => unreachable!(), + }; + let const_u32 = |v: Value| match const_ctor(v) { + ConstCtor::SpvInst(spv_inst) => { + assert!(spv_inst.opcode == wk.OpConstant); + match spv_inst.imms[..] { + [spv::Imm::Short(_, x)] => x, + _ => unreachable!(), + } + } + _ => unreachable!(), + }; + + span_regen.src_loc_to_rustc(SrcLocDecoration { + file_name: &cx[const_str(file)], + line_start: const_u32(line_start), + line_end: const_u32(line_end), + col_start: const_u32(col_start), + col_end: const_u32(col_end), + }) + }) + .or_else(|| span_regen.spirt_attrs_to_rustc_span(cx, func_attrs)), } } @@ -245,25 +339,35 @@ impl UseOrigin<'_> { } Self::IntraFunc { func_attrs, - func_export_key, + special_func, + last_debug_src_loc_inst: _, inst_attrs: _, origin, } => { - let func_desc = func_export_key - .map(|export_key| match export_key { - &ExportKey::LinkName(name) => format!("function export `{}`", &cx[name]), - ExportKey::SpvEntryPoint { imms, .. } => match imms[..] { - [em @ spv::Imm::Short(em_kind, _), ref name_imms @ ..] => { - assert_eq!(em_kind, wk.ExecutionModel); - let em = spv::print::operand_from_imms([em]).concat_to_plain_text(); - decode_spv_lit_str_with(name_imms, |name| { - format!( - "{} entry-point `{name}`", - em.strip_prefix("ExecutionModel.").unwrap() - ) - }) + let func_desc = special_func + .map(|special_func| match special_func { + SpecialFunc::Exported(&ExportKey::LinkName(name)) => { + format!("function export `{}`", &cx[name]) + } + SpecialFunc::Exported(ExportKey::SpvEntryPoint { imms, .. }) => { + match imms[..] { + [em @ spv::Imm::Short(em_kind, _), ref name_imms @ ..] => { + assert_eq!(em_kind, wk.ExecutionModel); + let em = + spv::print::operand_from_imms([em]).concat_to_plain_text(); + decode_spv_lit_str_with(name_imms, |name| { + format!( + "{} entry-point `{name}`", + em.strip_prefix("ExecutionModel.").unwrap() + ) + }) + } + _ => unreachable!(), } - _ => unreachable!(), + } + SpecialFunc::Inlined { callee_name } => match &cx[callee_name] { + "" => "unnamed function".into(), + callee_name => format!("`{callee_name}`"), }, }) .unwrap_or_else(|| name_from_attrs(*func_attrs, "function")); @@ -313,6 +417,13 @@ impl DiagnosticReporter<'_> { let ZombieDecoration { reason } = zombie.decode(); let def_span = current_def .and_then(|def| def.to_rustc_span(self.cx, &mut self.span_regen)) + .or_else(|| { + // If there's no clear source for the span, try to get + // it from the same attrs as the zombie, which could + // be missing from `use_stack` in some edge cases + // (such as zombied function parameters). + self.span_regen.spirt_attrs_to_rustc_span(self.cx, attrs) + }) .unwrap_or(DUMMY_SP); let mut err = self.sess.struct_span_err(def_span, reason); for use_origin in use_stack_for_def.iter().rev() { @@ -375,7 +486,7 @@ impl DiagnosticReporter<'_> { } } -impl Visitor<'_> for DiagnosticReporter<'_> { +impl<'a> Visitor<'a> for DiagnosticReporter<'a> { fn visit_attr_set_use(&mut self, attrs: AttrSet) { // HACK(eddyb) this avoids reporting the same diagnostics more than once. if self.seen_attrs.insert(attrs) { @@ -396,12 +507,20 @@ impl Visitor<'_> for DiagnosticReporter<'_> { fn visit_const_use(&mut self, ct: Const) { if self.seen_consts.insert(ct) { let ct_def = &self.cx[ct]; - self.use_stack.push(UseOrigin::Global { - kind: &"constant", - attrs: ct_def.attrs, - }); - self.visit_const_def(ct_def); - self.use_stack.pop(); + match ct_def.ctor { + // HACK(eddyb) don't push an `UseOrigin` for `GlobalVar` pointers. + ConstCtor::PtrToGlobalVar(_) if ct_def.attrs == AttrSet::default() => { + self.visit_const_def(ct_def); + } + _ => { + self.use_stack.push(UseOrigin::Global { + kind: &"constant", + attrs: ct_def.attrs, + }); + self.visit_const_def(ct_def); + self.use_stack.pop(); + } + } } } @@ -423,7 +542,8 @@ impl Visitor<'_> for DiagnosticReporter<'_> { let func_decl = &self.module.funcs[func]; self.use_stack.push(UseOrigin::IntraFunc { func_attrs: func_decl.attrs, - func_export_key: None, + special_func: None, + last_debug_src_loc_inst: None, inst_attrs: AttrSet::default(), origin: IntraFuncUseOrigin::Other, }); @@ -432,18 +552,140 @@ impl Visitor<'_> for DiagnosticReporter<'_> { } } - fn visit_data_inst_def(&mut self, data_inst_def: &DataInstDef) { + fn visit_func_decl(&mut self, func_decl: &'a FuncDecl) { + // Intra-function the `use_stack` can gather extra entries, and there's + // a risk they'd pile up without being popped, so here's a sanity check. + let original_use_stack_len = self.use_stack.len(); + + func_decl.inner_visit_with(self); + + assert!(self.use_stack.len() >= original_use_stack_len); + let extra = self.use_stack.len() - original_use_stack_len; + if extra > 0 { + // HACK(eddyb) synthesize a diagnostic to report right away. + self.report_from_attrs( + AttrSet::default().append_diag( + self.cx, + Diag::bug([format!( + "{extra} extraneous `use_stack` frame(s) found \ + (missing `PopInlinedCallFrame`?)" + ) + .into()]), + ), + ); + } + self.use_stack.truncate(original_use_stack_len); + } + + fn visit_control_node_def(&mut self, func_at_control_node: FuncAt<'a, ControlNode>) { + func_at_control_node.inner_visit_with(self); + + // HACK(eddyb) this relies on the fact that `ControlNodeKind::Block` maps + // to one original SPIR-V block, which may not necessarily be true, and + // steps should be taken elsewhere to explicitly unset debuginfo, instead + // of relying on the end of a SPIR-V block implicitly unsetting it all. + // NOTE(eddyb) allowing debuginfo to apply *outside* of a `Block` could + // be useful in allowing *some* structured control-flow to have debuginfo, + // but that would likely require more work on the SPIR-T side. + if let ControlNodeKind::Block { .. } = func_at_control_node.def().kind { + match self.use_stack.last_mut() { + Some(UseOrigin::IntraFunc { + last_debug_src_loc_inst, + .. + }) => *last_debug_src_loc_inst = None, + _ => unreachable!(), + } + } + } + + fn visit_data_inst_def(&mut self, data_inst_def: &'a DataInstDef) { + let replace_origin = |this: &mut Self, new_origin| match this.use_stack.last_mut() { + Some(UseOrigin::IntraFunc { origin, .. }) => mem::replace(origin, new_origin), + _ => unreachable!(), + }; + match self.use_stack.last_mut() { - Some(UseOrigin::IntraFunc { inst_attrs, .. }) => *inst_attrs = data_inst_def.attrs, + Some(UseOrigin::IntraFunc { + inst_attrs, + last_debug_src_loc_inst, + .. + }) => { + *inst_attrs = data_inst_def.attrs; + + // FIXME(eddyb) deduplicate with `spirt_passes::debuginfo`. + if let DataInstKind::SpvExtInst { + ext_set, + inst: ext_inst, + } = data_inst_def.kind + { + if ext_set == self.custom_ext_inst_set { + match CustomOp::decode(ext_inst) { + CustomOp::SetDebugSrcLoc => { + *last_debug_src_loc_inst = Some(data_inst_def); + } + CustomOp::ClearDebugSrcLoc => { + *last_debug_src_loc_inst = None; + } + op => match op.with_operands(&data_inst_def.inputs) { + CustomInst::SetDebugSrcLoc { .. } + | CustomInst::ClearDebugSrcLoc => unreachable!(), + CustomInst::PushInlinedCallFrame { callee_name } => { + // Treat this like a call, in the caller. + replace_origin(self, IntraFuncUseOrigin::CallCallee); + + let const_ctor = |v: Value| match v { + Value::Const(ct) => &self.cx[ct].ctor, + _ => unreachable!(), + }; + let const_str = |v: Value| match const_ctor(v) { + &ConstCtor::SpvStringLiteralForExtInst(s) => s, + _ => unreachable!(), + }; + self.use_stack.push(UseOrigin::IntraFunc { + func_attrs: AttrSet::default(), + special_func: Some(SpecialFunc::Inlined { + callee_name: const_str(callee_name), + }), + last_debug_src_loc_inst: None, + inst_attrs: AttrSet::default(), + origin: IntraFuncUseOrigin::Other, + }); + } + CustomInst::PopInlinedCallFrame => { + match self.use_stack.last() { + Some(UseOrigin::IntraFunc { special_func, .. }) => { + if let Some(SpecialFunc::Inlined { .. }) = special_func + { + self.use_stack.pop().unwrap(); + // Undo what `PushInlinedCallFrame` did to the + // original `UseOrigin::IntraFunc`. + replace_origin(self, IntraFuncUseOrigin::Other); + } else { + // HACK(eddyb) synthesize a diagnostic to report right away. + self.report_from_attrs( + AttrSet::default().append_diag( + self.cx, + Diag::bug([ + "`PopInlinedCallFrame` without an \ + inlined call frame in `use_stack`" + .into(), + ]), + ), + ); + } + } + _ => unreachable!(), + } + } + }, + } + } + } + } _ => unreachable!(), } if let DataInstKind::FuncCall(func) = data_inst_def.kind { - let replace_origin = |this: &mut Self, new_origin| match this.use_stack.last_mut() { - Some(UseOrigin::IntraFunc { origin, .. }) => mem::replace(origin, new_origin), - _ => unreachable!(), - }; - // HACK(eddyb) visit `func` early, to control its `use_stack`, with // the later visit from `inner_visit_with` ignored as a duplicate. let old_origin = replace_origin(self, IntraFuncUseOrigin::CallCallee); diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs index 4c0d77c328..59accf7370 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs @@ -1,5 +1,6 @@ //! SPIR-T pass infrastructure and supporting utilities. +pub(crate) mod debuginfo; pub(crate) mod diagnostics; mod fuse_selects; mod reduce; diff --git a/crates/rustc_codegen_spirv/src/linker/zombies.rs b/crates/rustc_codegen_spirv/src/linker/zombies.rs index 011e2def48..d91c459abd 100644 --- a/crates/rustc_codegen_spirv/src/linker/zombies.rs +++ b/crates/rustc_codegen_spirv/src/linker/zombies.rs @@ -1,10 +1,11 @@ //! See documentation on `CodegenCx::zombie` for a description of the zombie system. use super::{get_name, get_names}; -use crate::decorations::{CustomDecoration, SpanRegenerator, ZombieDecoration}; +use crate::custom_decorations::{CustomDecoration, SpanRegenerator, ZombieDecoration}; +use crate::custom_insts::{self, CustomOp}; use rspirv::dr::{Instruction, Module, Operand}; use rspirv::spirv::{Op, Word}; -use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed}; use rustc_session::Session; use rustc_span::{Span, DUMMY_SP}; @@ -13,22 +14,23 @@ use smallvec::SmallVec; #[derive(Copy, Clone)] struct Zombie<'a> { id: Word, - kind: &'a ZombieKind, + kind: &'a ZombieKind<'a>, } -enum ZombieKind { +enum ZombieKind<'a> { /// Definition annotated with `ZombieDecoration`. Leaf, /// Transitively zombie'd by using other zombies, from an instruction. - Uses(Vec), + Uses(Vec>), } -struct ZombieUse { +struct ZombieUse<'a> { used_zombie_id: Word, - /// Operands of the active `OpLine` at the time of the use, if any. - use_file_id_line_col: Option<(Word, u32, u32)>, + /// Active debug "source location" instruction at the time of the use, if any + /// (both `OpLine` and `CustomInst::SetDebugSrcLoc` are supported). + use_debug_src_loc_inst: Option<&'a Instruction>, origin: UseOrigin, } @@ -39,11 +41,15 @@ enum UseOrigin { CallCalleeOperand { caller_func_id: Word }, } -struct Zombies { - id_to_zombie_kind: FxIndexMap, +struct Zombies<'a> { + /// ID of `OpExtInstImport` for our custom "extended instruction set", + /// if present (see `crate::custom_insts` for more details). + custom_ext_inst_set_import: Option, + + id_to_zombie_kind: FxIndexMap>, } -impl Zombies { +impl<'a> Zombies<'a> { // FIXME(eddyb) rename all the other methods to say `_inst` explicitly. fn get_zombie_by_id(&self, id: Word) -> Option> { self.id_to_zombie_kind @@ -51,31 +57,35 @@ impl Zombies { .map(|kind| Zombie { id, kind }) } - fn zombies_used_from_inst<'a>( - &'a self, - inst: &'a Instruction, - ) -> impl Iterator> + 'a { + fn zombies_used_from_inst<'b>( + &'b self, + inst: &'b Instruction, + ) -> impl Iterator> + 'b { inst.result_type .into_iter() .chain(inst.operands.iter().filter_map(|op| op.id_ref_any())) .filter_map(move |id| self.get_zombie_by_id(id)) } - fn spread(&mut self, module: &Module) -> bool { + fn spread(&mut self, module: &'a Module) -> bool { let mut any = false; // globals are easy { - let mut file_id_line_col = None; + let mut debug_src_loc_inst = None; for inst in module.global_inst_iter() { match inst.class.opcode { - Op::Line => { - file_id_line_col = Some(( - inst.operands[0].unwrap_id_ref(), - inst.operands[1].unwrap_literal_int32(), - inst.operands[2].unwrap_literal_int32(), - )); + Op::Line => debug_src_loc_inst = Some(inst), + Op::NoLine => debug_src_loc_inst = None, + Op::ExtInst + if Some(inst.operands[0].unwrap_id_ref()) + == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => debug_src_loc_inst = Some(inst), + CustomOp::ClearDebugSrcLoc => debug_src_loc_inst = None, + _ => {} + } } - Op::NoLine => file_id_line_col = None, _ => {} } @@ -87,7 +97,7 @@ impl Zombies { .zombies_used_from_inst(inst) .map(|zombie| ZombieUse { used_zombie_id: zombie.id, - use_file_id_line_col: file_id_line_col, + use_debug_src_loc_inst: debug_src_loc_inst, origin: UseOrigin::GlobalOperandOrResultType, }) .collect(); @@ -113,17 +123,22 @@ impl Zombies { } let mut all_zombie_uses_in_func = vec![]; - let mut file_id_line_col = None; + let mut debug_src_loc_inst = None; for inst in func.all_inst_iter() { match inst.class.opcode { - Op::Line => { - file_id_line_col = Some(( - inst.operands[0].unwrap_id_ref(), - inst.operands[1].unwrap_literal_int32(), - inst.operands[2].unwrap_literal_int32(), - )); + Op::Line => debug_src_loc_inst = Some(inst), + // NOTE(eddyb) each block starts out with cleared debuginfo. + Op::Label | Op::NoLine => debug_src_loc_inst = None, + Op::ExtInst + if Some(inst.operands[0].unwrap_id_ref()) + == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => debug_src_loc_inst = Some(inst), + CustomOp::ClearDebugSrcLoc => debug_src_loc_inst = None, + _ => {} + } } - Op::NoLine => file_id_line_col = None, _ => {} } @@ -136,7 +151,7 @@ impl Zombies { .zombies_used_from_inst(inst) .map(|zombie| ZombieUse { used_zombie_id: zombie.id, - use_file_id_line_col: file_id_line_col, + use_debug_src_loc_inst: debug_src_loc_inst, origin: UseOrigin::IntraFuncOperandOrResultType { parent_func_id: func_id, }, @@ -169,7 +184,7 @@ impl Zombies { }; ZombieUse { used_zombie_id: zombie.id, - use_file_id_line_col: file_id_line_col, + use_debug_src_loc_inst: debug_src_loc_inst, origin, } }), @@ -188,13 +203,13 @@ impl Zombies { struct ZombieReporter<'a> { sess: &'a Session, module: &'a Module, - zombies: &'a Zombies, + zombies: &'a Zombies<'a>, id_to_name: Option>, span_regen: SpanRegenerator<'a>, } impl<'a> ZombieReporter<'a> { - fn new(sess: &'a Session, module: &'a Module, zombies: &'a Zombies) -> Self { + fn new(sess: &'a Session, module: &'a Module, zombies: &'a Zombies<'a>) -> Self { Self { sess, module, @@ -227,7 +242,7 @@ impl<'a> ZombieReporter<'a> { err: &mut DiagnosticBuilder<'a, ErrorGuaranteed>, span: Span, zombie: Zombie<'_>, - zombie_use: &ZombieUse, + zombie_use: &ZombieUse<'_>, ) { let mut id_to_name = |id, kind| { self.id_to_name @@ -252,12 +267,9 @@ impl<'a> ZombieReporter<'a> { format!("called by {}", id_to_name(caller_func_id, "function")) } }; - let span = zombie_use - .use_file_id_line_col - .and_then(|(file_id, line, col)| { - self.span_regen.src_loc_from_op_line(file_id, line, col) - }) + .use_debug_src_loc_inst + .and_then(|inst| self.span_regen.src_loc_from_debug_inst(inst)) .and_then(|src_loc| self.span_regen.src_loc_to_rustc(src_loc)) .unwrap_or(span); err.span_note(span, note); @@ -315,6 +327,16 @@ pub fn report_and_remove_zombies( module: &mut Module, ) -> super::Result<()> { let mut zombies = Zombies { + // FIXME(eddyb) avoid repeating this across different passes/helpers. + custom_ext_inst_set_import: module + .ext_inst_imports + .iter() + .find(|inst| { + assert_eq!(inst.class.opcode, Op::ExtInstImport); + inst.operands[0].unwrap_literal_string() == &custom_insts::CUSTOM_EXT_INST_SET[..] + }) + .map(|inst| inst.result_id.unwrap()), + id_to_zombie_kind: ZombieDecoration::decode_all(module) .map(|(id, _)| (id, ZombieKind::Leaf)) .collect(), @@ -372,11 +394,17 @@ pub fn report_and_remove_zombies( // FIXME(eddyb) this should be unnecessary, either something is unused, and // it will get DCE'd *anyway*, or it caused an error. { + // HACK(eddyb) cannot use the original map because it borrows the `Module`. + let all_zombies: FxHashSet<_> = zombies.id_to_zombie_kind.into_keys().collect(); let keep = |inst: &Instruction| { if let Some(result_id) = inst.result_id { - zombies.get_zombie_by_id(result_id).is_none() + !all_zombies.contains(&result_id) } else { - zombies.zombies_used_from_inst(inst).next().is_none() + let mut inst_ids = inst + .result_type + .into_iter() + .chain(inst.operands.iter().filter_map(|op| op.id_ref_any())); + !inst_ids.any(|id| all_zombies.contains(&id)) } }; module.capabilities.retain(keep); diff --git a/docs/src/codegen-args.md b/docs/src/codegen-args.md index 53e47a393f..060bbbcb6e 100644 --- a/docs/src/codegen-args.md +++ b/docs/src/codegen-args.md @@ -191,3 +191,12 @@ _Note: passes that are not already enabled by default are considered experimenta Dump the `SPIR-🇹` module across passes (i.e. all of the versions before/after each pass), as a combined report, to a pair of files (`.spirt` and `.spirt.html`) in `DIR`. (the `.spirt.html` version of the report is the recommended form for viewing, as it uses tabling for versions, syntax-highlighting-like styling, and use->def linking) + +### `--spirt-keep-custom-debuginfo-in-dumps` + +When dumping (pretty-printed) `SPIR-🇹` (e.g. with `--dump-spirt-passes`), preserve +all the custom (Rust-GPU-specific) debuginfo instructions, instead of converting +them to the standard SPIR-V debuginfo (which `SPIR-🇹` pretty-prints specially). + +The default (of performing that conversion) has prettier results, but is lossier +if you want to see all instructions exactly as e.g. `--spirt-passes` see them. diff --git a/tests/ui/dis/ptr_copy.normal.stderr b/tests/ui/dis/ptr_copy.normal.stderr index 42d4114aa3..416a87115c 100644 --- a/tests/ui/dis/ptr_copy.normal.stderr +++ b/tests/ui/dis/ptr_copy.normal.stderr @@ -2,28 +2,28 @@ error: cannot memcpy dynamically sized data --> $CORE_SRC/intrinsics.rs:2460:9 | 2460 | copy(src, dst, count) - | ^ + | ^^^^^^^^^^^^^^^^^^^^^ | note: used from within `core::intrinsics::copy::` --> $CORE_SRC/intrinsics.rs:2447:21 | 2447 | pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { - | ^ + | ^^^^ note: called by `ptr_copy::copy_via_raw_ptr` --> $DIR/ptr_copy.rs:28:18 | 28 | unsafe { core::ptr::copy(src, dst, 1) } - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: called by `ptr_copy::main` --> $DIR/ptr_copy.rs:33:5 | 33 | copy_via_raw_ptr(&i, o); - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^ note: called by `main` - --> $DIR/ptr_copy.rs:31:1 + --> $DIR/ptr_copy.rs:32:8 | -31 | #[spirv(fragment)] - | ^ +32 | pub fn main(i: f32, o: &mut f32) { + | ^^^^ error: aborting due to previous error diff --git a/tests/ui/lang/consts/nested-ref-in-composite.stderr b/tests/ui/lang/consts/nested-ref-in-composite.stderr index d6205a866a..632757f31d 100644 --- a/tests/ui/lang/consts/nested-ref-in-composite.stderr +++ b/tests/ui/lang/consts/nested-ref-in-composite.stderr @@ -2,35 +2,35 @@ error: constant arrays/structs cannot contain pointers to other constants --> $DIR/nested-ref-in-composite.rs:20:17 | 20 | *pair_out = pair_deep_load(&(&123, &3.14)); - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: used from within `nested_ref_in_composite::main_pair` --> $DIR/nested-ref-in-composite.rs:20:17 | 20 | *pair_out = pair_deep_load(&(&123, &3.14)); - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: called by `main_pair` - --> $DIR/nested-ref-in-composite.rs:18:1 + --> $DIR/nested-ref-in-composite.rs:19:8 | -18 | #[spirv(fragment)] - | ^ +19 | pub fn main_pair(pair_out: &mut (u32, f32)) { + | ^^^^^^^^^ error: constant arrays/structs cannot contain pointers to other constants --> $DIR/nested-ref-in-composite.rs:25:19 | 25 | *array3_out = array3_deep_load(&[&0, &1, &2]); - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: used from within `nested_ref_in_composite::main_array3` --> $DIR/nested-ref-in-composite.rs:25:19 | 25 | *array3_out = array3_deep_load(&[&0, &1, &2]); - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: called by `main_array3` - --> $DIR/nested-ref-in-composite.rs:23:1 + --> $DIR/nested-ref-in-composite.rs:24:8 | -23 | #[spirv(fragment)] - | ^ +24 | pub fn main_array3(array3_out: &mut [u32; 3]) { + | ^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/tests/ui/lang/core/ptr/allocate_const_scalar.stderr b/tests/ui/lang/core/ptr/allocate_const_scalar.stderr index c6ca98ddc3..1e14ba7962 100644 --- a/tests/ui/lang/core/ptr/allocate_const_scalar.stderr +++ b/tests/ui/lang/core/ptr/allocate_const_scalar.stderr @@ -4,12 +4,12 @@ note: used from within `allocate_const_scalar::main` --> $DIR/allocate_const_scalar.rs:15:20 | 15 | let _pointer = POINTER; - | ^ + | ^^^^^^^ note: called by `main` - --> $DIR/allocate_const_scalar.rs:13:1 + --> $DIR/allocate_const_scalar.rs:14:8 | -13 | #[spirv(fragment)] - | ^ +14 | pub fn main() { + | ^^^^ error: aborting due to previous error diff --git a/tests/ui/lang/core/ref/zst_member_ref_arg-broken.stderr b/tests/ui/lang/core/ref/zst_member_ref_arg-broken.stderr index 0bae7acf77..d2f24359aa 100644 --- a/tests/ui/lang/core/ref/zst_member_ref_arg-broken.stderr +++ b/tests/ui/lang/core/ref/zst_member_ref_arg-broken.stderr @@ -1,40 +1,21 @@ -error: cannot cast between pointer types - from `*struct (usize, usize) { u32, u32 }` - to `*struct B { }` - --> $DIR/zst_member_ref_arg-broken.rs:33:5 - | -33 | f(&s.y); - | ^ - | -note: used from within `zst_member_ref_arg_broken::main_scalar_scalar_pair_nested` - --> $DIR/zst_member_ref_arg-broken.rs:33:5 - | -33 | f(&s.y); - | ^ -note: called by `main_scalar_scalar_pair_nested` - --> $DIR/zst_member_ref_arg-broken.rs:31:1 - | -31 | #[spirv(fragment)] - | ^ - error: cannot cast between pointer types from `*u32` to `*struct B { }` --> $DIR/zst_member_ref_arg-broken.rs:23:5 | 23 | f(&s.y); - | ^ + | ^^^^^^^ | note: used from within `zst_member_ref_arg_broken::main_scalar` --> $DIR/zst_member_ref_arg-broken.rs:23:5 | 23 | f(&s.y); - | ^ + | ^^^^^^^ note: called by `main_scalar` - --> $DIR/zst_member_ref_arg-broken.rs:21:1 + --> $DIR/zst_member_ref_arg-broken.rs:22:8 | -21 | #[spirv(fragment)] - | ^ +22 | pub fn main_scalar(#[spirv(push_constant)] s: &S) { + | ^^^^^^^^^^^ error: cannot cast between pointer types from `*struct S { u32, u32 }` @@ -42,18 +23,37 @@ error: cannot cast between pointer types --> $DIR/zst_member_ref_arg-broken.rs:28:5 | 28 | f(&s.y); - | ^ + | ^^^^^^^ | note: used from within `zst_member_ref_arg_broken::main_scalar_pair` --> $DIR/zst_member_ref_arg-broken.rs:28:5 | 28 | f(&s.y); - | ^ + | ^^^^^^^ note: called by `main_scalar_pair` - --> $DIR/zst_member_ref_arg-broken.rs:26:1 + --> $DIR/zst_member_ref_arg-broken.rs:27:8 + | +27 | pub fn main_scalar_pair(#[spirv(push_constant)] s: &S) { + | ^^^^^^^^^^^^^^^^ + +error: cannot cast between pointer types + from `*struct (usize, usize) { u32, u32 }` + to `*struct B { }` + --> $DIR/zst_member_ref_arg-broken.rs:33:5 + | +33 | f(&s.y); + | ^^^^^^^ + | +note: used from within `zst_member_ref_arg_broken::main_scalar_scalar_pair_nested` + --> $DIR/zst_member_ref_arg-broken.rs:33:5 + | +33 | f(&s.y); + | ^^^^^^^ +note: called by `main_scalar_scalar_pair_nested` + --> $DIR/zst_member_ref_arg-broken.rs:32:8 | -26 | #[spirv(fragment)] - | ^ +32 | pub fn main_scalar_scalar_pair_nested(#[spirv(push_constant)] s: &S<(usize, usize)>) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 3 previous errors