From 687bffa49375aa00bacc51f5d9adfb84a9453e17 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Mon, 7 Aug 2023 14:24:41 -0700 Subject: [PATCH] Use the same DISubprogram for each instance of the same inlined function within the caller --- compiler/rustc_codegen_gcc/src/debuginfo.rs | 2 +- .../src/debuginfo/create_scope_map.rs | 35 +++++---- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 76 +++++++++++-------- .../rustc_codegen_ssa/src/mir/debuginfo.rs | 14 +++- compiler/rustc_codegen_ssa/src/mir/mod.rs | 2 +- .../rustc_codegen_ssa/src/traits/debuginfo.rs | 2 +- .../debuginfo-inline-callsite-location.rs | 26 +++++++ 7 files changed, 106 insertions(+), 51 deletions(-) create mode 100644 tests/codegen/debuginfo-inline-callsite-location.rs diff --git a/compiler/rustc_codegen_gcc/src/debuginfo.rs b/compiler/rustc_codegen_gcc/src/debuginfo.rs index a81585d412846..d1bfd833cd87f 100644 --- a/compiler/rustc_codegen_gcc/src/debuginfo.rs +++ b/compiler/rustc_codegen_gcc/src/debuginfo.rs @@ -55,7 +55,7 @@ impl<'gcc, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'gcc, 'tcx> { _fn_abi: &FnAbi<'tcx, Ty<'tcx>>, _llfn: RValue<'gcc>, _mir: &mir::Body<'tcx>, - ) -> Option> { + ) -> Option> { // TODO(antoyo) None } diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs index d174a3593b999..7a68c291aa597 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs @@ -20,7 +20,7 @@ pub fn compute_mir_scopes<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>, mir: &Body<'tcx>, - debug_context: &mut FunctionDebugContext<&'ll DIScope, &'ll DILocation>, + debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>, ) { // Find all scopes with variables defined in them. let variables = if cx.sess().opts.debuginfo == DebugInfo::Full { @@ -51,7 +51,7 @@ fn make_mir_scope<'ll, 'tcx>( instance: Instance<'tcx>, mir: &Body<'tcx>, variables: &Option>, - debug_context: &mut FunctionDebugContext<&'ll DIScope, &'ll DILocation>, + debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>, instantiated: &mut BitSet, scope: SourceScope, ) { @@ -84,7 +84,6 @@ fn make_mir_scope<'ll, 'tcx>( } let loc = cx.lookup_debug_loc(scope_data.span.lo()); - let file_metadata = file_metadata(cx, &loc.file); let dbg_scope = match scope_data.inlined { Some((callee, _)) => { @@ -95,18 +94,26 @@ fn make_mir_scope<'ll, 'tcx>( ty::ParamEnv::reveal_all(), ty::EarlyBinder::bind(callee), ); - let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty()); - cx.dbg_scope_fn(callee, callee_fn_abi, None) + debug_context.inlined_function_scopes.entry(callee).or_insert_with(|| { + let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty()); + cx.dbg_scope_fn(callee, callee_fn_abi, None) + }) + } + None => { + let file_metadata = file_metadata(cx, &loc.file); + debug_context + .lexical_blocks + .entry((parent_scope.dbg_scope, loc.line, loc.col, file_metadata)) + .or_insert_with(|| unsafe { + llvm::LLVMRustDIBuilderCreateLexicalBlock( + DIB(cx), + parent_scope.dbg_scope, + file_metadata, + loc.line, + loc.col, + ) + }) } - None => unsafe { - llvm::LLVMRustDIBuilderCreateLexicalBlock( - DIB(cx), - parent_scope.dbg_scope, - file_metadata, - loc.line, - loc.col, - ) - }, }; let inlined_at = scope_data.inlined.map(|(_, callsite_span)| { diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 40714a0afe91c..d03819b16881b 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -5,7 +5,7 @@ use rustc_codegen_ssa::mir::debuginfo::VariableKind::*; use self::metadata::{file_metadata, type_di_node}; use self::metadata::{UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER}; use self::namespace::mangled_name_of_instance; -use self::utils::{create_DIArray, is_node_local_to_unit, DIB}; +use self::utils::{create_DIArray, debug_context, is_node_local_to_unit, DIB}; use crate::abi::FnAbi; use crate::builder::Builder; @@ -67,6 +67,8 @@ pub struct CodegenUnitDebugContext<'ll, 'tcx> { type_map: metadata::TypeMap<'ll, 'tcx>, namespace_map: RefCell>, recursion_marker_type: OnceCell<&'ll DIType>, + /// Maps a varaible (name, scope, kind (argument or local), span) to its debug information. + variables: RefCell>, } impl Drop for CodegenUnitDebugContext<'_, '_> { @@ -91,6 +93,7 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> { type_map: Default::default(), namespace_map: RefCell::new(Default::default()), recursion_marker_type: OnceCell::new(), + variables: RefCell::new(Default::default()), } } @@ -292,7 +295,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { fn_abi: &FnAbi<'tcx, Ty<'tcx>>, llfn: &'ll Value, mir: &mir::Body<'tcx>, - ) -> Option> { + ) -> Option> { if self.sess().opts.debuginfo == DebugInfo::None { return None; } @@ -304,8 +307,11 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { file_start_pos: BytePos(0), file_end_pos: BytePos(0), }; - let mut fn_debug_context = - FunctionDebugContext { scopes: IndexVec::from_elem(empty_scope, &mir.source_scopes) }; + let mut fn_debug_context = FunctionDebugContext { + scopes: IndexVec::from_elem(empty_scope, &mir.source_scopes), + inlined_function_scopes: Default::default(), + lexical_blocks: Default::default(), + }; // Fill in all the scopes, with the information from the MIR body. compute_mir_scopes(self, instance, mir, &mut fn_debug_context); @@ -606,33 +612,39 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { variable_kind: VariableKind, span: Span, ) -> &'ll DIVariable { - let loc = self.lookup_debug_loc(span.lo()); - let file_metadata = file_metadata(self, &loc.file); - - let type_metadata = type_di_node(self, variable_type); - - let (argument_index, dwarf_tag) = match variable_kind { - ArgumentVariable(index) => (index as c_uint, DW_TAG_arg_variable), - LocalVariable => (0, DW_TAG_auto_variable), - }; - let align = self.align_of(variable_type); - - let name = variable_name.as_str(); - unsafe { - llvm::LLVMRustDIBuilderCreateVariable( - DIB(self), - dwarf_tag, - scope_metadata, - name.as_ptr().cast(), - name.len(), - file_metadata, - loc.line, - type_metadata, - true, - DIFlags::FlagZero, - argument_index, - align.bytes() as u32, - ) - } + debug_context(self) + .variables + .borrow_mut() + .entry((variable_name, scope_metadata, variable_kind, span)) + .or_insert_with(|| { + let loc = self.lookup_debug_loc(span.lo()); + let file_metadata = file_metadata(self, &loc.file); + + let type_metadata = type_di_node(self, variable_type); + + let (argument_index, dwarf_tag) = match variable_kind { + ArgumentVariable(index) => (index as c_uint, DW_TAG_arg_variable), + LocalVariable => (0, DW_TAG_auto_variable), + }; + let align = self.align_of(variable_type); + + let name = variable_name.as_str(); + unsafe { + llvm::LLVMRustDIBuilderCreateVariable( + DIB(self), + dwarf_tag, + scope_metadata, + name.as_ptr().cast(), + name.len(), + file_metadata, + loc.line, + type_metadata, + true, + DIFlags::FlagZero, + argument_index, + align.bytes() as u32, + ) + } + }) } } diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index 4167a85ccd590..b9d5b54dc6217 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -1,10 +1,12 @@ use crate::traits::*; +use rustc_data_structures::fx::FxHashMap; use rustc_index::IndexVec; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir; use rustc_middle::ty; use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; +use rustc_middle::ty::Instance; use rustc_middle::ty::Ty; use rustc_session::config::DebugInfo; use rustc_span::symbol::{kw, Symbol}; @@ -17,11 +19,19 @@ use super::{FunctionCx, LocalRef}; use std::ops::Range; -pub struct FunctionDebugContext { +pub struct FunctionDebugContext<'tcx, S, L> { + /// Maps from source code to the corresponding debug info scope. pub scopes: IndexVec>, + + /// Maps from a given inlined function to its debug info declaration. + pub inlined_function_scopes: FxHashMap, S>, + + /// Maps from a lexical block (parent scope, line, column, file) to its debug info declaration. + /// This is particularily useful if the parent scope is an inlined function. + pub lexical_blocks: FxHashMap<(S, u32, u32, S), S>, } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Hash)] pub enum VariableKind { ArgumentVariable(usize /*index*/), LocalVariable, diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 3464f910829da..ccf93f208d50d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -45,7 +45,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { mir: &'tcx mir::Body<'tcx>, - debug_context: Option>, + debug_context: Option>, llfn: Bx::Function, diff --git a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs index 63fecaf34fd5b..4acc0ea076c13 100644 --- a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs @@ -26,7 +26,7 @@ pub trait DebugInfoMethods<'tcx>: BackendTypes { fn_abi: &FnAbi<'tcx, Ty<'tcx>>, llfn: Self::Function, mir: &mir::Body<'tcx>, - ) -> Option>; + ) -> Option>; // FIXME(eddyb) find a common convention for all of the debuginfo-related // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.). diff --git a/tests/codegen/debuginfo-inline-callsite-location.rs b/tests/codegen/debuginfo-inline-callsite-location.rs new file mode 100644 index 0000000000000..e2ea9dda4a097 --- /dev/null +++ b/tests/codegen/debuginfo-inline-callsite-location.rs @@ -0,0 +1,26 @@ +// compile-flags: -g -O + +// Check that each inline call site for the same function uses the same "sub-program" so that LLVM +// can correctly merge the debug info if it merges the inlined code (e.g., for merging of tail +// calls to panic. + +// CHECK: tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E +// CHECK-SAME: !dbg ![[#first_dbg:]] +// CHECK: tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E +// CHECK-SAME: !dbg ![[#second_dbg:]] + +// CHECK: ![[#func_dbg:]] = distinct !DISubprogram(name: "unwrap" +// CHECK: ![[#first_dbg]] = !DILocation(line: [[#]] +// CHECK-SAME: scope: ![[#func_dbg]], inlinedAt: ![[#]]) +// CHECK: ![[#second_dbg]] = !DILocation(line: [[#]] +// CHECK-SAME: scope: ![[#func_dbg]], inlinedAt: ![[#]]) + +#![crate_type = "lib"] + +#[no_mangle] +extern "C" fn add_numbers(x: &Option, y: &Option) -> i32 { + let x1 = x.unwrap(); + let y1 = y.unwrap(); + + x1 + y1 +}