-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Introduce debuginfo to statements in MIR #142771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
06f5a9d
29357bc
a0b4fe1
ec25192
458ad93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,19 +160,23 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> { | |
&mut self, | ||
dbg_var: &'ll DIVariable, | ||
dbg_loc: &'ll DILocation, | ||
variable_alloca: Self::Value, | ||
is_declared: bool, | ||
val: Self::Value, | ||
direct_offset: Size, | ||
indirect_offsets: &[Size], | ||
fragment: Option<Range<Size>>, | ||
) { | ||
use dwarf_const::{DW_OP_LLVM_fragment, DW_OP_deref, DW_OP_plus_uconst}; | ||
use dwarf_const::{DW_OP_LLVM_fragment, DW_OP_deref, DW_OP_plus_uconst, DW_OP_stack_value}; | ||
|
||
// Convert the direct and indirect offsets and fragment byte range to address ops. | ||
let mut addr_ops = SmallVec::<[u64; 8]>::new(); | ||
|
||
if direct_offset.bytes() > 0 { | ||
addr_ops.push(DW_OP_plus_uconst); | ||
addr_ops.push(direct_offset.bytes() as u64); | ||
if !is_declared { | ||
addr_ops.push(DW_OP_stack_value); | ||
} | ||
} | ||
for &offset in indirect_offsets { | ||
addr_ops.push(DW_OP_deref); | ||
|
@@ -189,17 +193,30 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> { | |
addr_ops.push((fragment.end - fragment.start).bits() as u64); | ||
} | ||
|
||
unsafe { | ||
// FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`. | ||
llvm::LLVMRustDIBuilderInsertDeclareAtEnd( | ||
DIB(self.cx()), | ||
variable_alloca, | ||
dbg_var, | ||
addr_ops.as_ptr(), | ||
addr_ops.len() as c_uint, | ||
dbg_loc, | ||
self.llbb(), | ||
); | ||
if is_declared { | ||
unsafe { | ||
llvm::LLVMRustDIBuilderInsertDeclareAtEnd( | ||
DIB(self.cx()), | ||
val, | ||
dbg_var, | ||
addr_ops.as_ptr(), | ||
addr_ops.len() as c_uint, | ||
dbg_loc, | ||
self.llbb(), | ||
); | ||
} | ||
} else { | ||
unsafe { | ||
llvm::LLVMRustDIBuilderInsertDbgValueAtEnd( | ||
DIB(self.cx()), | ||
val, | ||
dbg_var, | ||
addr_ops.as_ptr(), | ||
addr_ops.len() as c_uint, | ||
dbg_loc, | ||
self.llbb(), | ||
); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to move all debuginfo to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, but it's very complicated. It might be better to let LLVM do it. LLVM might also preserve the declare: https://rust.godbolt.org/z/GKWYeT7rK, but this seems like it could be dropped. |
||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -253,6 +253,52 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
spill_slot | ||
} | ||
|
||
pub(crate) fn debug_new_value_to_local( | ||
&self, | ||
bx: &mut Bx, | ||
local: mir::Local, | ||
base: PlaceValue<Bx::Value>, | ||
layout: TyAndLayout<'tcx>, | ||
projection: &[mir::PlaceElem<'tcx>], | ||
) { | ||
let full_debug_info = bx.sess().opts.debuginfo == DebugInfo::Full; | ||
if !full_debug_info { | ||
return; | ||
} | ||
|
||
let vars = match &self.per_local_var_debug_info { | ||
Some(per_local) => &per_local[local], | ||
None => return, | ||
}; | ||
|
||
for var in vars.iter().cloned() { | ||
self.debug_new_value_to_local_as_var(bx, base, layout, projection, var); | ||
} | ||
} | ||
|
||
fn debug_new_value_to_local_as_var( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be inlined? This should allow to compute offsets only once outside the loop. |
||
&self, | ||
bx: &mut Bx, | ||
base: PlaceValue<Bx::Value>, | ||
layout: TyAndLayout<'tcx>, | ||
projection: &[mir::PlaceElem<'tcx>], | ||
var: PerLocalVarDebugInfo<'tcx, Bx::DIVariable>, | ||
) { | ||
let Some(dbg_var) = var.dbg_var else { return }; | ||
let Some(dbg_loc) = self.dbg_loc(var.source_info) else { return }; | ||
let DebugInfoOffset { direct_offset, indirect_offsets, result: _ } = | ||
calculate_debuginfo_offset(bx, projection, layout); | ||
bx.dbg_var_addr( | ||
dbg_var, | ||
dbg_loc, | ||
false, | ||
base.llval, | ||
direct_offset, | ||
&indirect_offsets, | ||
var.fragment, | ||
); | ||
} | ||
|
||
/// Apply debuginfo and/or name, after creating the `alloca` for a local, | ||
/// or initializing the local with an operand (whichever applies). | ||
pub(crate) fn debug_introduce_local(&self, bx: &mut Bx, local: mir::Local) { | ||
|
@@ -421,6 +467,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
bx.dbg_var_addr( | ||
dbg_var, | ||
dbg_loc, | ||
true, | ||
alloca.val.llval, | ||
Size::ZERO, | ||
&[Size::ZERO], | ||
|
@@ -430,6 +477,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
bx.dbg_var_addr( | ||
dbg_var, | ||
dbg_loc, | ||
true, | ||
base.val.llval, | ||
direct_offset, | ||
&indirect_offsets, | ||
|
@@ -455,7 +503,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
let base = FunctionCx::spill_operand_to_stack(operand, Some(name), bx); | ||
bx.clear_dbg_loc(); | ||
|
||
bx.dbg_var_addr(dbg_var, dbg_loc, base.val.llval, Size::ZERO, &[], fragment); | ||
bx.dbg_var_addr(dbg_var, dbg_loc, true, base.val.llval, Size::ZERO, &[], fragment); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,17 @@ | ||
use rustc_middle::mir::{self, NonDivergingIntrinsic}; | ||
use rustc_middle::span_bug; | ||
use rustc_middle::mir::{self, NonDivergingIntrinsic, StmtDebugInfo}; | ||
use rustc_middle::{bug, span_bug}; | ||
use tracing::instrument; | ||
|
||
use super::{FunctionCx, LocalRef}; | ||
use crate::common::TypeKind; | ||
use crate::mir::operand::OperandValue; | ||
use crate::mir::place::PlaceRef; | ||
use crate::traits::*; | ||
|
||
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | ||
#[instrument(level = "debug", skip(self, bx))] | ||
pub(crate) fn codegen_statement(&mut self, bx: &mut Bx, statement: &mir::Statement<'tcx>) { | ||
self.codegen_stmt_debuginfos(bx, &statement.debuginfos); | ||
self.set_debug_loc(bx, statement.source_info); | ||
match statement.kind { | ||
mir::StatementKind::Assign(box (ref place, ref rvalue)) => { | ||
|
@@ -101,4 +105,74 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |
| mir::StatementKind::Nop => {} | ||
} | ||
} | ||
|
||
pub(crate) fn codegen_stmt_debuginfo(&mut self, bx: &mut Bx, debuginfo: &StmtDebugInfo<'tcx>) { | ||
match debuginfo { | ||
StmtDebugInfo::AssignRef(dest, place) => { | ||
let place_ref = match self.locals[place.local] { | ||
LocalRef::Place(place_ref) | LocalRef::UnsizedPlace(place_ref) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This local is poorly named. It represents |
||
Some(place_ref) | ||
} | ||
LocalRef::Operand(operand_ref) => match operand_ref.val { | ||
OperandValue::Immediate(v) => { | ||
Some(PlaceRef::new_sized(v, operand_ref.layout)) | ||
} | ||
OperandValue::Ref(_) | ||
| OperandValue::Pair(_, _) | ||
| OperandValue::ZeroSized => None, | ||
}, | ||
LocalRef::PendingOperand => None, | ||
} | ||
.filter(|place_ref| { | ||
// Drop unsupported projections. | ||
// FIXME: Add a test case. | ||
place.projection.iter().all(|p| p.can_use_in_debuginfo()) && | ||
// Only pointers can calculate addresses. | ||
bx.type_kind(bx.val_ty(place_ref.val.llval)) == TypeKind::Pointer | ||
}); | ||
let assign_ref = match (place_ref, place.is_indirect_first_projection()) { | ||
(Some(place_ref), false) => { | ||
Some((place_ref.val, place_ref.layout, place.projection.as_slice())) | ||
} | ||
(Some(place_ref), true) => { | ||
let projected_ty = place_ref | ||
.layout | ||
.ty | ||
.builtin_deref(true) | ||
.unwrap_or_else(|| bug!("deref of non-pointer {:?}", place_ref)); | ||
let layout = bx.cx().layout_of(projected_ty); | ||
Some((place_ref.val, layout, &place.projection[1..])) | ||
} | ||
_ => None, | ||
}; | ||
let (val, layout, projection) = assign_ref.unwrap_or_else(|| { | ||
// If the address cannot be computed, use poison to indicate that the value has been optimized out. | ||
let ty = self.monomorphize(self.mir.local_decls[*dest].ty); | ||
let layout = bx.cx().layout_of(ty); | ||
let to_backend_ty = bx.cx().immediate_backend_type(layout); | ||
let place_ref = | ||
PlaceRef::new_sized(bx.cx().const_poison(to_backend_ty), layout); | ||
(place_ref.val, layout, [].as_slice()) | ||
}); | ||
self.debug_new_value_to_local(bx, *dest, val, layout, projection); | ||
} | ||
StmtDebugInfo::InvalidAssign(local) => { | ||
let ty = self.monomorphize(self.mir.local_decls[*local].ty); | ||
let layout = bx.cx().layout_of(ty); | ||
let to_backend_ty = bx.cx().immediate_backend_type(layout); | ||
let place_ref = PlaceRef::new_sized(bx.cx().const_poison(to_backend_ty), layout); | ||
self.debug_new_value_to_local(bx, *local, place_ref.val, layout, &[]); | ||
} | ||
} | ||
} | ||
|
||
pub(crate) fn codegen_stmt_debuginfos( | ||
&mut self, | ||
bx: &mut Bx, | ||
debuginfos: &[StmtDebugInfo<'tcx>], | ||
) { | ||
for debuginfo in debuginfos { | ||
self.codegen_stmt_debuginfo(bx, debuginfo); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain I understand how the codegen part works. Is the
is_declared
a property of the MIR local? Or can the same MIR local mix declared and undeclared debuginfo? When the local is declared, what doesval
represent? Should we have anenum LocalDbgValue { ByRef(Value), ByValue(Value) }
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, we can mix these two debuginfos, I don't see any issues for this.
#dbg_declare
is used to annotate a variable with an alloca pointer, but#dbg_value
is used to tell a variable is set to a new value.Perhaps I could add a new method
dbg_var_val
or renamedbg_var_addr
todbg_var
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just have 2 methods? a
dbg_var_addr
and adbg_var_value
?