From eeffeaa14cdd165132973c5554febec50bf708b1 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 01/14] Revert "linker/inline: use `OpPhi` instead of `OpVariable` for return values." This reverts commit fcd1b1e750b6fba7684f348bb2f37477ecc1bcdf. --- .../rustc_codegen_spirv/src/linker/inline.rs | 136 +++++++++--------- .../ui/dis/complex_image_sample_inst.stderr | 21 +-- 2 files changed, 80 insertions(+), 77 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 0ab19bd40f1..e2aac8c548f 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -94,6 +94,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { header, debug_string_source: &mut module.debug_string_source, annotations: &mut module.annotations, + types_global_values: &mut module.types_global_values, legal_globals, @@ -492,6 +493,7 @@ struct Inliner<'a, 'b> { header: &'b mut ModuleHeader, debug_string_source: &'b mut Vec, annotations: &'b mut Vec, + types_global_values: &'b mut Vec, legal_globals: FxHashMap, functions_that_may_abort: FxHashSet, @@ -521,6 +523,29 @@ impl Inliner<'_, '_> { } } + fn ptr_ty(&mut self, pointee: Word) -> Word { + // TODO: This is horribly slow, fix this + let existing = self.types_global_values.iter().find(|inst| { + inst.class.opcode == Op::TypePointer + && inst.operands[0].unwrap_storage_class() == StorageClass::Function + && inst.operands[1].unwrap_id_ref() == pointee + }); + if let Some(existing) = existing { + return existing.result_id.unwrap(); + } + let inst_id = self.id(); + self.types_global_values.push(Instruction::new( + Op::TypePointer, + None, + Some(inst_id), + vec![ + Operand::StorageClass(StorageClass::Function), + Operand::IdRef(pointee), + ], + )); + inst_id + } + fn inline_fn( &mut self, function: &mut Function, @@ -597,19 +622,15 @@ impl Inliner<'_, '_> { .insert(caller.def_id().unwrap()); } - let mut maybe_call_result_phi = { + let call_result_type = { let ty = call_inst.result_type.unwrap(); if ty == self.op_type_void_id { None } else { - Some(Instruction::new( - Op::Phi, - Some(ty), - Some(call_inst.result_id.unwrap()), - vec![], - )) + Some(ty) } }; + let call_result_id = call_inst.result_id.unwrap(); // Get the debug "source location" instruction that applies to the call. let custom_ext_inst_set_import = self.custom_ext_inst_set_import; @@ -646,12 +667,17 @@ impl Inliner<'_, '_> { }); let mut rewrite_rules = callee_parameters.zip(call_arguments).collect(); + let return_variable = if call_result_type.is_some() { + Some(self.id()) + } else { + None + }; let return_jump = self.id(); // Rewrite OpReturns of the callee. let mut inlined_callee_blocks = self.get_inlined_blocks( callee, call_debug_src_loc_inst, - maybe_call_result_phi.as_mut(), + return_variable, return_jump, ); // Clone the IDs of the callee, because otherwise they'd be defined multiple times if the @@ -660,55 +686,6 @@ impl Inliner<'_, '_> { apply_rewrite_rules(&rewrite_rules, &mut inlined_callee_blocks); self.apply_rewrite_for_decorations(&rewrite_rules); - if let Some(call_result_phi) = &mut maybe_call_result_phi { - // HACK(eddyb) new IDs should be generated earlier, to avoid pushing - // callee IDs to `call_result_phi.operands` only to rewrite them here. - for op in &mut call_result_phi.operands { - if let Some(id) = op.id_ref_any_mut() - && let Some(&rewrite) = rewrite_rules.get(id) - { - *id = rewrite; - } - } - - // HACK(eddyb) this special-casing of the single-return case is - // really necessary for passes like `mem2reg` which are not capable - // of skipping through the extraneous `OpPhi`s on their own. - if let [returned_value, _return_block] = &call_result_phi.operands[..] { - let call_result_id = call_result_phi.result_id.unwrap(); - let returned_value_id = returned_value.unwrap_id_ref(); - - maybe_call_result_phi = None; - - // HACK(eddyb) this is a conservative approximation of all the - // instructions that could potentially reference the call result. - let reaching_insts = { - let (pre_call_blocks, call_and_post_call_blocks) = - caller.blocks.split_at_mut(block_idx); - (pre_call_blocks.iter_mut().flat_map(|block| { - block - .instructions - .iter_mut() - .take_while(|inst| inst.class.opcode == Op::Phi) - })) - .chain( - call_and_post_call_blocks - .iter_mut() - .flat_map(|block| &mut block.instructions), - ) - }; - for reaching_inst in reaching_insts { - for op in &mut reaching_inst.operands { - if let Some(id) = op.id_ref_any_mut() - && *id == call_result_id - { - *id = returned_value_id; - } - } - } - } - } - // Split the block containing the `OpFunctionCall` into pre-call vs post-call. let pre_call_block_idx = block_idx; #[expect(unused)] @@ -724,6 +701,18 @@ impl Inliner<'_, '_> { .unwrap(); assert!(call.class.opcode == Op::FunctionCall); + 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. + let ret_var_inst = Instruction::new( + Op::Variable, + Some(self.ptr_ty(call_result_type)), + Some(return_variable.unwrap()), + vec![Operand::StorageClass(StorageClass::Function)], + ); + self.insert_opvariables(&mut caller.blocks[0], [ret_var_inst]); + } + // Insert non-entry inlined callee blocks just after the pre-call block. let non_entry_inlined_callee_blocks = inlined_callee_blocks.drain(1..); let num_non_entry_inlined_callee_blocks = non_entry_inlined_callee_blocks.len(); @@ -732,9 +721,18 @@ impl Inliner<'_, '_> { non_entry_inlined_callee_blocks, ); - if let Some(call_result_phi) = maybe_call_result_phi { - // Add the `OpPhi` for the call result value, after the inlined function. - post_call_block_insts.insert(0, call_result_phi); + if let Some(call_result_type) = call_result_type { + // Add the load of the result value after the inlined function. Note there's guaranteed no + // OpPhi instructions since we just split this block. + post_call_block_insts.insert( + 0, + Instruction::new( + Op::Load, + Some(call_result_type), + Some(call_result_id), + vec![Operand::IdRef(return_variable.unwrap())], + ), + ); } // Insert the post-call block, after all the inlined callee blocks. @@ -901,7 +899,7 @@ impl Inliner<'_, '_> { &mut self, callee: &Function, call_debug_src_loc_inst: Option<&Instruction>, - mut maybe_call_result_phi: Option<&mut Instruction>, + return_variable: Option, return_jump: Word, ) -> Vec { let Self { @@ -999,13 +997,17 @@ impl Inliner<'_, '_> { if let Op::Return | Op::ReturnValue = terminator.class.opcode { if Op::ReturnValue == terminator.class.opcode { let return_value = terminator.operands[0].id_ref_any().unwrap(); - let call_result_phi = maybe_call_result_phi.as_deref_mut().unwrap(); - call_result_phi.operands.extend([ - Operand::IdRef(return_value), - Operand::IdRef(block.label_id().unwrap()), - ]); + block.instructions.push(Instruction::new( + Op::Store, + None, + None, + vec![ + Operand::IdRef(return_variable.unwrap()), + Operand::IdRef(return_value), + ], + )); } else { - assert!(maybe_call_result_phi.is_none()); + assert!(return_variable.is_none()); } terminator = Instruction::new(Op::Branch, None, None, vec![Operand::IdRef(return_jump)]); diff --git a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr index b99b63a736b..3d62336afd9 100644 --- a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr +++ b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr @@ -3,18 +3,19 @@ %5 = OpFunctionParameter %6 %7 = OpFunctionParameter %6 %8 = OpLabel -%9 = OpCompositeExtract %10 %5 0 -%11 = OpCompositeExtract %10 %5 1 -%12 = OpCompositeConstruct %6 %9 %11 -%13 = OpCompositeExtract %10 %7 0 -%14 = OpCompositeExtract %10 %7 1 -%15 = OpCompositeConstruct %6 %13 %14 -OpLine %16 29 13 +OpLine %9 10 25 +%10 = OpCompositeExtract %11 %5 0 +%12 = OpCompositeExtract %11 %5 1 +%13 = OpCompositeConstruct %6 %10 %12 +%14 = OpCompositeExtract %11 %7 0 +%15 = OpCompositeExtract %11 %7 1 +%16 = OpCompositeConstruct %6 %14 %15 +OpLine %9 29 13 %17 = OpAccessChain %18 %19 %20 -OpLine %16 30 13 +OpLine %9 30 13 %21 = OpLoad %22 %17 -OpLine %16 34 13 -%23 = OpImageSampleProjExplicitLod %2 %21 %4 Grad %12 %15 +OpLine %9 34 13 +%23 = OpImageSampleProjExplicitLod %2 %21 %4 Grad %13 %16 OpNoLine OpReturnValue %23 OpFunctionEnd From 1ee7ca3f2244073a89ac89a8aa235d160bd1191c Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 02/14] Revert "WIP: mem2reg speedup" This reverts commit efbf694edef096f01c10482d262882fb3d1711ff. --- crates/rustc_codegen_spirv/src/linker/dce.rs | 11 +-- .../rustc_codegen_spirv/src/linker/mem2reg.rs | 91 ++++++++----------- crates/rustc_codegen_spirv/src/linker/mod.rs | 7 +- 3 files changed, 46 insertions(+), 63 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/dce.rs b/crates/rustc_codegen_spirv/src/linker/dce.rs index 7e9297e5d2a..3ea62e1693d 100644 --- a/crates/rustc_codegen_spirv/src/linker/dce.rs +++ b/crates/rustc_codegen_spirv/src/linker/dce.rs @@ -7,10 +7,9 @@ //! *references* a rooted thing is also rooted, not the other way around - but that's the basic //! concept. -use rspirv::dr::{Block, Function, Instruction, Module, Operand}; +use rspirv::dr::{Function, Instruction, Module, Operand}; use rspirv::spirv::{Decoration, LinkageType, Op, StorageClass, Word}; -use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use std::hash::Hash; +use rustc_data_structures::fx::FxIndexSet; pub fn dce(module: &mut Module) { let mut rooted = collect_roots(module); @@ -138,11 +137,11 @@ fn kill_unrooted(module: &mut Module, rooted: &FxIndexSet) { } } -pub fn dce_phi(blocks: &mut FxIndexMap) { +pub fn dce_phi(func: &mut Function) { let mut used = FxIndexSet::default(); loop { let mut changed = false; - for inst in blocks.values().flat_map(|block| &block.instructions) { + for inst in func.all_inst_iter() { if inst.class.opcode != Op::Phi || used.contains(&inst.result_id.unwrap()) { for op in &inst.operands { if let Some(id) = op.id_ref_any() { @@ -155,7 +154,7 @@ pub fn dce_phi(blocks: &mut FxIndexMap) { break; } } - for block in blocks.values_mut() { + for block in &mut func.blocks { block .instructions .retain(|inst| inst.class.opcode != Op::Phi || used.contains(&inst.result_id.unwrap())); diff --git a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs index df2434d63d9..16889cbcc31 100644 --- a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs +++ b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs @@ -13,14 +13,10 @@ use super::simple_passes::outgoing_edges; use super::{apply_rewrite_rules, id}; use rspirv::dr::{Block, Function, Instruction, ModuleHeader, Operand}; use rspirv::spirv::{Op, Word}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_middle::bug; use std::collections::hash_map; -// HACK(eddyb) newtype instead of type alias to avoid mistakes. -#[derive(Copy, Clone, PartialEq, Eq, Hash)] -struct LabelId(Word); - pub fn mem2reg( header: &mut ModuleHeader, types_global_values: &mut Vec, @@ -28,16 +24,8 @@ pub fn mem2reg( constants: &FxHashMap, func: &mut Function, ) { - // HACK(eddyb) this ad-hoc indexing might be useful elsewhere as well, but - // it's made completely irrelevant by SPIR-T so only applies to legacy code. - let mut blocks: FxIndexMap<_, _> = func - .blocks - .iter_mut() - .map(|block| (LabelId(block.label_id().unwrap()), block)) - .collect(); - - let reachable = compute_reachable(&blocks); - let preds = compute_preds(&blocks, &reachable); + let reachable = compute_reachable(&func.blocks); + let preds = compute_preds(&func.blocks, &reachable); let idom = compute_idom(&preds, &reachable); let dominance_frontier = compute_dominance_frontier(&preds, &idom); loop { @@ -46,27 +34,31 @@ pub fn mem2reg( types_global_values, pointer_to_pointee, constants, - &mut blocks, + &mut func.blocks, &dominance_frontier, ); if !changed { break; } // mem2reg produces minimal SSA form, not pruned, so DCE the dead ones - super::dce::dce_phi(&mut blocks); + super::dce::dce_phi(func); } } -fn compute_reachable(blocks: &FxIndexMap) -> Vec { - fn recurse(blocks: &FxIndexMap, reachable: &mut [bool], block: usize) { +fn label_to_index(blocks: &[Block], id: Word) -> usize { + blocks + .iter() + .position(|b| b.label_id().unwrap() == id) + .unwrap() +} + +fn compute_reachable(blocks: &[Block]) -> Vec { + fn recurse(blocks: &[Block], reachable: &mut [bool], block: usize) { if !reachable[block] { reachable[block] = true; - for dest_id in outgoing_edges(blocks[block]) { - recurse( - blocks, - reachable, - blocks.get_index_of(&LabelId(dest_id)).unwrap(), - ); + for dest_id in outgoing_edges(&blocks[block]) { + let dest_idx = label_to_index(blocks, dest_id); + recurse(blocks, reachable, dest_idx); } } } @@ -75,19 +67,17 @@ fn compute_reachable(blocks: &FxIndexMap) -> Vec { reachable } -fn compute_preds( - blocks: &FxIndexMap, - reachable_blocks: &[bool], -) -> Vec> { +fn compute_preds(blocks: &[Block], reachable_blocks: &[bool]) -> Vec> { let mut result = vec![vec![]; blocks.len()]; // Do not count unreachable blocks as valid preds of blocks for (source_idx, source) in blocks - .values() + .iter() .enumerate() .filter(|&(b, _)| reachable_blocks[b]) { for dest_id in outgoing_edges(source) { - result[blocks.get_index_of(&LabelId(dest_id)).unwrap()].push(source_idx); + let dest_idx = label_to_index(blocks, dest_id); + result[dest_idx].push(source_idx); } } result @@ -171,7 +161,7 @@ fn insert_phis_all( types_global_values: &mut Vec, pointer_to_pointee: &FxHashMap, constants: &FxHashMap, - blocks: &mut FxIndexMap, + blocks: &mut [Block], dominance_frontier: &[FxHashSet], ) -> bool { let var_maps_and_types = blocks[0] @@ -208,11 +198,7 @@ fn insert_phis_all( rewrite_rules: FxHashMap::default(), }; renamer.rename(0, None); - // FIXME(eddyb) shouldn't this full rescan of the function be done once? - apply_rewrite_rules( - &renamer.rewrite_rules, - blocks.values_mut().map(|block| &mut **block), - ); + apply_rewrite_rules(&renamer.rewrite_rules, blocks); remove_nops(blocks); } remove_old_variables(blocks, &var_maps_and_types); @@ -230,7 +216,7 @@ struct VarInfo { fn collect_access_chains( pointer_to_pointee: &FxHashMap, constants: &FxHashMap, - blocks: &FxIndexMap, + blocks: &[Block], base_var: Word, base_var_ty: Word, ) -> Option> { @@ -263,7 +249,7 @@ fn collect_access_chains( // Loop in case a previous block references a later AccessChain loop { let mut changed = false; - for inst in blocks.values().flat_map(|b| &b.instructions) { + for inst in blocks.iter().flat_map(|b| &b.instructions) { for (index, op) in inst.operands.iter().enumerate() { if let Operand::IdRef(id) = op && variables.contains_key(id) @@ -317,10 +303,10 @@ fn collect_access_chains( // same var map (e.g. `s.x = s.y;`). fn split_copy_memory( header: &mut ModuleHeader, - blocks: &mut FxIndexMap, + blocks: &mut [Block], var_map: &FxHashMap, ) { - for block in blocks.values_mut() { + for block in blocks { let mut inst_index = 0; while inst_index < block.instructions.len() { let inst = &block.instructions[inst_index]; @@ -379,7 +365,7 @@ fn has_store(block: &Block, var_map: &FxHashMap) -> bool { } fn insert_phis( - blocks: &FxIndexMap, + blocks: &[Block], dominance_frontier: &[FxHashSet], var_map: &FxHashMap, ) -> FxHashSet { @@ -388,7 +374,7 @@ fn insert_phis( let mut ever_on_work_list = FxHashSet::default(); let mut work_list = Vec::new(); let mut blocks_with_phi = FxHashSet::default(); - for (block_idx, block) in blocks.values().enumerate() { + for (block_idx, block) in blocks.iter().enumerate() { if has_store(block, var_map) { ever_on_work_list.insert(block_idx); work_list.push(block_idx); @@ -433,10 +419,10 @@ fn top_stack_or_undef( } } -struct Renamer<'a, 'b> { +struct Renamer<'a> { header: &'a mut ModuleHeader, types_global_values: &'a mut Vec, - blocks: &'a mut FxIndexMap, + blocks: &'a mut [Block], blocks_with_phi: FxHashSet, base_var_type: Word, var_map: &'a FxHashMap, @@ -446,7 +432,7 @@ struct Renamer<'a, 'b> { rewrite_rules: FxHashMap, } -impl Renamer<'_, '_> { +impl Renamer<'_> { // Returns the phi definition. fn insert_phi_value(&mut self, block: usize, from_block: usize) -> Word { let from_block_label = self.blocks[from_block].label_id().unwrap(); @@ -568,8 +554,9 @@ impl Renamer<'_, '_> { } } - for dest_id in outgoing_edges(self.blocks[block]).collect::>() { - let dest_idx = self.blocks.get_index_of(&LabelId(dest_id)).unwrap(); + for dest_id in outgoing_edges(&self.blocks[block]).collect::>() { + // TODO: Don't do this find + let dest_idx = label_to_index(self.blocks, dest_id); self.rename(dest_idx, Some(block)); } @@ -579,8 +566,8 @@ impl Renamer<'_, '_> { } } -fn remove_nops(blocks: &mut FxIndexMap) { - for block in blocks.values_mut() { +fn remove_nops(blocks: &mut [Block]) { + for block in blocks { block .instructions .retain(|inst| inst.class.opcode != Op::Nop); @@ -588,7 +575,7 @@ fn remove_nops(blocks: &mut FxIndexMap) { } fn remove_old_variables( - blocks: &mut FxIndexMap, + blocks: &mut [Block], var_maps_and_types: &[(FxHashMap, u32)], ) { blocks[0].instructions.retain(|inst| { @@ -599,7 +586,7 @@ fn remove_old_variables( .all(|(var_map, _)| !var_map.contains_key(&result_id)) } }); - for block in blocks.values_mut() { + for block in blocks { block.instructions.retain(|inst| { !matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) || inst.operands.iter().all(|op| { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index b6e3b9a05f3..e347aec7123 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -86,12 +86,9 @@ fn id(header: &mut ModuleHeader) -> Word { result } -fn apply_rewrite_rules<'a>( - rewrite_rules: &FxHashMap, - blocks: impl IntoIterator, -) { +fn apply_rewrite_rules(rewrite_rules: &FxHashMap, blocks: &mut [Block]) { let all_ids_mut = blocks - .into_iter() + .iter_mut() .flat_map(|b| b.label.iter_mut().chain(b.instructions.iter_mut())) .flat_map(|inst| { inst.result_id From 20a4e71bcf753a7e85c6f0690f937550d1cf616d Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 03/14] Revert "WIP: couple of inliner things that need to be disentangled" This reverts commit ea20ef36a7348084a72e8553150f26df16af4361. --- .../rustc_codegen_spirv/src/linker/inline.rs | 297 ++++++++++-------- .../ui/lang/consts/nested-ref.stderr | 5 +- .../core/ref/member_ref_arg-broken.stderr | 9 +- .../ui/lang/core/ref/member_ref_arg.stderr | 5 +- 4 files changed, 173 insertions(+), 143 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index e2aac8c548f..432056c9358 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -8,15 +8,13 @@ use super::apply_rewrite_rules; use super::ipo::CallGraph; use super::simple_passes::outgoing_edges; use super::{get_name, get_names}; -use crate::custom_decorations::SpanRegenerator; 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, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; use smallvec::SmallVec; -use std::cmp::Ordering; use std::mem; // FIXME(eddyb) this is a bit silly, but this keeps being repeated everywhere. @@ -42,10 +40,40 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { }) .map(|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(); + let mut inlined_to_legalize_dont_inlines = Vec::new(); + module.functions.retain(|f| { + let should_inline_f = should_inline(&legal_globals, &functions_that_may_abort, f, None); + if should_inline_f != Ok(false) { + if should_inline_f == Err(MustInlineToLegalize) && has_dont_inline(f) { + inlined_to_legalize_dont_inlines.push(f.def_id().unwrap()); + } + // TODO: We should insert all defined IDs in this function. + dropped_ids.insert(f.def_id().unwrap()); + false + } else { + true + } + }); + + if !inlined_to_legalize_dont_inlines.is_empty() { + let names = get_names(module); + for f in inlined_to_legalize_dont_inlines { + sess.dcx().warn(format!( + "`#[inline(never)]` function `{}` needs to be inlined \ + because it has illegal argument or return types", + get_name(&names, f) + )); + } + } + */ + let legal_globals = LegalGlobal::gather_from_module(module); let header = module.header.as_mut().unwrap(); - // FIXME(eddyb) clippy false positive (separate `map` required for borrowck). #[allow(clippy::map_unwrap_or)] let mut inliner = Inliner { @@ -125,8 +153,6 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { .then_some(func.def_id().unwrap()) }) .collect(), - - inlined_dont_inlines_to_cause_and_callers: FxIndexMap::default(), }; let mut functions: Vec<_> = mem::take(&mut module.functions) @@ -145,52 +171,14 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { module.functions = functions.into_iter().map(|func| func.unwrap()).collect(); - let Inliner { - id_to_name, - inlined_dont_inlines_to_cause_and_callers, - .. - } = inliner; - - let mut span_regen = SpanRegenerator::new(sess.source_map(), module); - for (callee_id, (cause, callers)) in inlined_dont_inlines_to_cause_and_callers { - let callee_name = get_name(&id_to_name, callee_id); - - // HACK(eddyb) `libcore` hides panics behind `#[inline(never)]` `fn`s, - // making this too noisy and useless (since it's an impl detail). - if cause == "panicking" && callee_name.starts_with("core::") { - continue; - } - - let callee_span = span_regen - .src_loc_for_id(callee_id) - .and_then(|src_loc| span_regen.src_loc_to_rustc(src_loc)) - .unwrap_or_default(); - sess.dcx() - .struct_span_warn( - callee_span, - format!("`#[inline(never)]` function `{callee_name}` has been inlined"), - ) - .with_note(format!("inlining was required due to {cause}")) - .with_note(format!( - "called from {}", - callers - .iter() - .enumerate() - .filter_map(|(i, &caller_id)| { - // HACK(eddyb) avoid showing too many names. - match i.cmp(&4) { - Ordering::Less => { - Some(format!("`{}`", get_name(&id_to_name, caller_id))) - } - Ordering::Equal => Some(format!("and {} more", callers.len() - i)), - Ordering::Greater => None, - } - }) - .collect::>() - .join(", ") - )) - .emit(); - } + /* + // Drop OpName etc. for inlined functions + module.debug_names.retain(|inst| { + !inst + .operands + .iter() + .any(|op| op.id_ref_any().is_some_and(|id| dropped_ids.contains(&id))) + });*/ Ok(()) } @@ -373,42 +361,42 @@ fn has_dont_inline(function: &Function) -> bool { /// Helper error type for `should_inline` (see its doc comment). #[derive(Copy, Clone, PartialEq, Eq)] -struct MustInlineToLegalize(&'static str); +struct MustInlineToLegalize; -/// Returns `Ok(true)`/`Err(MustInlineToLegalize(_))` if `callee` should/must be +/// Returns `Ok(true)`/`Err(MustInlineToLegalize)` if `callee` should/must be /// inlined (either in general, or specifically from `call_site`, if provided). /// -/// The distinction made here is that `Err(MustInlineToLegalize(cause))` is -/// very much *not* a heuristic, and inlining is *mandatory* due to `cause` -/// (usually illegal signature/arguments, but also the panicking mechanism). -// -// FIXME(eddyb) the causes here are not fine-grained enough. +/// The distinction made is that `Err(MustInlineToLegalize)` is not a heuristic, +/// and inlining is *mandatory* due to an illegal signature/arguments. fn should_inline( legal_globals: &FxHashMap, functions_that_may_abort: &FxHashSet, callee: &Function, - call_site: CallSite<'_>, + call_site: Option>, ) -> Result { let callee_def = callee.def.as_ref().unwrap(); let callee_control = callee_def.operands[0].unwrap_function_control(); - if functions_that_may_abort.contains(&callee.def_id().unwrap()) { - return Err(MustInlineToLegalize("panicking")); + // HACK(eddyb) this "has a call-site" check ensures entry-points don't get + // accidentally removed as "must inline to legalize" function, but can still + // be inlined into other entry-points (if such an unusual situation arises). + if call_site.is_some() && functions_that_may_abort.contains(&callee.def_id().unwrap()) { + return Err(MustInlineToLegalize); } let ret_ty = legal_globals .get(&callee_def.result_type.unwrap()) - .ok_or(MustInlineToLegalize("illegal return type"))?; + .ok_or(MustInlineToLegalize)?; if !ret_ty.legal_as_fn_ret_ty() { - return Err(MustInlineToLegalize("illegal (pointer) return type")); + return Err(MustInlineToLegalize); } for (i, param) in callee.parameters.iter().enumerate() { let param_ty = legal_globals .get(param.result_type.as_ref().unwrap()) - .ok_or(MustInlineToLegalize("illegal parameter type"))?; + .ok_or(MustInlineToLegalize)?; if !param_ty.legal_as_fn_param_ty() { - return Err(MustInlineToLegalize("illegal (pointer) parameter type")); + return Err(MustInlineToLegalize); } // If the call isn't passing a legal pointer argument (a "memory object", @@ -416,13 +404,13 @@ fn should_inline( // then inlining is required to have a chance at producing legal SPIR-V. // // FIXME(eddyb) rewriting away the pointer could be another alternative. - if let LegalGlobal::TypePointer(_) = param_ty { + if let (LegalGlobal::TypePointer(_), Some(call_site)) = (param_ty, call_site) { let ptr_arg = call_site.call_inst.operands[i + 1].unwrap_id_ref(); match legal_globals.get(&ptr_arg) { Some(LegalGlobal::Variable) => {} // FIXME(eddyb) should some constants (undef/null) be allowed? - Some(_) => return Err(MustInlineToLegalize("illegal (pointer) argument")), + Some(_) => return Err(MustInlineToLegalize), None => { let mut caller_param_and_var_ids = call_site @@ -448,7 +436,7 @@ fn should_inline( .map(|caller_inst| caller_inst.result_id.unwrap()); if !caller_param_and_var_ids.any(|id| ptr_arg == id) { - return Err(MustInlineToLegalize("illegal (pointer) argument")); + return Err(MustInlineToLegalize); } } } @@ -469,7 +457,7 @@ struct FuncIsBeingInlined; // Renumber IDs // Insert blocks -struct Inliner<'a, 'b> { +struct Inliner<'m> { /// ID of `OpExtInstImport` for our custom "extended instruction set" /// (see `crate::custom_insts` for more details). custom_ext_inst_set_import: Word, @@ -481,27 +469,26 @@ struct Inliner<'a, 'b> { /// 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, + 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<&'a str, Word>, + cached_op_strings: FxHashMap<&'m str, Word>, - header: &'b mut ModuleHeader, - debug_string_source: &'b mut Vec, - annotations: &'b mut Vec, - types_global_values: &'b mut Vec, + header: &'m mut ModuleHeader, + debug_string_source: &'m mut Vec, + annotations: &'m mut Vec, + types_global_values: &'m mut Vec, legal_globals: FxHashMap, functions_that_may_abort: FxHashSet, - inlined_dont_inlines_to_cause_and_callers: FxIndexMap)>, // rewrite_rules: FxHashMap, } -impl Inliner<'_, '_> { +impl Inliner<'_> { fn id(&mut self) -> Word { next_id(self.header) } @@ -593,19 +580,10 @@ impl Inliner<'_, '_> { &self.legal_globals, &self.functions_that_may_abort, f, - call_site, + Some(call_site), ) { Ok(inline) => inline, - Err(MustInlineToLegalize(cause)) => { - if has_dont_inline(f) { - self.inlined_dont_inlines_to_cause_and_callers - .entry(f.def_id().unwrap()) - .or_insert_with(|| (cause, Default::default())) - .1 - .insert(caller.def_id().unwrap()); - } - true - } + Err(MustInlineToLegalize) => true, } }); let (call_index, call_inst, callee) = match call { @@ -632,28 +610,18 @@ impl Inliner<'_, '_> { }; let call_result_id = call_inst.result_id.unwrap(); - // Get the debug "source location" instruction that applies to the call. + // Get the debuginfo instructions that apply to the call. + // TODO(eddyb) only one instruction should be necessary here w/ bottom-up. let custom_ext_inst_set_import = self.custom_ext_inst_set_import; - let call_debug_src_loc_inst = caller.blocks[block_idx].instructions[..call_index] + let call_debug_insts = caller.blocks[block_idx].instructions[..call_index] .iter() - .rev() - .find_map(|inst| { - Some(match inst.class.opcode { - Op::Line => Some(inst), - Op::NoLine => None, - Op::ExtInst - if inst.operands[0].unwrap_id_ref() == custom_ext_inst_set_import => - { - match CustomOp::decode_from_ext_inst(inst) { - CustomOp::SetDebugSrcLoc => Some(inst), - CustomOp::ClearDebugSrcLoc => None, - _ => return None, - } - } - _ => return None, - }) - }) - .flatten(); + .filter(|inst| match inst.class.opcode { + Op::Line | Op::NoLine => true, + Op::ExtInst if inst.operands[0].unwrap_id_ref() == custom_ext_inst_set_import => { + CustomOp::decode_from_ext_inst(inst).is_debuginfo() + } + _ => false, + }); // Rewrite parameters to arguments let call_arguments = call_inst @@ -674,12 +642,9 @@ impl Inliner<'_, '_> { }; let return_jump = self.id(); // Rewrite OpReturns of the callee. - let mut inlined_callee_blocks = self.get_inlined_blocks( - callee, - call_debug_src_loc_inst, - return_variable, - return_jump, - ); + #[allow(clippy::needless_borrow)] + let (mut inlined_callee_blocks, extra_debug_insts_pre_call, extra_debug_insts_post_call) = + self.get_inlined_blocks(&callee, call_debug_insts, 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); @@ -701,6 +666,13 @@ impl Inliner<'_, '_> { .unwrap(); assert!(call.class.opcode == Op::FunctionCall); + // HACK(eddyb) inject the additional debuginfo instructions generated by + // `get_inlined_blocks`, so the inlined call frame "stack" isn't corrupted. + caller.blocks[pre_call_block_idx] + .instructions + .extend(extra_debug_insts_pre_call); + post_call_block_insts.splice(0..0, extra_debug_insts_post_call); + 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. @@ -895,19 +867,59 @@ impl Inliner<'_, '_> { } } - fn get_inlined_blocks( + // HACK(eddyb) the second and third return values are additional debuginfo + // instructions that need to be inserted just before/after the callsite. + fn get_inlined_blocks<'a>( &mut self, callee: &Function, - call_debug_src_loc_inst: Option<&Instruction>, + call_debug_insts: impl Iterator, return_variable: Option, return_jump: Word, - ) -> Vec { + ) -> ( + Vec, + SmallVec<[Instruction; 8]>, + SmallVec<[Instruction; 8]>, + ) { let Self { custom_ext_inst_set_import, op_type_void_id, .. } = *self; + // TODO(eddyb) kill this as it shouldn't be needed for bottom-up inline. + // HACK(eddyb) this is terrible, but we have to deal with it because of + // how this inliner is outside-in, instead of inside-out, meaning that + // context builds up "outside" of the callee blocks, inside the caller. + let mut enclosing_inlined_frames = SmallVec::<[_; 8]>::new(); + let mut current_debug_src_loc_inst = None; + for inst in call_debug_insts { + match inst.class.opcode { + Op::Line => current_debug_src_loc_inst = Some(inst), + Op::NoLine => current_debug_src_loc_inst = None, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => current_debug_src_loc_inst = Some(inst), + CustomOp::ClearDebugSrcLoc => current_debug_src_loc_inst = None, + CustomOp::PushInlinedCallFrame => { + enclosing_inlined_frames + .push((current_debug_src_loc_inst.take(), inst)); + } + CustomOp::PopInlinedCallFrame => { + if let Some((callsite_debug_src_loc_inst, _)) = + enclosing_inlined_frames.pop() + { + current_debug_src_loc_inst = callsite_debug_src_loc_inst; + } + } + CustomOp::Abort => {} + } + } + _ => {} + } + } + // 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 @@ -932,7 +944,7 @@ impl Inliner<'_, '_> { )); id }); - let mut mk_debuginfo_prefix_and_suffix = || { + let mut mk_debuginfo_prefix_and_suffix = |include_callee_frame| { // 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 instantiate_debuginfo = |this: &mut Self, inst: &Instruction| { @@ -956,18 +968,33 @@ impl Inliner<'_, '_> { .collect(), ) }; + // FIXME(eddyb) this only allocates to avoid borrow conflicts. + let mut prefix = SmallVec::<[_; 8]>::new(); + let mut suffix = SmallVec::<[_; 8]>::new(); + for &(callsite_debug_src_loc_inst, push_inlined_call_frame_inst) in + &enclosing_inlined_frames + { + prefix.extend( + callsite_debug_src_loc_inst + .into_iter() + .chain([push_inlined_call_frame_inst]) + .map(|inst| instantiate_debuginfo(self, inst)), + ); + suffix.push(custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)); + } + prefix.extend(current_debug_src_loc_inst.map(|inst| instantiate_debuginfo(self, inst))); + + if include_callee_frame { + prefix.push(custom_inst_to_inst( + self, + CustomInst::PushInlinedCallFrame { + callee_name: Operand::IdRef(callee_name_id), + }, + )); + suffix.push(custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)); + } - ( - (call_debug_src_loc_inst.map(|inst| instantiate_debuginfo(self, inst))) - .into_iter() - .chain([custom_inst_to_inst( - self, - CustomInst::PushInlinedCallFrame { - callee_name: Operand::IdRef(callee_name_id), - }, - )]), - [custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)], - ) + (prefix, suffix) }; let mut blocks = callee.blocks.clone(); @@ -1021,7 +1048,7 @@ impl Inliner<'_, '_> { // HACK(eddyb) avoid adding debuginfo to otherwise-empty blocks. if block.instructions.len() > num_phis { - let (debuginfo_prefix, debuginfo_suffix) = mk_debuginfo_prefix_and_suffix(); + let (debuginfo_prefix, debuginfo_suffix) = mk_debuginfo_prefix_and_suffix(true); // Insert the prefix debuginfo instructions after `OpPhi`s, // which sadly can't be covered by them. block @@ -1035,7 +1062,13 @@ impl Inliner<'_, '_> { block.instructions.push(terminator); } - blocks + let (caller_restore_debuginfo_after_call, calleer_reset_debuginfo_before_call) = + mk_debuginfo_prefix_and_suffix(false); + ( + blocks, + calleer_reset_debuginfo_before_call, + caller_restore_debuginfo_after_call, + ) } fn insert_opvariables(&self, block: &mut Block, insts: impl IntoIterator) { diff --git a/tests/compiletests/ui/lang/consts/nested-ref.stderr b/tests/compiletests/ui/lang/consts/nested-ref.stderr index ef34288a66e..786deee6cce 100644 --- a/tests/compiletests/ui/lang/consts/nested-ref.stderr +++ b/tests/compiletests/ui/lang/consts/nested-ref.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `nested_ref::deep_load` has been inlined - --> <$DIR/nested-ref.rs>:12:4 + --> $DIR/nested-ref.rs:12:4 | LL | fn deep_load(r: &'static &'static u32) -> u32 { | ^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn deep_load(r: &'static &'static u32) -> u32 { = note: called from `nested_ref::main` warning: `#[inline(never)]` function `nested_ref::deep_transpose` has been inlined - --> <$DIR/nested-ref.rs>:19:4 + --> $DIR/nested-ref.rs:19:4 | LL | fn deep_transpose(r: &'static &'static Mat2) -> Mat2 { | ^^^^^^^^^^^^^^ @@ -17,4 +17,3 @@ LL | fn deep_transpose(r: &'static &'static Mat2) -> Mat2 { = note: called from `nested_ref::main` warning: 2 warnings emitted - diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr index 17c52a8fabc..c0e85521c01 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `member_ref_arg_broken::f` has been inlined - --> <$DIR/member_ref_arg-broken.rs>:20:4 + --> $DIR/member_ref_arg-broken.rs:20:4 | LL | fn f(x: &u32) -> u32 { | ^ @@ -8,7 +8,7 @@ LL | fn f(x: &u32) -> u32 { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::g` has been inlined - --> <$DIR/member_ref_arg-broken.rs>:25:4 + --> $DIR/member_ref_arg-broken.rs:25:4 | LL | fn g(xy: (&u32, &u32)) -> (u32, u32) { | ^ @@ -17,7 +17,7 @@ LL | fn g(xy: (&u32, &u32)) -> (u32, u32) { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::h` has been inlined - --> <$DIR/member_ref_arg-broken.rs>:30:4 + --> $DIR/member_ref_arg-broken.rs:30:4 | LL | fn h(xyz: (&u32, &u32, &u32)) -> (u32, u32, u32) { | ^ @@ -26,7 +26,7 @@ LL | fn h(xyz: (&u32, &u32, &u32)) -> (u32, u32, u32) { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::h_newtyped` has been inlined - --> <$DIR/member_ref_arg-broken.rs>:41:4 + --> $DIR/member_ref_arg-broken.rs:41:4 | LL | fn h_newtyped(xyz: ((&u32, &u32, &u32),)) -> (u32, u32, u32) { | ^^^^^^^^^^ @@ -42,4 +42,3 @@ error: error:0:0 - OpLoad Pointer '$ID[%$ID]' is not a logical pointer. = note: module `$TEST_BUILD_DIR/lang/core/ref/member_ref_arg-broken` error: aborting due to 1 previous error; 4 warnings emitted - diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr index 7596b6f46a3..4feecc9adf5 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `member_ref_arg::f` has been inlined - --> <$DIR/member_ref_arg.rs>:14:4 + --> $DIR/member_ref_arg.rs:14:4 | LL | fn f(x: &u32) {} | ^ @@ -8,7 +8,7 @@ LL | fn f(x: &u32) {} = note: called from `member_ref_arg::main` warning: `#[inline(never)]` function `member_ref_arg::g` has been inlined - --> <$DIR/member_ref_arg.rs>:17:4 + --> $DIR/member_ref_arg.rs:17:4 | LL | fn g(xy: (&u32, &u32)) {} | ^ @@ -17,4 +17,3 @@ LL | fn g(xy: (&u32, &u32)) {} = note: called from `member_ref_arg::main` warning: 2 warnings emitted - From 61cdafd0f48640a74fbd4093e1ea8577af566fcd Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 04/14] Revert "WIP: (TODO: finish bottom-up cleanups) bottom-up inlining" This reverts commit 41ec7ea82b5bdae99879a2aa9715dc73e302c741. --- .../rustc_codegen_spirv/src/linker/inline.rs | 173 ++++++++---------- crates/rustc_codegen_spirv/src/linker/ipo.rs | 16 +- .../ui/dis/panic_builtin_bounds_check.stderr | 86 ++++----- .../ui/lang/consts/nested-ref.stderr | 1 + .../core/ref/member_ref_arg-broken.stderr | 1 + 5 files changed, 127 insertions(+), 150 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 432056c9358..1e8ddbc5b54 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -17,6 +17,8 @@ use rustc_session::Session; use smallvec::SmallVec; use std::mem; +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; @@ -28,9 +30,6 @@ 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)?; - // Compute the call-graph that will drive (inside-out, aka bottom-up) inlining. - let (call_graph, func_id_to_idx) = CallGraph::collect_with_func_id_to_idx(module); - let custom_ext_inst_set_import = module .ext_inst_imports .iter() @@ -40,7 +39,62 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { }) .map(|inst| inst.result_id.unwrap()); - /* + // HACK(eddyb) compute the set of functions that may `Abort` *transitively*, + // which is only needed because of how we inline (sometimes it's outside-in, + // aka top-down, instead of always being inside-out, aka bottom-up). + // + // (inlining is needed in the first place because our custom `Abort` + // instructions get lowered to a simple `OpReturn` in entry-points, but + // that requires that they get inlined all the way up to the entry-points) + let functions_that_may_abort = custom_ext_inst_set_import + .map(|custom_ext_inst_set_import| { + let mut may_abort_by_id = FxHashSet::default(); + + // FIXME(eddyb) use this `CallGraph` abstraction more during inlining. + let call_graph = CallGraph::collect(module); + for func_idx in call_graph.post_order() { + let func_id = module.functions[func_idx].def_id().unwrap(); + + let any_callee_may_abort = call_graph.callees[func_idx].iter().any(|&callee_idx| { + may_abort_by_id.contains(&module.functions[callee_idx].def_id().unwrap()) + }); + if any_callee_may_abort { + may_abort_by_id.insert(func_id); + continue; + } + + let may_abort_directly = module.functions[func_idx].blocks.iter().any(|block| { + match &block.instructions[..] { + [.., last_normal_inst, terminator_inst] + if last_normal_inst.class.opcode == Op::ExtInst + && last_normal_inst.operands[0].unwrap_id_ref() + == custom_ext_inst_set_import + && CustomOp::decode_from_ext_inst(last_normal_inst) + == CustomOp::Abort => + { + assert_eq!(terminator_inst.class.opcode, Op::Unreachable); + true + } + + _ => false, + } + }); + if may_abort_directly { + may_abort_by_id.insert(func_id); + } + } + + may_abort_by_id + }) + .unwrap_or_default(); + + let functions = module + .functions + .iter() + .map(|f| (f.def_id().unwrap(), f.clone())) + .collect(); + let legal_globals = LegalGlobal::gather_from_module(module); + // 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(); @@ -69,9 +123,6 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { )); } } - */ - - let legal_globals = LegalGlobal::gather_from_module(module); let header = module.header.as_mut().unwrap(); // FIXME(eddyb) clippy false positive (separate `map` required for borrowck). @@ -103,8 +154,6 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { id }), - func_id_to_idx, - id_to_name: module .debug_names .iter() @@ -124,61 +173,22 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { annotations: &mut module.annotations, types_global_values: &mut module.types_global_values, - legal_globals, - - // NOTE(eddyb) this is needed because our custom `Abort` instructions get - // lowered to a simple `OpReturn` in entry-points, but that requires that - // they get inlined all the way up to the entry-points in the first place. - functions_that_may_abort: module - .functions - .iter() - .filter_map(|func| { - let custom_ext_inst_set_import = custom_ext_inst_set_import?; - func.blocks - .iter() - .any(|block| match &block.instructions[..] { - [.., last_normal_inst, terminator_inst] - if last_normal_inst.class.opcode == Op::ExtInst - && last_normal_inst.operands[0].unwrap_id_ref() - == custom_ext_inst_set_import - && CustomOp::decode_from_ext_inst(last_normal_inst) - == CustomOp::Abort => - { - assert_eq!(terminator_inst.class.opcode, Op::Unreachable); - true - } - - _ => false, - }) - .then_some(func.def_id().unwrap()) - }) - .collect(), + functions: &functions, + legal_globals: &legal_globals, + functions_that_may_abort: &functions_that_may_abort, }; - - let mut functions: Vec<_> = mem::take(&mut module.functions) - .into_iter() - .map(Ok) - .collect(); - - // Inline functions in post-order (aka inside-out aka bottom-out) - that is, - // callees are processed before their callers, to avoid duplicating work. - for func_idx in call_graph.post_order() { - let mut function = mem::replace(&mut functions[func_idx], Err(FuncIsBeingInlined)).unwrap(); - inliner.inline_fn(&mut function, &functions); - fuse_trivial_branches(&mut function); - functions[func_idx] = Ok(function); + for function in &mut module.functions { + inliner.inline_fn(function); + fuse_trivial_branches(function); } - module.functions = functions.into_iter().map(|func| func.unwrap()).collect(); - - /* // Drop OpName etc. for inlined functions module.debug_names.retain(|inst| { !inst .operands .iter() .any(|op| op.id_ref_any().is_some_and(|id| dropped_ids.contains(&id))) - });*/ + }); Ok(()) } @@ -446,27 +456,19 @@ fn should_inline( Ok(callee_control.contains(FunctionControl::INLINE)) } -/// Helper error type for `Inliner`'s `functions` field, indicating a `Function` -/// was taken out of its slot because it's being inlined. -#[derive(Debug)] -struct FuncIsBeingInlined; - // Steps: // Move OpVariable decls // Rewrite return // Renumber IDs // Insert blocks -struct Inliner<'m> { +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, - /// Map from each function's ID to its index in `functions`. - func_id_to_idx: FxHashMap, - /// 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, @@ -483,12 +485,13 @@ struct Inliner<'m> { annotations: &'m mut Vec, types_global_values: &'m mut Vec, - legal_globals: FxHashMap, - functions_that_may_abort: FxHashSet, + functions: &'map FunctionMap, + legal_globals: &'map FxHashMap, + functions_that_may_abort: &'map FxHashSet, // rewrite_rules: FxHashMap, } -impl Inliner<'_> { +impl Inliner<'_, '_> { fn id(&mut self) -> Word { next_id(self.header) } @@ -533,29 +536,19 @@ impl Inliner<'_> { inst_id } - fn inline_fn( - &mut self, - function: &mut Function, - functions: &[Result], - ) { + fn inline_fn(&mut self, function: &mut Function) { let mut block_idx = 0; while block_idx < function.blocks.len() { // If we successfully inlined a block, then repeat processing on the same block, in // case the newly inlined block has more inlined calls. // TODO: This is quadratic - if !self.inline_block(function, block_idx, functions) { - // TODO(eddyb) skip past the inlined callee without rescanning it. + if !self.inline_block(function, block_idx) { block_idx += 1; } } } - fn inline_block( - &mut self, - caller: &mut Function, - block_idx: usize, - functions: &[Result], - ) -> bool { + fn inline_block(&mut self, caller: &mut Function, block_idx: usize) -> bool { // Find the first inlined OpFunctionCall let call = caller.blocks[block_idx] .instructions @@ -566,8 +559,8 @@ impl Inliner<'_> { ( index, inst, - functions[self.func_id_to_idx[&inst.operands[0].id_ref_any().unwrap()]] - .as_ref() + self.functions + .get(&inst.operands[0].id_ref_any().unwrap()) .unwrap(), ) }) @@ -577,8 +570,8 @@ impl Inliner<'_> { call_inst: inst, }; match should_inline( - &self.legal_globals, - &self.functions_that_may_abort, + self.legal_globals, + self.functions_that_may_abort, f, Some(call_site), ) { @@ -590,16 +583,6 @@ impl Inliner<'_> { None => return false, Some(call) => call, }; - - // Propagate "may abort" from callee to caller (i.e. as aborts get inlined). - if self - .functions_that_may_abort - .contains(&callee.def_id().unwrap()) - { - self.functions_that_may_abort - .insert(caller.def_id().unwrap()); - } - let call_result_type = { let ty = call_inst.result_type.unwrap(); if ty == self.op_type_void_id { @@ -611,7 +594,6 @@ impl Inliner<'_> { let call_result_id = call_inst.result_id.unwrap(); // Get the debuginfo instructions that apply to the call. - // TODO(eddyb) only one instruction should be necessary here w/ bottom-up. let custom_ext_inst_set_import = self.custom_ext_inst_set_import; let call_debug_insts = caller.blocks[block_idx].instructions[..call_index] .iter() @@ -886,7 +868,6 @@ impl Inliner<'_> { .. } = *self; - // TODO(eddyb) kill this as it shouldn't be needed for bottom-up inline. // HACK(eddyb) this is terrible, but we have to deal with it because of // how this inliner is outside-in, instead of inside-out, meaning that // context builds up "outside" of the callee blocks, inside the caller. diff --git a/crates/rustc_codegen_spirv/src/linker/ipo.rs b/crates/rustc_codegen_spirv/src/linker/ipo.rs index 96c7bbe6aed..475f5c50c15 100644 --- a/crates/rustc_codegen_spirv/src/linker/ipo.rs +++ b/crates/rustc_codegen_spirv/src/linker/ipo.rs @@ -4,7 +4,7 @@ use indexmap::IndexSet; use rspirv::dr::Module; -use rspirv::spirv::{Op, Word}; +use rspirv::spirv::Op; use rustc_data_structures::fx::FxHashMap; // FIXME(eddyb) use newtyped indices and `IndexVec`. @@ -19,9 +19,6 @@ pub struct CallGraph { impl CallGraph { pub fn collect(module: &Module) -> Self { - Self::collect_with_func_id_to_idx(module).0 - } - pub fn collect_with_func_id_to_idx(module: &Module) -> (Self, FxHashMap) { let func_id_to_idx: FxHashMap<_, _> = module .functions .iter() @@ -54,13 +51,10 @@ impl CallGraph { .collect() }) .collect(); - ( - Self { - entry_points, - callees, - }, - func_id_to_idx, - ) + Self { + entry_points, + callees, + } } /// Order functions using a post-order traversal, i.e. callees before callers. diff --git a/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr b/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr index 2a6a8e7746e..6486f832eb2 100644 --- a/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr +++ b/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr @@ -4,53 +4,53 @@ OpExtension "SPV_KHR_non_semantic_info" OpMemoryModel Logical Simple OpEntryPoint Fragment %2 "main" OpExecutionMode %2 OriginUpperLeft -%3 = OpString "/n[Rust panicked at $DIR/panic_builtin_bounds_check.rs:27:5]/n index out of bounds: the len is %u but the index is %u/n in main()/n" -%4 = OpString "$DIR/panic_builtin_bounds_check.rs" -OpDecorate %5 ArrayStride 4 -%6 = OpTypeVoid -%7 = OpTypeFunction %6 -%8 = OpTypeInt 32 0 -%9 = OpConstant %8 4 -%10 = OpTypeArray %8 %9 -%11 = OpTypePointer Function %10 -%5 = OpTypeArray %8 %9 -%12 = OpConstant %8 0 -%13 = OpConstant %8 1 -%14 = OpConstant %8 2 -%15 = OpConstant %8 3 -%16 = OpTypeBool -%17 = OpConstant %8 5 -%18 = OpTypePointer Function %8 -%2 = OpFunction %6 None %7 -%19 = OpLabel -OpLine %4 32 4 -%20 = OpVariable %11 Function -OpLine %4 32 23 -%21 = OpCompositeConstruct %5 %12 %13 %14 %15 -OpLine %4 32 4 -%22 = OpCompositeExtract %8 %21 0 -%23 = OpCompositeExtract %8 %21 1 -%24 = OpCompositeExtract %8 %21 2 -%25 = OpCompositeExtract %8 %21 3 -%26 = OpCompositeConstruct %10 %22 %23 %24 %25 -OpStore %20 %26 -OpLine %4 27 4 -%27 = OpULessThan %16 %17 %9 +%3 = OpString "/n[Rust panicked at $SYSROOT/lib/rustlib/src/rust/library/core/src/panicking.rs:276:5]/n index out of bounds: the len is %u but the index is %u/n in main()/n" +%4 = OpString $SYSROOT/lib/rustlib/src/rust/library/core/src/panicking.rs" +%5 = OpString "$DIR/panic_builtin_bounds_check.rs" +OpDecorate %6 ArrayStride 4 +%7 = OpTypeVoid +%8 = OpTypeFunction %7 +%9 = OpTypeInt 32 0 +%10 = OpConstant %9 4 +%11 = OpTypeArray %9 %10 +%12 = OpTypePointer Function %11 +%6 = OpTypeArray %9 %10 +%13 = OpConstant %9 0 +%14 = OpConstant %9 1 +%15 = OpConstant %9 2 +%16 = OpConstant %9 3 +%17 = OpTypeBool +%18 = OpConstant %9 5 +%19 = OpTypePointer Function %9 +%2 = OpFunction %7 None %8 +%20 = OpLabel +OpLine %5 32 4 +%21 = OpVariable %12 Function +OpLine %5 32 23 +%22 = OpCompositeConstruct %6 %13 %14 %15 %16 +OpLine %5 27 4 +%23 = OpCompositeExtract %9 %22 0 +%24 = OpCompositeExtract %9 %22 1 +%25 = OpCompositeExtract %9 %22 2 +%26 = OpCompositeExtract %9 %22 3 +%27 = OpCompositeConstruct %11 %23 %24 %25 %26 +OpStore %21 %27 +%28 = OpULessThan %17 %18 %10 OpNoLine -OpSelectionMerge %28 None -OpBranchConditional %27 %29 %30 -%29 = OpLabel -OpBranch %28 +OpSelectionMerge %29 None +OpBranchConditional %28 %30 %31 %30 = OpLabel -OpLine %4 27 4 -%31 = OpExtInst %6 %1 1 %3 %9 %17 +OpBranch %29 +%31 = OpLabel +OpLine %4 276 4 +%32 = OpExtInst %7 %1 1 %3 %10 %18 OpNoLine OpReturn -%28 = OpLabel -OpLine %4 27 4 -%32 = OpIAdd %8 %12 %17 -%33 = OpInBoundsAccessChain %18 %20 %32 -%34 = OpLoad %8 %33 +%29 = OpLabel +OpLine %5 27 4 +%33 = OpIAdd %9 %13 %18 +%34 = OpInBoundsAccessChain %19 %21 %33 +%35 = OpLoad %9 %34 OpNoLine OpReturn OpFunctionEnd diff --git a/tests/compiletests/ui/lang/consts/nested-ref.stderr b/tests/compiletests/ui/lang/consts/nested-ref.stderr index 786deee6cce..805ec9b6076 100644 --- a/tests/compiletests/ui/lang/consts/nested-ref.stderr +++ b/tests/compiletests/ui/lang/consts/nested-ref.stderr @@ -17,3 +17,4 @@ LL | fn deep_transpose(r: &'static &'static Mat2) -> Mat2 { = note: called from `nested_ref::main` warning: 2 warnings emitted + diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr index c0e85521c01..c46b2e0276e 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr @@ -42,3 +42,4 @@ error: error:0:0 - OpLoad Pointer '$ID[%$ID]' is not a logical pointer. = note: module `$TEST_BUILD_DIR/lang/core/ref/member_ref_arg-broken` error: aborting due to 1 previous error; 4 warnings emitted + From 0c93d1118d26ab8773a87203c6a5297741e14b14 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 05/14] Revert "linker/inline: fix `OpVariable` debuginfo collection and insertion." This reverts commit 355122de8b7b940d987eb499e11a74ca3fbe1dc3. --- .../rustc_codegen_spirv/src/linker/inline.rs | 202 +++++------------- 1 file changed, 52 insertions(+), 150 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 1e8ddbc5b54..a1892875ab3 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -15,7 +15,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; use smallvec::SmallVec; -use std::mem; +use std::mem::{self, take}; type FunctionMap = FxHashMap; @@ -658,13 +658,15 @@ 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. - let ret_var_inst = Instruction::new( - Op::Variable, - Some(self.ptr_ty(call_result_type)), - Some(return_variable.unwrap()), - vec![Operand::StorageClass(StorageClass::Function)], + insert_opvariables( + &mut caller.blocks[0], + [Instruction::new( + Op::Variable, + Some(self.ptr_ty(call_result_type)), + Some(return_variable.unwrap()), + vec![Operand::StorageClass(StorageClass::Function)], + )], ); - self.insert_opvariables(&mut caller.blocks[0], [ret_var_inst]); } // Insert non-entry inlined callee blocks just after the pre-call block. @@ -710,115 +712,52 @@ 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. { - // NOTE(eddyb) `OpExtInst`s have a result ID, even if unused, and - // it has to be unique, so this allocates new IDs as-needed. - let instantiate_debuginfo = |this: &mut Self, inst: &Instruction| { - let mut inst = inst.clone(); - if let Some(id) = &mut inst.result_id { - *id = this.id(); - } - inst - }; - - let custom_inst_to_inst = |this: &mut Self, inst: CustomInst<_>| { - Instruction::new( - Op::ExtInst, - Some(this.op_type_void_id), - Some(this.id()), - [ - Operand::IdRef(this.custom_ext_inst_set_import), - Operand::LiteralExtInstInteger(inst.op() as u32), - ] - .into_iter() - .chain(inst.into_operands()) - .collect(), - ) - }; - // 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| { - // HACK(eddyb) this duplicates some code from `get_inlined_blocks`, - // but that will be removed once the inliner is refactored to be - // inside-out instead of outside-in (already finished in a branch). - let mut enclosing_inlined_frames = SmallVec::<[_; 8]>::new(); - let mut current_debug_src_loc_inst = None; - let mut vars_and_debuginfo_range = 0..0; - while vars_and_debuginfo_range.end < insts.len() { - let inst = &insts[vars_and_debuginfo_range.end]; - match inst.class.opcode { - Op::Line => current_debug_src_loc_inst = Some(inst), - Op::NoLine => current_debug_src_loc_inst = None, - Op::ExtInst - if inst.operands[0].unwrap_id_ref() - == self.custom_ext_inst_set_import => - { - match CustomOp::decode_from_ext_inst(inst) { - CustomOp::SetDebugSrcLoc => current_debug_src_loc_inst = Some(inst), - CustomOp::ClearDebugSrcLoc => current_debug_src_loc_inst = None, - CustomOp::PushInlinedCallFrame => { - enclosing_inlined_frames - .push((current_debug_src_loc_inst.take(), inst)); - } - CustomOp::PopInlinedCallFrame => { - if let Some((callsite_debug_src_loc_inst, _)) = - enclosing_inlined_frames.pop() - { - current_debug_src_loc_inst = callsite_debug_src_loc_inst; - } - } - CustomOp::Abort => break, - } + let mut vars_and_debuginfo = vec![]; + insts.retain_mut(|inst| { + let is_debuginfo = match inst.class.opcode { + Op::Line | Op::NoLine => true, + Op::ExtInst => { + inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import + && CustomOp::decode_from_ext_inst(inst).is_debuginfo() } - Op::Variable => {} - _ => break, + _ => 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_range.end += 1; - } - - // `vars_and_debuginfo_range.end` indicates where `OpVariable`s - // end and other instructions start (module debuginfo), but to - // split the block in two, both sides of the "cut" need "repair": - // - the variables are missing "inlined call frames" pops, that - // may happen later in the block, and have to be synthesized - // - the non-variables are missing "inlined call frames" pushes, - // that must be recreated to avoid ending up with dangling pops - // - // FIXME(eddyb) this only collects to avoid borrow conflicts, - // between e.g. `enclosing_inlined_frames` and mutating `insts`, - // but also between different uses of `self`. - let all_pops_after_vars: SmallVec<[_; 8]> = enclosing_inlined_frames - .iter() - .map(|_| custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)) - .collect(); - let all_repushes_before_non_vars: SmallVec<[_; 8]> = - (enclosing_inlined_frames.into_iter().flat_map( - |(callsite_debug_src_loc_inst, push_inlined_call_frame_inst)| { - (callsite_debug_src_loc_inst.into_iter()) - .chain([push_inlined_call_frame_inst]) - }, - )) - .chain(current_debug_src_loc_inst) - .map(|inst| instantiate_debuginfo(self, inst)) - .collect(); - - let vars_and_debuginfo = - insts.splice(vars_and_debuginfo_range, all_repushes_before_non_vars); - let repaired_vars_and_debuginfo = vars_and_debuginfo.chain(all_pops_after_vars); - - // FIXME(eddyb) collecting shouldn't be necessary but this is - // nested in a closure, and `splice` borrows the original `Vec`. - repaired_vars_and_debuginfo.collect::>() + }); + vars_and_debuginfo }; let [mut inlined_callee_entry_block]: [_; 1] = inlined_callee_blocks.try_into().unwrap(); // Move the `OpVariable`s of the callee to the caller. - let callee_vars_and_debuginfo = - steal_vars(&mut inlined_callee_entry_block.instructions); - self.insert_opvariables(&mut caller.blocks[0], callee_vars_and_debuginfo); + insert_opvariables( + &mut caller.blocks[0], + steal_vars(&mut inlined_callee_entry_block.instructions), + ); caller.blocks[pre_call_block_idx] .instructions @@ -1051,52 +990,15 @@ impl Inliner<'_, '_> { caller_restore_debuginfo_after_call, ) } +} - fn insert_opvariables(&self, block: &mut Block, insts: impl IntoIterator) { - // HACK(eddyb) this isn't as efficient as it could be in theory, but it's - // very important to make sure sure to never insert new instructions in - // the middle of debuginfo (as it would be affected by it). - let mut inlined_frames_depth = 0usize; - let mut outermost_has_debug_src_loc = false; - let mut last_debugless_var_insertion_point_candidate = None; - for (i, inst) in block.instructions.iter().enumerate() { - last_debugless_var_insertion_point_candidate = - (inlined_frames_depth == 0 && !outermost_has_debug_src_loc).then_some(i); - - let changed_has_debug_src_loc = match inst.class.opcode { - Op::Line => true, - Op::NoLine => false, - Op::ExtInst - if inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import => - { - match CustomOp::decode_from_ext_inst(inst) { - CustomOp::SetDebugSrcLoc => true, - CustomOp::ClearDebugSrcLoc => false, - CustomOp::PushInlinedCallFrame => { - inlined_frames_depth += 1; - continue; - } - CustomOp::PopInlinedCallFrame => { - inlined_frames_depth = inlined_frames_depth.saturating_sub(1); - continue; - } - CustomOp::Abort => break, - } - } - Op::Variable => continue, - _ => break, - }; - - if inlined_frames_depth == 0 { - outermost_has_debug_src_loc = changed_has_debug_src_loc; - } - } - - // HACK(eddyb) fallback to inserting at the start, which should be correct. - // FIXME(eddyb) some level of debuginfo repair could prevent needing this. - let i = last_debugless_var_insertion_point_candidate.unwrap_or(0); - block.instructions.splice(i..i, insts); - } +fn insert_opvariables(block: &mut Block, insts: impl IntoIterator) { + let first_non_variable = block + .instructions + .iter() + .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) { @@ -1131,7 +1033,7 @@ fn fuse_trivial_branches(function: &mut Function) { }; let pred_insts = &function.blocks[pred].instructions; if pred_insts.last().unwrap().class.opcode == Op::Branch { - let mut dest_insts = mem::take(&mut function.blocks[dest_block].instructions); + let mut dest_insts = take(&mut function.blocks[dest_block].instructions); let pred_insts = &mut function.blocks[pred].instructions; pred_insts.pop(); // pop the branch pred_insts.append(&mut dest_insts); From aec6a0281f3c23d83cc5b85b20e0bcb563622b3e Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 06/14] [2024] linker/inline: fix `OpVariable` debuginfo collection and insertion. --- .../rustc_codegen_spirv/src/linker/inline.rs | 202 +++++++++++++----- 1 file changed, 150 insertions(+), 52 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index a1892875ab3..1e8ddbc5b54 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -15,7 +15,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; use smallvec::SmallVec; -use std::mem::{self, take}; +use std::mem; type FunctionMap = FxHashMap; @@ -658,15 +658,13 @@ 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_opvariables( - &mut caller.blocks[0], - [Instruction::new( - Op::Variable, - Some(self.ptr_ty(call_result_type)), - Some(return_variable.unwrap()), - vec![Operand::StorageClass(StorageClass::Function)], - )], + let ret_var_inst = Instruction::new( + Op::Variable, + Some(self.ptr_ty(call_result_type)), + Some(return_variable.unwrap()), + vec![Operand::StorageClass(StorageClass::Function)], ); + self.insert_opvariables(&mut caller.blocks[0], [ret_var_inst]); } // Insert non-entry inlined callee blocks just after the pre-call block. @@ -712,52 +710,115 @@ 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. { + // NOTE(eddyb) `OpExtInst`s have a result ID, even if unused, and + // it has to be unique, so this allocates new IDs as-needed. + let instantiate_debuginfo = |this: &mut Self, inst: &Instruction| { + let mut inst = inst.clone(); + if let Some(id) = &mut inst.result_id { + *id = this.id(); + } + inst + }; + + let custom_inst_to_inst = |this: &mut Self, inst: CustomInst<_>| { + Instruction::new( + Op::ExtInst, + Some(this.op_type_void_id), + Some(this.id()), + [ + Operand::IdRef(this.custom_ext_inst_set_import), + Operand::LiteralExtInstInteger(inst.op() as u32), + ] + .into_iter() + .chain(inst.into_operands()) + .collect(), + ) + }; + // 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 => { - inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import - && CustomOp::decode_from_ext_inst(inst).is_debuginfo() - } - _ => 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(); + // HACK(eddyb) this duplicates some code from `get_inlined_blocks`, + // but that will be removed once the inliner is refactored to be + // inside-out instead of outside-in (already finished in a branch). + let mut enclosing_inlined_frames = SmallVec::<[_; 8]>::new(); + let mut current_debug_src_loc_inst = None; + let mut vars_and_debuginfo_range = 0..0; + while vars_and_debuginfo_range.end < insts.len() { + let inst = &insts[vars_and_debuginfo_range.end]; + match inst.class.opcode { + Op::Line => current_debug_src_loc_inst = Some(inst), + Op::NoLine => current_debug_src_loc_inst = None, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() + == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => current_debug_src_loc_inst = Some(inst), + CustomOp::ClearDebugSrcLoc => current_debug_src_loc_inst = None, + CustomOp::PushInlinedCallFrame => { + enclosing_inlined_frames + .push((current_debug_src_loc_inst.take(), inst)); + } + CustomOp::PopInlinedCallFrame => { + if let Some((callsite_debug_src_loc_inst, _)) = + enclosing_inlined_frames.pop() + { + current_debug_src_loc_inst = callsite_debug_src_loc_inst; + } + } + CustomOp::Abort => break, + } } - 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 + Op::Variable => {} + _ => break, } - }); - vars_and_debuginfo + vars_and_debuginfo_range.end += 1; + } + + // `vars_and_debuginfo_range.end` indicates where `OpVariable`s + // end and other instructions start (module debuginfo), but to + // split the block in two, both sides of the "cut" need "repair": + // - the variables are missing "inlined call frames" pops, that + // may happen later in the block, and have to be synthesized + // - the non-variables are missing "inlined call frames" pushes, + // that must be recreated to avoid ending up with dangling pops + // + // FIXME(eddyb) this only collects to avoid borrow conflicts, + // between e.g. `enclosing_inlined_frames` and mutating `insts`, + // but also between different uses of `self`. + let all_pops_after_vars: SmallVec<[_; 8]> = enclosing_inlined_frames + .iter() + .map(|_| custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)) + .collect(); + let all_repushes_before_non_vars: SmallVec<[_; 8]> = + (enclosing_inlined_frames.into_iter().flat_map( + |(callsite_debug_src_loc_inst, push_inlined_call_frame_inst)| { + (callsite_debug_src_loc_inst.into_iter()) + .chain([push_inlined_call_frame_inst]) + }, + )) + .chain(current_debug_src_loc_inst) + .map(|inst| instantiate_debuginfo(self, inst)) + .collect(); + + let vars_and_debuginfo = + insts.splice(vars_and_debuginfo_range, all_repushes_before_non_vars); + let repaired_vars_and_debuginfo = vars_and_debuginfo.chain(all_pops_after_vars); + + // FIXME(eddyb) collecting shouldn't be necessary but this is + // nested in a closure, and `splice` borrows the original `Vec`. + repaired_vars_and_debuginfo.collect::>() }; let [mut inlined_callee_entry_block]: [_; 1] = inlined_callee_blocks.try_into().unwrap(); // Move the `OpVariable`s of the callee to the caller. - insert_opvariables( - &mut caller.blocks[0], - steal_vars(&mut inlined_callee_entry_block.instructions), - ); + let callee_vars_and_debuginfo = + steal_vars(&mut inlined_callee_entry_block.instructions); + self.insert_opvariables(&mut caller.blocks[0], callee_vars_and_debuginfo); caller.blocks[pre_call_block_idx] .instructions @@ -990,15 +1051,52 @@ impl Inliner<'_, '_> { caller_restore_debuginfo_after_call, ) } -} -fn insert_opvariables(block: &mut Block, insts: impl IntoIterator) { - let first_non_variable = block - .instructions - .iter() - .position(|inst| inst.class.opcode != Op::Variable); - let i = first_non_variable.unwrap_or(block.instructions.len()); - block.instructions.splice(i..i, insts); + fn insert_opvariables(&self, block: &mut Block, insts: impl IntoIterator) { + // HACK(eddyb) this isn't as efficient as it could be in theory, but it's + // very important to make sure sure to never insert new instructions in + // the middle of debuginfo (as it would be affected by it). + let mut inlined_frames_depth = 0usize; + let mut outermost_has_debug_src_loc = false; + let mut last_debugless_var_insertion_point_candidate = None; + for (i, inst) in block.instructions.iter().enumerate() { + last_debugless_var_insertion_point_candidate = + (inlined_frames_depth == 0 && !outermost_has_debug_src_loc).then_some(i); + + let changed_has_debug_src_loc = match inst.class.opcode { + Op::Line => true, + Op::NoLine => false, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => true, + CustomOp::ClearDebugSrcLoc => false, + CustomOp::PushInlinedCallFrame => { + inlined_frames_depth += 1; + continue; + } + CustomOp::PopInlinedCallFrame => { + inlined_frames_depth = inlined_frames_depth.saturating_sub(1); + continue; + } + CustomOp::Abort => break, + } + } + Op::Variable => continue, + _ => break, + }; + + if inlined_frames_depth == 0 { + outermost_has_debug_src_loc = changed_has_debug_src_loc; + } + } + + // HACK(eddyb) fallback to inserting at the start, which should be correct. + // FIXME(eddyb) some level of debuginfo repair could prevent needing this. + let i = last_debugless_var_insertion_point_candidate.unwrap_or(0); + block.instructions.splice(i..i, insts); + } } fn fuse_trivial_branches(function: &mut Function) { @@ -1033,7 +1131,7 @@ fn fuse_trivial_branches(function: &mut Function) { }; let pred_insts = &function.blocks[pred].instructions; if pred_insts.last().unwrap().class.opcode == Op::Branch { - let mut dest_insts = take(&mut function.blocks[dest_block].instructions); + let mut dest_insts = mem::take(&mut function.blocks[dest_block].instructions); let pred_insts = &mut function.blocks[pred].instructions; pred_insts.pop(); // pop the branch pred_insts.append(&mut dest_insts); From 3920ddc11b08532bd58282166514854056830509 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 07/14] [2024] linker/inline: use bottom-up inlining to minimize redundancy. --- .../rustc_codegen_spirv/src/linker/inline.rs | 448 +++++++++--------- crates/rustc_codegen_spirv/src/linker/ipo.rs | 16 +- .../ui/lang/core/ref/member_ref_arg.stderr | 1 + 3 files changed, 229 insertions(+), 236 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 1e8ddbc5b54..e2aac8c548f 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -8,17 +8,17 @@ use super::apply_rewrite_rules; use super::ipo::CallGraph; use super::simple_passes::outgoing_edges; use super::{get_name, get_names}; +use crate::custom_decorations::SpanRegenerator; 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}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet}; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; use smallvec::SmallVec; +use std::cmp::Ordering; use std::mem; -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; @@ -30,6 +30,9 @@ 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)?; + // Compute the call-graph that will drive (inside-out, aka bottom-up) inlining. + let (call_graph, func_id_to_idx) = CallGraph::collect_with_func_id_to_idx(module); + let custom_ext_inst_set_import = module .ext_inst_imports .iter() @@ -39,92 +42,10 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { }) .map(|inst| inst.result_id.unwrap()); - // HACK(eddyb) compute the set of functions that may `Abort` *transitively*, - // which is only needed because of how we inline (sometimes it's outside-in, - // aka top-down, instead of always being inside-out, aka bottom-up). - // - // (inlining is needed in the first place because our custom `Abort` - // instructions get lowered to a simple `OpReturn` in entry-points, but - // that requires that they get inlined all the way up to the entry-points) - let functions_that_may_abort = custom_ext_inst_set_import - .map(|custom_ext_inst_set_import| { - let mut may_abort_by_id = FxHashSet::default(); - - // FIXME(eddyb) use this `CallGraph` abstraction more during inlining. - let call_graph = CallGraph::collect(module); - for func_idx in call_graph.post_order() { - let func_id = module.functions[func_idx].def_id().unwrap(); - - let any_callee_may_abort = call_graph.callees[func_idx].iter().any(|&callee_idx| { - may_abort_by_id.contains(&module.functions[callee_idx].def_id().unwrap()) - }); - if any_callee_may_abort { - may_abort_by_id.insert(func_id); - continue; - } - - let may_abort_directly = module.functions[func_idx].blocks.iter().any(|block| { - match &block.instructions[..] { - [.., last_normal_inst, terminator_inst] - if last_normal_inst.class.opcode == Op::ExtInst - && last_normal_inst.operands[0].unwrap_id_ref() - == custom_ext_inst_set_import - && CustomOp::decode_from_ext_inst(last_normal_inst) - == CustomOp::Abort => - { - assert_eq!(terminator_inst.class.opcode, Op::Unreachable); - true - } - - _ => false, - } - }); - if may_abort_directly { - may_abort_by_id.insert(func_id); - } - } - - may_abort_by_id - }) - .unwrap_or_default(); - - let functions = module - .functions - .iter() - .map(|f| (f.def_id().unwrap(), f.clone())) - .collect(); let legal_globals = LegalGlobal::gather_from_module(module); - // 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(); - let mut inlined_to_legalize_dont_inlines = Vec::new(); - module.functions.retain(|f| { - let should_inline_f = should_inline(&legal_globals, &functions_that_may_abort, f, None); - if should_inline_f != Ok(false) { - if should_inline_f == Err(MustInlineToLegalize) && has_dont_inline(f) { - inlined_to_legalize_dont_inlines.push(f.def_id().unwrap()); - } - // TODO: We should insert all defined IDs in this function. - dropped_ids.insert(f.def_id().unwrap()); - false - } else { - true - } - }); - - if !inlined_to_legalize_dont_inlines.is_empty() { - let names = get_names(module); - for f in inlined_to_legalize_dont_inlines { - sess.dcx().warn(format!( - "`#[inline(never)]` function `{}` needs to be inlined \ - because it has illegal argument or return types", - get_name(&names, f) - )); - } - } - let header = module.header.as_mut().unwrap(); + // FIXME(eddyb) clippy false positive (separate `map` required for borrowck). #[allow(clippy::map_unwrap_or)] let mut inliner = Inliner { @@ -154,6 +75,8 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { id }), + func_id_to_idx, + id_to_name: module .debug_names .iter() @@ -173,22 +96,101 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { annotations: &mut module.annotations, types_global_values: &mut module.types_global_values, - functions: &functions, - legal_globals: &legal_globals, - functions_that_may_abort: &functions_that_may_abort, + legal_globals, + + // NOTE(eddyb) this is needed because our custom `Abort` instructions get + // lowered to a simple `OpReturn` in entry-points, but that requires that + // they get inlined all the way up to the entry-points in the first place. + functions_that_may_abort: module + .functions + .iter() + .filter_map(|func| { + let custom_ext_inst_set_import = custom_ext_inst_set_import?; + func.blocks + .iter() + .any(|block| match &block.instructions[..] { + [.., last_normal_inst, terminator_inst] + if last_normal_inst.class.opcode == Op::ExtInst + && last_normal_inst.operands[0].unwrap_id_ref() + == custom_ext_inst_set_import + && CustomOp::decode_from_ext_inst(last_normal_inst) + == CustomOp::Abort => + { + assert_eq!(terminator_inst.class.opcode, Op::Unreachable); + true + } + + _ => false, + }) + .then_some(func.def_id().unwrap()) + }) + .collect(), + + inlined_dont_inlines_to_cause_and_callers: FxIndexMap::default(), }; - for function in &mut module.functions { - inliner.inline_fn(function); - fuse_trivial_branches(function); + + let mut functions: Vec<_> = mem::take(&mut module.functions) + .into_iter() + .map(Ok) + .collect(); + + // Inline functions in post-order (aka inside-out aka bottom-out) - that is, + // callees are processed before their callers, to avoid duplicating work. + for func_idx in call_graph.post_order() { + let mut function = mem::replace(&mut functions[func_idx], Err(FuncIsBeingInlined)).unwrap(); + inliner.inline_fn(&mut function, &functions); + fuse_trivial_branches(&mut function); + functions[func_idx] = Ok(function); } - // Drop OpName etc. for inlined functions - module.debug_names.retain(|inst| { - !inst - .operands - .iter() - .any(|op| op.id_ref_any().is_some_and(|id| dropped_ids.contains(&id))) - }); + module.functions = functions.into_iter().map(|func| func.unwrap()).collect(); + + let Inliner { + id_to_name, + inlined_dont_inlines_to_cause_and_callers, + .. + } = inliner; + + let mut span_regen = SpanRegenerator::new(sess.source_map(), module); + for (callee_id, (cause, callers)) in inlined_dont_inlines_to_cause_and_callers { + let callee_name = get_name(&id_to_name, callee_id); + + // HACK(eddyb) `libcore` hides panics behind `#[inline(never)]` `fn`s, + // making this too noisy and useless (since it's an impl detail). + if cause == "panicking" && callee_name.starts_with("core::") { + continue; + } + + let callee_span = span_regen + .src_loc_for_id(callee_id) + .and_then(|src_loc| span_regen.src_loc_to_rustc(src_loc)) + .unwrap_or_default(); + sess.dcx() + .struct_span_warn( + callee_span, + format!("`#[inline(never)]` function `{callee_name}` has been inlined"), + ) + .with_note(format!("inlining was required due to {cause}")) + .with_note(format!( + "called from {}", + callers + .iter() + .enumerate() + .filter_map(|(i, &caller_id)| { + // HACK(eddyb) avoid showing too many names. + match i.cmp(&4) { + Ordering::Less => { + Some(format!("`{}`", get_name(&id_to_name, caller_id))) + } + Ordering::Equal => Some(format!("and {} more", callers.len() - i)), + Ordering::Greater => None, + } + }) + .collect::>() + .join(", ") + )) + .emit(); + } Ok(()) } @@ -371,42 +373,42 @@ fn has_dont_inline(function: &Function) -> bool { /// Helper error type for `should_inline` (see its doc comment). #[derive(Copy, Clone, PartialEq, Eq)] -struct MustInlineToLegalize; +struct MustInlineToLegalize(&'static str); -/// Returns `Ok(true)`/`Err(MustInlineToLegalize)` if `callee` should/must be +/// Returns `Ok(true)`/`Err(MustInlineToLegalize(_))` if `callee` should/must be /// inlined (either in general, or specifically from `call_site`, if provided). /// -/// The distinction made is that `Err(MustInlineToLegalize)` is not a heuristic, -/// and inlining is *mandatory* due to an illegal signature/arguments. +/// The distinction made here is that `Err(MustInlineToLegalize(cause))` is +/// very much *not* a heuristic, and inlining is *mandatory* due to `cause` +/// (usually illegal signature/arguments, but also the panicking mechanism). +// +// FIXME(eddyb) the causes here are not fine-grained enough. fn should_inline( legal_globals: &FxHashMap, functions_that_may_abort: &FxHashSet, callee: &Function, - call_site: Option>, + call_site: CallSite<'_>, ) -> Result { let callee_def = callee.def.as_ref().unwrap(); let callee_control = callee_def.operands[0].unwrap_function_control(); - // HACK(eddyb) this "has a call-site" check ensures entry-points don't get - // accidentally removed as "must inline to legalize" function, but can still - // be inlined into other entry-points (if such an unusual situation arises). - if call_site.is_some() && functions_that_may_abort.contains(&callee.def_id().unwrap()) { - return Err(MustInlineToLegalize); + if functions_that_may_abort.contains(&callee.def_id().unwrap()) { + return Err(MustInlineToLegalize("panicking")); } let ret_ty = legal_globals .get(&callee_def.result_type.unwrap()) - .ok_or(MustInlineToLegalize)?; + .ok_or(MustInlineToLegalize("illegal return type"))?; if !ret_ty.legal_as_fn_ret_ty() { - return Err(MustInlineToLegalize); + return Err(MustInlineToLegalize("illegal (pointer) return type")); } for (i, param) in callee.parameters.iter().enumerate() { let param_ty = legal_globals .get(param.result_type.as_ref().unwrap()) - .ok_or(MustInlineToLegalize)?; + .ok_or(MustInlineToLegalize("illegal parameter type"))?; if !param_ty.legal_as_fn_param_ty() { - return Err(MustInlineToLegalize); + return Err(MustInlineToLegalize("illegal (pointer) parameter type")); } // If the call isn't passing a legal pointer argument (a "memory object", @@ -414,13 +416,13 @@ fn should_inline( // then inlining is required to have a chance at producing legal SPIR-V. // // FIXME(eddyb) rewriting away the pointer could be another alternative. - if let (LegalGlobal::TypePointer(_), Some(call_site)) = (param_ty, call_site) { + if let LegalGlobal::TypePointer(_) = param_ty { let ptr_arg = call_site.call_inst.operands[i + 1].unwrap_id_ref(); match legal_globals.get(&ptr_arg) { Some(LegalGlobal::Variable) => {} // FIXME(eddyb) should some constants (undef/null) be allowed? - Some(_) => return Err(MustInlineToLegalize), + Some(_) => return Err(MustInlineToLegalize("illegal (pointer) argument")), None => { let mut caller_param_and_var_ids = call_site @@ -446,7 +448,7 @@ fn should_inline( .map(|caller_inst| caller_inst.result_id.unwrap()); if !caller_param_and_var_ids.any(|id| ptr_arg == id) { - return Err(MustInlineToLegalize); + return Err(MustInlineToLegalize("illegal (pointer) argument")); } } } @@ -456,38 +458,46 @@ fn should_inline( Ok(callee_control.contains(FunctionControl::INLINE)) } +/// Helper error type for `Inliner`'s `functions` field, indicating a `Function` +/// was taken out of its slot because it's being inlined. +#[derive(Debug)] +struct FuncIsBeingInlined; + // Steps: // Move OpVariable decls // Rewrite return // Renumber IDs // Insert blocks -struct Inliner<'m, 'map> { +struct Inliner<'a, 'b> { /// 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, + /// Map from each function's ID to its index in `functions`. + func_id_to_idx: FxHashMap, + /// 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, + 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>, + cached_op_strings: FxHashMap<&'a str, Word>, - header: &'m mut ModuleHeader, - debug_string_source: &'m mut Vec, - annotations: &'m mut Vec, - types_global_values: &'m mut Vec, + header: &'b mut ModuleHeader, + debug_string_source: &'b mut Vec, + annotations: &'b mut Vec, + types_global_values: &'b mut Vec, - functions: &'map FunctionMap, - legal_globals: &'map FxHashMap, - functions_that_may_abort: &'map FxHashSet, + legal_globals: FxHashMap, + functions_that_may_abort: FxHashSet, + inlined_dont_inlines_to_cause_and_callers: FxIndexMap)>, // rewrite_rules: FxHashMap, } @@ -536,19 +546,29 @@ impl Inliner<'_, '_> { inst_id } - fn inline_fn(&mut self, function: &mut Function) { + fn inline_fn( + &mut self, + function: &mut Function, + functions: &[Result], + ) { let mut block_idx = 0; while block_idx < function.blocks.len() { // If we successfully inlined a block, then repeat processing on the same block, in // case the newly inlined block has more inlined calls. // TODO: This is quadratic - if !self.inline_block(function, block_idx) { + if !self.inline_block(function, block_idx, functions) { + // TODO(eddyb) skip past the inlined callee without rescanning it. block_idx += 1; } } } - fn inline_block(&mut self, caller: &mut Function, block_idx: usize) -> bool { + fn inline_block( + &mut self, + caller: &mut Function, + block_idx: usize, + functions: &[Result], + ) -> bool { // Find the first inlined OpFunctionCall let call = caller.blocks[block_idx] .instructions @@ -559,8 +579,8 @@ impl Inliner<'_, '_> { ( index, inst, - self.functions - .get(&inst.operands[0].id_ref_any().unwrap()) + functions[self.func_id_to_idx[&inst.operands[0].id_ref_any().unwrap()]] + .as_ref() .unwrap(), ) }) @@ -570,19 +590,38 @@ impl Inliner<'_, '_> { call_inst: inst, }; match should_inline( - self.legal_globals, - self.functions_that_may_abort, + &self.legal_globals, + &self.functions_that_may_abort, f, - Some(call_site), + call_site, ) { Ok(inline) => inline, - Err(MustInlineToLegalize) => true, + Err(MustInlineToLegalize(cause)) => { + if has_dont_inline(f) { + self.inlined_dont_inlines_to_cause_and_callers + .entry(f.def_id().unwrap()) + .or_insert_with(|| (cause, Default::default())) + .1 + .insert(caller.def_id().unwrap()); + } + true + } } }); let (call_index, call_inst, callee) = match call { None => return false, Some(call) => call, }; + + // Propagate "may abort" from callee to caller (i.e. as aborts get inlined). + if self + .functions_that_may_abort + .contains(&callee.def_id().unwrap()) + { + self.functions_that_may_abort + .insert(caller.def_id().unwrap()); + } + let call_result_type = { let ty = call_inst.result_type.unwrap(); if ty == self.op_type_void_id { @@ -593,17 +632,28 @@ impl Inliner<'_, '_> { }; let call_result_id = call_inst.result_id.unwrap(); - // Get the debuginfo instructions that apply to the call. + // Get the debug "source location" instruction that applies to the call. let custom_ext_inst_set_import = self.custom_ext_inst_set_import; - let call_debug_insts = caller.blocks[block_idx].instructions[..call_index] + let call_debug_src_loc_inst = caller.blocks[block_idx].instructions[..call_index] .iter() - .filter(|inst| match inst.class.opcode { - Op::Line | Op::NoLine => true, - Op::ExtInst if inst.operands[0].unwrap_id_ref() == custom_ext_inst_set_import => { - CustomOp::decode_from_ext_inst(inst).is_debuginfo() - } - _ => false, - }); + .rev() + .find_map(|inst| { + Some(match inst.class.opcode { + Op::Line => Some(inst), + Op::NoLine => None, + Op::ExtInst + if inst.operands[0].unwrap_id_ref() == custom_ext_inst_set_import => + { + match CustomOp::decode_from_ext_inst(inst) { + CustomOp::SetDebugSrcLoc => Some(inst), + CustomOp::ClearDebugSrcLoc => None, + _ => return None, + } + } + _ => return None, + }) + }) + .flatten(); // Rewrite parameters to arguments let call_arguments = call_inst @@ -624,9 +674,12 @@ impl Inliner<'_, '_> { }; let return_jump = self.id(); // Rewrite OpReturns of the callee. - #[allow(clippy::needless_borrow)] - let (mut inlined_callee_blocks, extra_debug_insts_pre_call, extra_debug_insts_post_call) = - self.get_inlined_blocks(&callee, call_debug_insts, 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); @@ -648,13 +701,6 @@ impl Inliner<'_, '_> { .unwrap(); assert!(call.class.opcode == Op::FunctionCall); - // HACK(eddyb) inject the additional debuginfo instructions generated by - // `get_inlined_blocks`, so the inlined call frame "stack" isn't corrupted. - caller.blocks[pre_call_block_idx] - .instructions - .extend(extra_debug_insts_pre_call); - post_call_block_insts.splice(0..0, extra_debug_insts_post_call); - 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. @@ -849,58 +895,19 @@ impl Inliner<'_, '_> { } } - // HACK(eddyb) the second and third return values are additional debuginfo - // instructions that need to be inserted just before/after the callsite. - fn get_inlined_blocks<'a>( + fn get_inlined_blocks( &mut self, callee: &Function, - call_debug_insts: impl Iterator, + call_debug_src_loc_inst: Option<&Instruction>, return_variable: Option, return_jump: Word, - ) -> ( - Vec, - SmallVec<[Instruction; 8]>, - SmallVec<[Instruction; 8]>, - ) { + ) -> Vec { let Self { custom_ext_inst_set_import, op_type_void_id, .. } = *self; - // HACK(eddyb) this is terrible, but we have to deal with it because of - // how this inliner is outside-in, instead of inside-out, meaning that - // context builds up "outside" of the callee blocks, inside the caller. - let mut enclosing_inlined_frames = SmallVec::<[_; 8]>::new(); - let mut current_debug_src_loc_inst = None; - for inst in call_debug_insts { - match inst.class.opcode { - Op::Line => current_debug_src_loc_inst = Some(inst), - Op::NoLine => current_debug_src_loc_inst = None, - Op::ExtInst - if inst.operands[0].unwrap_id_ref() == self.custom_ext_inst_set_import => - { - match CustomOp::decode_from_ext_inst(inst) { - CustomOp::SetDebugSrcLoc => current_debug_src_loc_inst = Some(inst), - CustomOp::ClearDebugSrcLoc => current_debug_src_loc_inst = None, - CustomOp::PushInlinedCallFrame => { - enclosing_inlined_frames - .push((current_debug_src_loc_inst.take(), inst)); - } - CustomOp::PopInlinedCallFrame => { - if let Some((callsite_debug_src_loc_inst, _)) = - enclosing_inlined_frames.pop() - { - current_debug_src_loc_inst = callsite_debug_src_loc_inst; - } - } - CustomOp::Abort => {} - } - } - _ => {} - } - } - // 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 @@ -925,7 +932,7 @@ impl Inliner<'_, '_> { )); id }); - let mut mk_debuginfo_prefix_and_suffix = |include_callee_frame| { + 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 instantiate_debuginfo = |this: &mut Self, inst: &Instruction| { @@ -949,33 +956,18 @@ impl Inliner<'_, '_> { .collect(), ) }; - // FIXME(eddyb) this only allocates to avoid borrow conflicts. - let mut prefix = SmallVec::<[_; 8]>::new(); - let mut suffix = SmallVec::<[_; 8]>::new(); - for &(callsite_debug_src_loc_inst, push_inlined_call_frame_inst) in - &enclosing_inlined_frames - { - prefix.extend( - callsite_debug_src_loc_inst - .into_iter() - .chain([push_inlined_call_frame_inst]) - .map(|inst| instantiate_debuginfo(self, inst)), - ); - suffix.push(custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)); - } - prefix.extend(current_debug_src_loc_inst.map(|inst| instantiate_debuginfo(self, inst))); - - if include_callee_frame { - prefix.push(custom_inst_to_inst( - self, - CustomInst::PushInlinedCallFrame { - callee_name: Operand::IdRef(callee_name_id), - }, - )); - suffix.push(custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)); - } - (prefix, suffix) + ( + (call_debug_src_loc_inst.map(|inst| instantiate_debuginfo(self, inst))) + .into_iter() + .chain([custom_inst_to_inst( + self, + CustomInst::PushInlinedCallFrame { + callee_name: Operand::IdRef(callee_name_id), + }, + )]), + [custom_inst_to_inst(self, CustomInst::PopInlinedCallFrame)], + ) }; let mut blocks = callee.blocks.clone(); @@ -1029,7 +1021,7 @@ impl Inliner<'_, '_> { // HACK(eddyb) avoid adding debuginfo to otherwise-empty blocks. if block.instructions.len() > num_phis { - let (debuginfo_prefix, debuginfo_suffix) = mk_debuginfo_prefix_and_suffix(true); + let (debuginfo_prefix, debuginfo_suffix) = mk_debuginfo_prefix_and_suffix(); // Insert the prefix debuginfo instructions after `OpPhi`s, // which sadly can't be covered by them. block @@ -1043,13 +1035,7 @@ impl Inliner<'_, '_> { block.instructions.push(terminator); } - let (caller_restore_debuginfo_after_call, calleer_reset_debuginfo_before_call) = - mk_debuginfo_prefix_and_suffix(false); - ( - blocks, - calleer_reset_debuginfo_before_call, - caller_restore_debuginfo_after_call, - ) + blocks } fn insert_opvariables(&self, block: &mut Block, insts: impl IntoIterator) { diff --git a/crates/rustc_codegen_spirv/src/linker/ipo.rs b/crates/rustc_codegen_spirv/src/linker/ipo.rs index 475f5c50c15..96c7bbe6aed 100644 --- a/crates/rustc_codegen_spirv/src/linker/ipo.rs +++ b/crates/rustc_codegen_spirv/src/linker/ipo.rs @@ -4,7 +4,7 @@ use indexmap::IndexSet; use rspirv::dr::Module; -use rspirv::spirv::Op; +use rspirv::spirv::{Op, Word}; use rustc_data_structures::fx::FxHashMap; // FIXME(eddyb) use newtyped indices and `IndexVec`. @@ -19,6 +19,9 @@ pub struct CallGraph { impl CallGraph { pub fn collect(module: &Module) -> Self { + Self::collect_with_func_id_to_idx(module).0 + } + pub fn collect_with_func_id_to_idx(module: &Module) -> (Self, FxHashMap) { let func_id_to_idx: FxHashMap<_, _> = module .functions .iter() @@ -51,10 +54,13 @@ impl CallGraph { .collect() }) .collect(); - Self { - entry_points, - callees, - } + ( + Self { + entry_points, + callees, + }, + func_id_to_idx, + ) } /// Order functions using a post-order traversal, i.e. callees before callers. diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr index 4feecc9adf5..5ec9e260b4a 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr @@ -17,3 +17,4 @@ LL | fn g(xy: (&u32, &u32)) {} = note: called from `member_ref_arg::main` warning: 2 warnings emitted + From 19b74729750c456778e1cfea6adc2e774a4b826c Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 08/14] [2024] linker/mem2reg: index SPIR-V blocks by their label IDs for O(1) lookup. --- crates/rustc_codegen_spirv/src/linker/dce.rs | 11 ++- .../rustc_codegen_spirv/src/linker/mem2reg.rs | 91 +++++++++++-------- crates/rustc_codegen_spirv/src/linker/mod.rs | 7 +- 3 files changed, 63 insertions(+), 46 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/dce.rs b/crates/rustc_codegen_spirv/src/linker/dce.rs index 3ea62e1693d..7e9297e5d2a 100644 --- a/crates/rustc_codegen_spirv/src/linker/dce.rs +++ b/crates/rustc_codegen_spirv/src/linker/dce.rs @@ -7,9 +7,10 @@ //! *references* a rooted thing is also rooted, not the other way around - but that's the basic //! concept. -use rspirv::dr::{Function, Instruction, Module, Operand}; +use rspirv::dr::{Block, Function, Instruction, Module, Operand}; use rspirv::spirv::{Decoration, LinkageType, Op, StorageClass, Word}; -use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; +use std::hash::Hash; pub fn dce(module: &mut Module) { let mut rooted = collect_roots(module); @@ -137,11 +138,11 @@ fn kill_unrooted(module: &mut Module, rooted: &FxIndexSet) { } } -pub fn dce_phi(func: &mut Function) { +pub fn dce_phi(blocks: &mut FxIndexMap) { let mut used = FxIndexSet::default(); loop { let mut changed = false; - for inst in func.all_inst_iter() { + for inst in blocks.values().flat_map(|block| &block.instructions) { if inst.class.opcode != Op::Phi || used.contains(&inst.result_id.unwrap()) { for op in &inst.operands { if let Some(id) = op.id_ref_any() { @@ -154,7 +155,7 @@ pub fn dce_phi(func: &mut Function) { break; } } - for block in &mut func.blocks { + for block in blocks.values_mut() { block .instructions .retain(|inst| inst.class.opcode != Op::Phi || used.contains(&inst.result_id.unwrap())); diff --git a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs index 16889cbcc31..df2434d63d9 100644 --- a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs +++ b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs @@ -13,10 +13,14 @@ use super::simple_passes::outgoing_edges; use super::{apply_rewrite_rules, id}; use rspirv::dr::{Block, Function, Instruction, ModuleHeader, Operand}; use rspirv::spirv::{Op, Word}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_middle::bug; use std::collections::hash_map; +// HACK(eddyb) newtype instead of type alias to avoid mistakes. +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +struct LabelId(Word); + pub fn mem2reg( header: &mut ModuleHeader, types_global_values: &mut Vec, @@ -24,8 +28,16 @@ pub fn mem2reg( constants: &FxHashMap, func: &mut Function, ) { - let reachable = compute_reachable(&func.blocks); - let preds = compute_preds(&func.blocks, &reachable); + // HACK(eddyb) this ad-hoc indexing might be useful elsewhere as well, but + // it's made completely irrelevant by SPIR-T so only applies to legacy code. + let mut blocks: FxIndexMap<_, _> = func + .blocks + .iter_mut() + .map(|block| (LabelId(block.label_id().unwrap()), block)) + .collect(); + + let reachable = compute_reachable(&blocks); + let preds = compute_preds(&blocks, &reachable); let idom = compute_idom(&preds, &reachable); let dominance_frontier = compute_dominance_frontier(&preds, &idom); loop { @@ -34,31 +46,27 @@ pub fn mem2reg( types_global_values, pointer_to_pointee, constants, - &mut func.blocks, + &mut blocks, &dominance_frontier, ); if !changed { break; } // mem2reg produces minimal SSA form, not pruned, so DCE the dead ones - super::dce::dce_phi(func); + super::dce::dce_phi(&mut blocks); } } -fn label_to_index(blocks: &[Block], id: Word) -> usize { - blocks - .iter() - .position(|b| b.label_id().unwrap() == id) - .unwrap() -} - -fn compute_reachable(blocks: &[Block]) -> Vec { - fn recurse(blocks: &[Block], reachable: &mut [bool], block: usize) { +fn compute_reachable(blocks: &FxIndexMap) -> Vec { + fn recurse(blocks: &FxIndexMap, reachable: &mut [bool], block: usize) { if !reachable[block] { reachable[block] = true; - for dest_id in outgoing_edges(&blocks[block]) { - let dest_idx = label_to_index(blocks, dest_id); - recurse(blocks, reachable, dest_idx); + for dest_id in outgoing_edges(blocks[block]) { + recurse( + blocks, + reachable, + blocks.get_index_of(&LabelId(dest_id)).unwrap(), + ); } } } @@ -67,17 +75,19 @@ fn compute_reachable(blocks: &[Block]) -> Vec { reachable } -fn compute_preds(blocks: &[Block], reachable_blocks: &[bool]) -> Vec> { +fn compute_preds( + blocks: &FxIndexMap, + reachable_blocks: &[bool], +) -> Vec> { let mut result = vec![vec![]; blocks.len()]; // Do not count unreachable blocks as valid preds of blocks for (source_idx, source) in blocks - .iter() + .values() .enumerate() .filter(|&(b, _)| reachable_blocks[b]) { for dest_id in outgoing_edges(source) { - let dest_idx = label_to_index(blocks, dest_id); - result[dest_idx].push(source_idx); + result[blocks.get_index_of(&LabelId(dest_id)).unwrap()].push(source_idx); } } result @@ -161,7 +171,7 @@ fn insert_phis_all( types_global_values: &mut Vec, pointer_to_pointee: &FxHashMap, constants: &FxHashMap, - blocks: &mut [Block], + blocks: &mut FxIndexMap, dominance_frontier: &[FxHashSet], ) -> bool { let var_maps_and_types = blocks[0] @@ -198,7 +208,11 @@ fn insert_phis_all( rewrite_rules: FxHashMap::default(), }; renamer.rename(0, None); - apply_rewrite_rules(&renamer.rewrite_rules, blocks); + // FIXME(eddyb) shouldn't this full rescan of the function be done once? + apply_rewrite_rules( + &renamer.rewrite_rules, + blocks.values_mut().map(|block| &mut **block), + ); remove_nops(blocks); } remove_old_variables(blocks, &var_maps_and_types); @@ -216,7 +230,7 @@ struct VarInfo { fn collect_access_chains( pointer_to_pointee: &FxHashMap, constants: &FxHashMap, - blocks: &[Block], + blocks: &FxIndexMap, base_var: Word, base_var_ty: Word, ) -> Option> { @@ -249,7 +263,7 @@ fn collect_access_chains( // Loop in case a previous block references a later AccessChain loop { let mut changed = false; - for inst in blocks.iter().flat_map(|b| &b.instructions) { + for inst in blocks.values().flat_map(|b| &b.instructions) { for (index, op) in inst.operands.iter().enumerate() { if let Operand::IdRef(id) = op && variables.contains_key(id) @@ -303,10 +317,10 @@ fn collect_access_chains( // same var map (e.g. `s.x = s.y;`). fn split_copy_memory( header: &mut ModuleHeader, - blocks: &mut [Block], + blocks: &mut FxIndexMap, var_map: &FxHashMap, ) { - for block in blocks { + for block in blocks.values_mut() { let mut inst_index = 0; while inst_index < block.instructions.len() { let inst = &block.instructions[inst_index]; @@ -365,7 +379,7 @@ fn has_store(block: &Block, var_map: &FxHashMap) -> bool { } fn insert_phis( - blocks: &[Block], + blocks: &FxIndexMap, dominance_frontier: &[FxHashSet], var_map: &FxHashMap, ) -> FxHashSet { @@ -374,7 +388,7 @@ fn insert_phis( let mut ever_on_work_list = FxHashSet::default(); let mut work_list = Vec::new(); let mut blocks_with_phi = FxHashSet::default(); - for (block_idx, block) in blocks.iter().enumerate() { + for (block_idx, block) in blocks.values().enumerate() { if has_store(block, var_map) { ever_on_work_list.insert(block_idx); work_list.push(block_idx); @@ -419,10 +433,10 @@ fn top_stack_or_undef( } } -struct Renamer<'a> { +struct Renamer<'a, 'b> { header: &'a mut ModuleHeader, types_global_values: &'a mut Vec, - blocks: &'a mut [Block], + blocks: &'a mut FxIndexMap, blocks_with_phi: FxHashSet, base_var_type: Word, var_map: &'a FxHashMap, @@ -432,7 +446,7 @@ struct Renamer<'a> { rewrite_rules: FxHashMap, } -impl Renamer<'_> { +impl Renamer<'_, '_> { // Returns the phi definition. fn insert_phi_value(&mut self, block: usize, from_block: usize) -> Word { let from_block_label = self.blocks[from_block].label_id().unwrap(); @@ -554,9 +568,8 @@ impl Renamer<'_> { } } - for dest_id in outgoing_edges(&self.blocks[block]).collect::>() { - // TODO: Don't do this find - let dest_idx = label_to_index(self.blocks, dest_id); + for dest_id in outgoing_edges(self.blocks[block]).collect::>() { + let dest_idx = self.blocks.get_index_of(&LabelId(dest_id)).unwrap(); self.rename(dest_idx, Some(block)); } @@ -566,8 +579,8 @@ impl Renamer<'_> { } } -fn remove_nops(blocks: &mut [Block]) { - for block in blocks { +fn remove_nops(blocks: &mut FxIndexMap) { + for block in blocks.values_mut() { block .instructions .retain(|inst| inst.class.opcode != Op::Nop); @@ -575,7 +588,7 @@ fn remove_nops(blocks: &mut [Block]) { } fn remove_old_variables( - blocks: &mut [Block], + blocks: &mut FxIndexMap, var_maps_and_types: &[(FxHashMap, u32)], ) { blocks[0].instructions.retain(|inst| { @@ -586,7 +599,7 @@ fn remove_old_variables( .all(|(var_map, _)| !var_map.contains_key(&result_id)) } }); - for block in blocks { + for block in blocks.values_mut() { block.instructions.retain(|inst| { !matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) || inst.operands.iter().all(|op| { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index e347aec7123..b6e3b9a05f3 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -86,9 +86,12 @@ fn id(header: &mut ModuleHeader) -> Word { result } -fn apply_rewrite_rules(rewrite_rules: &FxHashMap, blocks: &mut [Block]) { +fn apply_rewrite_rules<'a>( + rewrite_rules: &FxHashMap, + blocks: impl IntoIterator, +) { let all_ids_mut = blocks - .iter_mut() + .into_iter() .flat_map(|b| b.label.iter_mut().chain(b.instructions.iter_mut())) .flat_map(|inst| { inst.result_id From e3c88368e68b6babbba2806d59315b34c10c2fb6 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 09/14] [2024] linker/inline: use `OpPhi` instead of `OpVariable` for return values. --- .../rustc_codegen_spirv/src/linker/inline.rs | 136 +++++++++--------- .../ui/dis/complex_image_sample_inst.stderr | 21 ++- 2 files changed, 77 insertions(+), 80 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index e2aac8c548f..0ab19bd40f1 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -94,7 +94,6 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { header, debug_string_source: &mut module.debug_string_source, annotations: &mut module.annotations, - types_global_values: &mut module.types_global_values, legal_globals, @@ -493,7 +492,6 @@ struct Inliner<'a, 'b> { header: &'b mut ModuleHeader, debug_string_source: &'b mut Vec, annotations: &'b mut Vec, - types_global_values: &'b mut Vec, legal_globals: FxHashMap, functions_that_may_abort: FxHashSet, @@ -523,29 +521,6 @@ impl Inliner<'_, '_> { } } - fn ptr_ty(&mut self, pointee: Word) -> Word { - // TODO: This is horribly slow, fix this - let existing = self.types_global_values.iter().find(|inst| { - inst.class.opcode == Op::TypePointer - && inst.operands[0].unwrap_storage_class() == StorageClass::Function - && inst.operands[1].unwrap_id_ref() == pointee - }); - if let Some(existing) = existing { - return existing.result_id.unwrap(); - } - let inst_id = self.id(); - self.types_global_values.push(Instruction::new( - Op::TypePointer, - None, - Some(inst_id), - vec![ - Operand::StorageClass(StorageClass::Function), - Operand::IdRef(pointee), - ], - )); - inst_id - } - fn inline_fn( &mut self, function: &mut Function, @@ -622,15 +597,19 @@ impl Inliner<'_, '_> { .insert(caller.def_id().unwrap()); } - let call_result_type = { + let mut maybe_call_result_phi = { let ty = call_inst.result_type.unwrap(); if ty == self.op_type_void_id { None } else { - Some(ty) + Some(Instruction::new( + Op::Phi, + Some(ty), + Some(call_inst.result_id.unwrap()), + vec![], + )) } }; - let call_result_id = call_inst.result_id.unwrap(); // Get the debug "source location" instruction that applies to the call. let custom_ext_inst_set_import = self.custom_ext_inst_set_import; @@ -667,17 +646,12 @@ impl Inliner<'_, '_> { }); let mut rewrite_rules = callee_parameters.zip(call_arguments).collect(); - let return_variable = if call_result_type.is_some() { - Some(self.id()) - } else { - None - }; let return_jump = self.id(); // Rewrite OpReturns of the callee. let mut inlined_callee_blocks = self.get_inlined_blocks( callee, call_debug_src_loc_inst, - return_variable, + maybe_call_result_phi.as_mut(), return_jump, ); // Clone the IDs of the callee, because otherwise they'd be defined multiple times if the @@ -686,6 +660,55 @@ impl Inliner<'_, '_> { apply_rewrite_rules(&rewrite_rules, &mut inlined_callee_blocks); self.apply_rewrite_for_decorations(&rewrite_rules); + if let Some(call_result_phi) = &mut maybe_call_result_phi { + // HACK(eddyb) new IDs should be generated earlier, to avoid pushing + // callee IDs to `call_result_phi.operands` only to rewrite them here. + for op in &mut call_result_phi.operands { + if let Some(id) = op.id_ref_any_mut() + && let Some(&rewrite) = rewrite_rules.get(id) + { + *id = rewrite; + } + } + + // HACK(eddyb) this special-casing of the single-return case is + // really necessary for passes like `mem2reg` which are not capable + // of skipping through the extraneous `OpPhi`s on their own. + if let [returned_value, _return_block] = &call_result_phi.operands[..] { + let call_result_id = call_result_phi.result_id.unwrap(); + let returned_value_id = returned_value.unwrap_id_ref(); + + maybe_call_result_phi = None; + + // HACK(eddyb) this is a conservative approximation of all the + // instructions that could potentially reference the call result. + let reaching_insts = { + let (pre_call_blocks, call_and_post_call_blocks) = + caller.blocks.split_at_mut(block_idx); + (pre_call_blocks.iter_mut().flat_map(|block| { + block + .instructions + .iter_mut() + .take_while(|inst| inst.class.opcode == Op::Phi) + })) + .chain( + call_and_post_call_blocks + .iter_mut() + .flat_map(|block| &mut block.instructions), + ) + }; + for reaching_inst in reaching_insts { + for op in &mut reaching_inst.operands { + if let Some(id) = op.id_ref_any_mut() + && *id == call_result_id + { + *id = returned_value_id; + } + } + } + } + } + // Split the block containing the `OpFunctionCall` into pre-call vs post-call. let pre_call_block_idx = block_idx; #[expect(unused)] @@ -701,18 +724,6 @@ impl Inliner<'_, '_> { .unwrap(); assert!(call.class.opcode == Op::FunctionCall); - 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. - let ret_var_inst = Instruction::new( - Op::Variable, - Some(self.ptr_ty(call_result_type)), - Some(return_variable.unwrap()), - vec![Operand::StorageClass(StorageClass::Function)], - ); - self.insert_opvariables(&mut caller.blocks[0], [ret_var_inst]); - } - // Insert non-entry inlined callee blocks just after the pre-call block. let non_entry_inlined_callee_blocks = inlined_callee_blocks.drain(1..); let num_non_entry_inlined_callee_blocks = non_entry_inlined_callee_blocks.len(); @@ -721,18 +732,9 @@ impl Inliner<'_, '_> { non_entry_inlined_callee_blocks, ); - if let Some(call_result_type) = call_result_type { - // Add the load of the result value after the inlined function. Note there's guaranteed no - // OpPhi instructions since we just split this block. - post_call_block_insts.insert( - 0, - Instruction::new( - Op::Load, - Some(call_result_type), - Some(call_result_id), - vec![Operand::IdRef(return_variable.unwrap())], - ), - ); + if let Some(call_result_phi) = maybe_call_result_phi { + // Add the `OpPhi` for the call result value, after the inlined function. + post_call_block_insts.insert(0, call_result_phi); } // Insert the post-call block, after all the inlined callee blocks. @@ -899,7 +901,7 @@ impl Inliner<'_, '_> { &mut self, callee: &Function, call_debug_src_loc_inst: Option<&Instruction>, - return_variable: Option, + mut maybe_call_result_phi: Option<&mut Instruction>, return_jump: Word, ) -> Vec { let Self { @@ -997,17 +999,13 @@ impl Inliner<'_, '_> { if let Op::Return | Op::ReturnValue = terminator.class.opcode { if Op::ReturnValue == terminator.class.opcode { let return_value = terminator.operands[0].id_ref_any().unwrap(); - block.instructions.push(Instruction::new( - Op::Store, - None, - None, - vec![ - Operand::IdRef(return_variable.unwrap()), - Operand::IdRef(return_value), - ], - )); + let call_result_phi = maybe_call_result_phi.as_deref_mut().unwrap(); + call_result_phi.operands.extend([ + Operand::IdRef(return_value), + Operand::IdRef(block.label_id().unwrap()), + ]); } else { - assert!(return_variable.is_none()); + assert!(maybe_call_result_phi.is_none()); } terminator = Instruction::new(Op::Branch, None, None, vec![Operand::IdRef(return_jump)]); diff --git a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr index 3d62336afd9..b99b63a736b 100644 --- a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr +++ b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr @@ -3,19 +3,18 @@ %5 = OpFunctionParameter %6 %7 = OpFunctionParameter %6 %8 = OpLabel -OpLine %9 10 25 -%10 = OpCompositeExtract %11 %5 0 -%12 = OpCompositeExtract %11 %5 1 -%13 = OpCompositeConstruct %6 %10 %12 -%14 = OpCompositeExtract %11 %7 0 -%15 = OpCompositeExtract %11 %7 1 -%16 = OpCompositeConstruct %6 %14 %15 -OpLine %9 29 13 +%9 = OpCompositeExtract %10 %5 0 +%11 = OpCompositeExtract %10 %5 1 +%12 = OpCompositeConstruct %6 %9 %11 +%13 = OpCompositeExtract %10 %7 0 +%14 = OpCompositeExtract %10 %7 1 +%15 = OpCompositeConstruct %6 %13 %14 +OpLine %16 29 13 %17 = OpAccessChain %18 %19 %20 -OpLine %9 30 13 +OpLine %16 30 13 %21 = OpLoad %22 %17 -OpLine %9 34 13 -%23 = OpImageSampleProjExplicitLod %2 %21 %4 Grad %13 %16 +OpLine %16 34 13 +%23 = OpImageSampleProjExplicitLod %2 %21 %4 Grad %12 %15 OpNoLine OpReturnValue %23 OpFunctionEnd From e4ac5c842554926535f466e41ddec86bc4fe2a0b Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 10/14] linker/inline: fix typos in comments. --- crates/rustc_codegen_spirv/src/linker/inline.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 0ab19bd40f1..a6abc0f2ee0 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -133,7 +133,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { .map(Ok) .collect(); - // Inline functions in post-order (aka inside-out aka bottom-out) - that is, + // Inline functions in post-order (aka inside-out aka bottom-up) - that is, // callees are processed before their callers, to avoid duplicating work. for func_idx in call_graph.post_order() { let mut function = mem::replace(&mut functions[func_idx], Err(FuncIsBeingInlined)).unwrap(); @@ -411,7 +411,7 @@ fn should_inline( } // If the call isn't passing a legal pointer argument (a "memory object", - // i.e. an `OpVariable` or one of the caller's `OpFunctionParameters), + // i.e. an `OpVariable` or one of the caller's `OpFunctionParameter`s), // then inlining is required to have a chance at producing legal SPIR-V. // // FIXME(eddyb) rewriting away the pointer could be another alternative. @@ -826,7 +826,7 @@ impl Inliner<'_, '_> { } // `vars_and_debuginfo_range.end` indicates where `OpVariable`s - // end and other instructions start (module debuginfo), but to + // end and other instructions start (modulo debuginfo), but to // split the block in two, both sides of the "cut" need "repair": // - the variables are missing "inlined call frames" pops, that // may happen later in the block, and have to be synthesized From 045b98c52d000e8c72962aa5e50631f275ed4046 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 11/14] [2024] linker/mem2reg: apply rewrite rules only once per `insert_phis_all` invocation. --- .../rustc_codegen_spirv/src/linker/mem2reg.rs | 17 +++++++++-------- crates/rustc_codegen_spirv/src/linker/mod.rs | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs index df2434d63d9..c6fd8c084ed 100644 --- a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs +++ b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs @@ -193,6 +193,8 @@ fn insert_phis_all( for (var_map, _) in &var_maps_and_types { split_copy_memory(header, blocks, var_map); } + + let mut rewrite_rules = FxHashMap::default(); for &(ref var_map, base_var_type) in &var_maps_and_types { let blocks_with_phi = insert_phis(blocks, dominance_frontier, var_map); let mut renamer = Renamer { @@ -205,16 +207,15 @@ fn insert_phis_all( phi_defs: FxHashSet::default(), visited: FxHashSet::default(), stack: Vec::new(), - rewrite_rules: FxHashMap::default(), + rewrite_rules: &mut rewrite_rules, }; renamer.rename(0, None); - // FIXME(eddyb) shouldn't this full rescan of the function be done once? - apply_rewrite_rules( - &renamer.rewrite_rules, - blocks.values_mut().map(|block| &mut **block), - ); - remove_nops(blocks); } + apply_rewrite_rules( + &rewrite_rules, + blocks.values_mut().map(|block| &mut **block), + ); + remove_nops(blocks); remove_old_variables(blocks, &var_maps_and_types); true } @@ -443,7 +444,7 @@ struct Renamer<'a, 'b> { phi_defs: FxHashSet, visited: FxHashSet, stack: Vec, - rewrite_rules: FxHashMap, + rewrite_rules: &'a mut FxHashMap, } impl Renamer<'_, '_> { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index b6e3b9a05f3..3001e95fefd 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -104,7 +104,7 @@ fn apply_rewrite_rules<'a>( ) }); for id in all_ids_mut { - if let Some(&rewrite) = rewrite_rules.get(id) { + while let Some(&rewrite) = rewrite_rules.get(id) { *id = rewrite; } } From 3c3912ecdeb875fb96c6ada2813904ebdcbf8df5 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 12/14] linker/inline: also run `remove_duplicate_debuginfo` on every fully-inlined function. --- .../rustc_codegen_spirv/src/linker/duplicates.rs | 16 +++++++++++++++- crates/rustc_codegen_spirv/src/linker/inline.rs | 6 ++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/rustc_codegen_spirv/src/linker/duplicates.rs b/crates/rustc_codegen_spirv/src/linker/duplicates.rs index 76f85a77134..70554cadddc 100644 --- a/crates/rustc_codegen_spirv/src/linker/duplicates.rs +++ b/crates/rustc_codegen_spirv/src/linker/duplicates.rs @@ -283,7 +283,20 @@ pub fn remove_duplicate_debuginfo(module: &mut Module) { }) .map(|inst| inst.result_id.unwrap()); + let deduper = DebuginfoDeduplicator { + custom_ext_inst_set_import, + }; for func in &mut module.functions { + deduper.remove_duplicate_debuginfo_in_function(func); + } +} + +pub struct DebuginfoDeduplicator { + pub custom_ext_inst_set_import: Option, +} + +impl DebuginfoDeduplicator { + pub fn remove_duplicate_debuginfo_in_function(&self, func: &mut rspirv::dr::Function) { for block in &mut func.blocks { // Ignore the terminator, it's effectively "outside" debuginfo. let (_, insts) = block.instructions.split_last_mut().unwrap(); @@ -339,7 +352,8 @@ pub fn remove_duplicate_debuginfo(module: &mut Module) { let inst = &insts[inst_idx]; let custom_op = match inst.class.opcode { Op::ExtInst - if Some(inst.operands[0].unwrap_id_ref()) == custom_ext_inst_set_import => + if Some(inst.operands[0].unwrap_id_ref()) + == self.custom_ext_inst_set_import => { Some(CustomOp::decode_from_ext_inst(inst)) } diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index a6abc0f2ee0..46aa3c95f83 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -139,6 +139,12 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { let mut function = mem::replace(&mut functions[func_idx], Err(FuncIsBeingInlined)).unwrap(); inliner.inline_fn(&mut function, &functions); fuse_trivial_branches(&mut function); + + super::duplicates::DebuginfoDeduplicator { + custom_ext_inst_set_import, + } + .remove_duplicate_debuginfo_in_function(&mut function); + functions[func_idx] = Ok(function); } From c665c62c5d87fbcd34b17f3d84fc3feef0b51f29 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:40:02 +0300 Subject: [PATCH 13/14] linker/inline: also run `mem2reg` on every fully-inlined function. --- .../rustc_codegen_spirv/src/linker/inline.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 46aa3c95f83..8e16a9555d3 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -133,6 +133,32 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { .map(Ok) .collect(); + let mut mem2reg_pointer_to_pointee = FxHashMap::default(); + let mut mem2reg_constants = FxHashMap::default(); + { + let mut u32 = None; + for inst in &module.types_global_values { + match inst.class.opcode { + Op::TypePointer => { + mem2reg_pointer_to_pointee + .insert(inst.result_id.unwrap(), inst.operands[1].unwrap_id_ref()); + } + Op::TypeInt + if inst.operands[0].unwrap_literal_bit32() == 32 + && inst.operands[1].unwrap_literal_bit32() == 0 => + { + assert!(u32.is_none()); + u32 = Some(inst.result_id.unwrap()); + } + Op::Constant if u32.is_some() && inst.result_type == u32 => { + let value = inst.operands[0].unwrap_literal_bit32(); + mem2reg_constants.insert(inst.result_id.unwrap(), value); + } + _ => {} + } + } + } + // Inline functions in post-order (aka inside-out aka bottom-up) - that is, // callees are processed before their callers, to avoid duplicating work. for func_idx in call_graph.post_order() { @@ -145,6 +171,19 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { } .remove_duplicate_debuginfo_in_function(&mut function); + { + super::simple_passes::block_ordering_pass(&mut function); + // Note: mem2reg requires functions to be in RPO order (i.e. block_ordering_pass) + super::mem2reg::mem2reg( + inliner.header, + &mut module.types_global_values, + &mem2reg_pointer_to_pointee, + &mem2reg_constants, + &mut function, + ); + super::destructure_composites::destructure_composites(&mut function); + } + functions[func_idx] = Ok(function); } From 0ae3cd059fa759464db635e0ac33ae07518bb41c Mon Sep 17 00:00:00 2001 From: Christian Legnitto Date: Tue, 7 Apr 2026 15:18:28 -0700 Subject: [PATCH 14/14] Update rebased compiletest expectations --- .../ui/dis/panic_builtin_bounds_check.stderr | 86 +++++++++---------- .../ui/lang/consts/nested-ref.stderr | 4 +- .../core/ref/member_ref_arg-broken.stderr | 8 +- .../ui/lang/core/ref/member_ref_arg.stderr | 4 +- 4 files changed, 51 insertions(+), 51 deletions(-) diff --git a/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr b/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr index 6486f832eb2..2a6a8e7746e 100644 --- a/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr +++ b/tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr @@ -4,53 +4,53 @@ OpExtension "SPV_KHR_non_semantic_info" OpMemoryModel Logical Simple OpEntryPoint Fragment %2 "main" OpExecutionMode %2 OriginUpperLeft -%3 = OpString "/n[Rust panicked at $SYSROOT/lib/rustlib/src/rust/library/core/src/panicking.rs:276:5]/n index out of bounds: the len is %u but the index is %u/n in main()/n" -%4 = OpString $SYSROOT/lib/rustlib/src/rust/library/core/src/panicking.rs" -%5 = OpString "$DIR/panic_builtin_bounds_check.rs" -OpDecorate %6 ArrayStride 4 -%7 = OpTypeVoid -%8 = OpTypeFunction %7 -%9 = OpTypeInt 32 0 -%10 = OpConstant %9 4 -%11 = OpTypeArray %9 %10 -%12 = OpTypePointer Function %11 -%6 = OpTypeArray %9 %10 -%13 = OpConstant %9 0 -%14 = OpConstant %9 1 -%15 = OpConstant %9 2 -%16 = OpConstant %9 3 -%17 = OpTypeBool -%18 = OpConstant %9 5 -%19 = OpTypePointer Function %9 -%2 = OpFunction %7 None %8 -%20 = OpLabel -OpLine %5 32 4 -%21 = OpVariable %12 Function -OpLine %5 32 23 -%22 = OpCompositeConstruct %6 %13 %14 %15 %16 -OpLine %5 27 4 -%23 = OpCompositeExtract %9 %22 0 -%24 = OpCompositeExtract %9 %22 1 -%25 = OpCompositeExtract %9 %22 2 -%26 = OpCompositeExtract %9 %22 3 -%27 = OpCompositeConstruct %11 %23 %24 %25 %26 -OpStore %21 %27 -%28 = OpULessThan %17 %18 %10 +%3 = OpString "/n[Rust panicked at $DIR/panic_builtin_bounds_check.rs:27:5]/n index out of bounds: the len is %u but the index is %u/n in main()/n" +%4 = OpString "$DIR/panic_builtin_bounds_check.rs" +OpDecorate %5 ArrayStride 4 +%6 = OpTypeVoid +%7 = OpTypeFunction %6 +%8 = OpTypeInt 32 0 +%9 = OpConstant %8 4 +%10 = OpTypeArray %8 %9 +%11 = OpTypePointer Function %10 +%5 = OpTypeArray %8 %9 +%12 = OpConstant %8 0 +%13 = OpConstant %8 1 +%14 = OpConstant %8 2 +%15 = OpConstant %8 3 +%16 = OpTypeBool +%17 = OpConstant %8 5 +%18 = OpTypePointer Function %8 +%2 = OpFunction %6 None %7 +%19 = OpLabel +OpLine %4 32 4 +%20 = OpVariable %11 Function +OpLine %4 32 23 +%21 = OpCompositeConstruct %5 %12 %13 %14 %15 +OpLine %4 32 4 +%22 = OpCompositeExtract %8 %21 0 +%23 = OpCompositeExtract %8 %21 1 +%24 = OpCompositeExtract %8 %21 2 +%25 = OpCompositeExtract %8 %21 3 +%26 = OpCompositeConstruct %10 %22 %23 %24 %25 +OpStore %20 %26 +OpLine %4 27 4 +%27 = OpULessThan %16 %17 %9 OpNoLine -OpSelectionMerge %29 None -OpBranchConditional %28 %30 %31 +OpSelectionMerge %28 None +OpBranchConditional %27 %29 %30 +%29 = OpLabel +OpBranch %28 %30 = OpLabel -OpBranch %29 -%31 = OpLabel -OpLine %4 276 4 -%32 = OpExtInst %7 %1 1 %3 %10 %18 +OpLine %4 27 4 +%31 = OpExtInst %6 %1 1 %3 %9 %17 OpNoLine OpReturn -%29 = OpLabel -OpLine %5 27 4 -%33 = OpIAdd %9 %13 %18 -%34 = OpInBoundsAccessChain %19 %21 %33 -%35 = OpLoad %9 %34 +%28 = OpLabel +OpLine %4 27 4 +%32 = OpIAdd %8 %12 %17 +%33 = OpInBoundsAccessChain %18 %20 %32 +%34 = OpLoad %8 %33 OpNoLine OpReturn OpFunctionEnd diff --git a/tests/compiletests/ui/lang/consts/nested-ref.stderr b/tests/compiletests/ui/lang/consts/nested-ref.stderr index 805ec9b6076..ef34288a66e 100644 --- a/tests/compiletests/ui/lang/consts/nested-ref.stderr +++ b/tests/compiletests/ui/lang/consts/nested-ref.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `nested_ref::deep_load` has been inlined - --> $DIR/nested-ref.rs:12:4 + --> <$DIR/nested-ref.rs>:12:4 | LL | fn deep_load(r: &'static &'static u32) -> u32 { | ^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn deep_load(r: &'static &'static u32) -> u32 { = note: called from `nested_ref::main` warning: `#[inline(never)]` function `nested_ref::deep_transpose` has been inlined - --> $DIR/nested-ref.rs:19:4 + --> <$DIR/nested-ref.rs>:19:4 | LL | fn deep_transpose(r: &'static &'static Mat2) -> Mat2 { | ^^^^^^^^^^^^^^ diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr index c46b2e0276e..17c52a8fabc 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg-broken.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `member_ref_arg_broken::f` has been inlined - --> $DIR/member_ref_arg-broken.rs:20:4 + --> <$DIR/member_ref_arg-broken.rs>:20:4 | LL | fn f(x: &u32) -> u32 { | ^ @@ -8,7 +8,7 @@ LL | fn f(x: &u32) -> u32 { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::g` has been inlined - --> $DIR/member_ref_arg-broken.rs:25:4 + --> <$DIR/member_ref_arg-broken.rs>:25:4 | LL | fn g(xy: (&u32, &u32)) -> (u32, u32) { | ^ @@ -17,7 +17,7 @@ LL | fn g(xy: (&u32, &u32)) -> (u32, u32) { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::h` has been inlined - --> $DIR/member_ref_arg-broken.rs:30:4 + --> <$DIR/member_ref_arg-broken.rs>:30:4 | LL | fn h(xyz: (&u32, &u32, &u32)) -> (u32, u32, u32) { | ^ @@ -26,7 +26,7 @@ LL | fn h(xyz: (&u32, &u32, &u32)) -> (u32, u32, u32) { = note: called from `member_ref_arg_broken::main` warning: `#[inline(never)]` function `member_ref_arg_broken::h_newtyped` has been inlined - --> $DIR/member_ref_arg-broken.rs:41:4 + --> <$DIR/member_ref_arg-broken.rs>:41:4 | LL | fn h_newtyped(xyz: ((&u32, &u32, &u32),)) -> (u32, u32, u32) { | ^^^^^^^^^^ diff --git a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr index 5ec9e260b4a..7596b6f46a3 100644 --- a/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr +++ b/tests/compiletests/ui/lang/core/ref/member_ref_arg.stderr @@ -1,5 +1,5 @@ warning: `#[inline(never)]` function `member_ref_arg::f` has been inlined - --> $DIR/member_ref_arg.rs:14:4 + --> <$DIR/member_ref_arg.rs>:14:4 | LL | fn f(x: &u32) {} | ^ @@ -8,7 +8,7 @@ LL | fn f(x: &u32) {} = note: called from `member_ref_arg::main` warning: `#[inline(never)]` function `member_ref_arg::g` has been inlined - --> $DIR/member_ref_arg.rs:17:4 + --> <$DIR/member_ref_arg.rs>:17:4 | LL | fn g(xy: (&u32, &u32)) {} | ^