From 9011856b00b8781f3ac4ddcbfd77d65a63b88638 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 1 Jun 2023 21:40:12 +0300 Subject: [PATCH] custom_inst: add `{Push,Pop}InlinedCallFrame` and emit them in the inliner. --- .../src/custom_decorations.rs | 2 +- .../rustc_codegen_spirv/src/custom_insts.rs | 9 + .../rustc_codegen_spirv/src/linker/inline.rs | 408 +++++++++++++----- .../src/linker/spirt_passes/debuginfo.rs | 5 + .../src/linker/spirt_passes/diagnostics.rs | 148 +++++-- .../rustc_codegen_spirv/src/linker/zombies.rs | 2 + 6 files changed, 437 insertions(+), 137 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/custom_decorations.rs b/crates/rustc_codegen_spirv/src/custom_decorations.rs index 63f386a2d5..6d82afebac 100644 --- a/crates/rustc_codegen_spirv/src/custom_decorations.rs +++ b/crates/rustc_codegen_spirv/src/custom_decorations.rs @@ -407,7 +407,7 @@ impl<'a> SpanRegenerator<'a> { const_u32(col_end), ) } - custom_inst @ CustomInst::ClearDebugSrcLoc => { + custom_inst => { unreachable!("src_loc_from_debug_inst({inst:?} => {custom_inst:?})") } } diff --git a/crates/rustc_codegen_spirv/src/custom_insts.rs b/crates/rustc_codegen_spirv/src/custom_insts.rs index 19926f02c8..9427b69323 100644 --- a/crates/rustc_codegen_spirv/src/custom_insts.rs +++ b/crates/rustc_codegen_spirv/src/custom_insts.rs @@ -116,4 +116,13 @@ def_custom_insts! { 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/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 144a1e98d2..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,9 +433,7 @@ 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. @@ -436,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 @@ -462,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); @@ -486,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)], + )], ); } @@ -536,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] = @@ -590,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/spirt_passes/debuginfo.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs index 9db741aa18..f89d456ddf 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs @@ -121,6 +121,11 @@ impl Transformer for CustomDebuginfoToSpv<'_> { insts_to_remove.push(inst); continue; } + CustomInst::PushInlinedCallFrame { .. } + | CustomInst::PopInlinedCallFrame => { + insts_to_remove.push(inst); + continue; + } } } } 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 1642795b46..c16b3d62bb 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs @@ -11,8 +11,8 @@ use spirt::func_at::FuncAt; use spirt::visit::{InnerVisit, Visitor}; use spirt::{ spv, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind, - DataInstDef, DataInstKind, Diag, DiagLevel, ExportKey, Exportee, Func, GlobalVar, InternedStr, - Module, Type, Value, + DataInstDef, DataInstKind, Diag, DiagLevel, ExportKey, Exportee, Func, FuncDecl, GlobalVar, + InternedStr, Module, Type, Value, }; use std::marker::PhantomData; use std::{mem, str}; @@ -51,7 +51,7 @@ pub(crate) fn report_diagnostics( 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, @@ -185,7 +185,7 @@ 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). @@ -196,6 +196,16 @@ enum UseOrigin<'a> { }, } +#[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, @@ -268,7 +278,7 @@ impl UseOrigin<'_> { col_start, col_end, } => (file, line_start, line_end, col_start, col_end), - CustomInst::ClearDebugSrcLoc => unreachable!(), + _ => unreachable!(), }; let const_ctor = |v: Value| match v { Value::Const(ct) => &cx[ct].ctor, @@ -329,26 +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")); @@ -523,7 +542,7 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { 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, @@ -533,6 +552,31 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { } } + 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); @@ -555,6 +599,11 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { } 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, @@ -577,6 +626,58 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { 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!(), + } + } + }, } } } @@ -585,11 +686,6 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { } 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/zombies.rs b/crates/rustc_codegen_spirv/src/linker/zombies.rs index 06ece27806..d91c459abd 100644 --- a/crates/rustc_codegen_spirv/src/linker/zombies.rs +++ b/crates/rustc_codegen_spirv/src/linker/zombies.rs @@ -83,6 +83,7 @@ impl<'a> Zombies<'a> { match CustomOp::decode_from_ext_inst(inst) { CustomOp::SetDebugSrcLoc => debug_src_loc_inst = Some(inst), CustomOp::ClearDebugSrcLoc => debug_src_loc_inst = None, + _ => {} } } _ => {} @@ -135,6 +136,7 @@ impl<'a> Zombies<'a> { match CustomOp::decode_from_ext_inst(inst) { CustomOp::SetDebugSrcLoc => debug_src_loc_inst = Some(inst), CustomOp::ClearDebugSrcLoc => debug_src_loc_inst = None, + _ => {} } } _ => {}