Skip to content

experiment: Use llvm.dbg.value instead of llvm.dbg.declare when possible. #68902

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 82 additions & 13 deletions src/librustc_codegen_llvm/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::value::Value;
use rustc::mir;
use rustc::session::config::{self, DebugInfo};
use rustc::ty::{self, Instance, InstanceDef, ParamEnv, Ty};
use rustc_codegen_ssa::common::TypeKind;
use rustc_codegen_ssa::debuginfo::type_names;
use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext, VariableKind};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -155,8 +156,9 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> {
direct_offset: Size,
indirect_offsets: &[Size],
span: Span,
is_by_val: bool,
) {
assert!(!dbg_context.source_locations_enabled);
assert!(!dbg_context.source_locations_enabled || is_by_val);
let cx = self.cx();

let loc = span_start(cx, span);
Expand Down Expand Up @@ -186,20 +188,87 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> {
);
unsafe {
let debug_loc = llvm::LLVMGetCurrentDebugLocation(self.llbuilder);
// FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`.
let instr = llvm::LLVMRustDIBuilderInsertDeclareAtEnd(
DIB(cx),
variable_alloca,
dbg_var,
addr_ops.as_ptr(),
addr_ops.len() as c_uint,
debug_loc,
self.llbb(),
);
let instr = if is_by_val {
let llval = variable_alloca;

let instr = llvm::LLVMRustDIBuilderInsertDbgValueAtEnd(
DIB(cx),
llval,
dbg_var,
addr_ops.as_ptr(),
addr_ops.len() as c_uint,
debug_loc,
self.llbb(),
);

// HACK(eddyb) keep the value we passed to `llvm.dbg.value`
// alive if we're not optimizing, otherwise LLVM will eagerly
// destroy it (and debuggers will show `<optimized out>`),
// if there are no other uses forcing the value to be computed.
// FIXME(#68817) implement the correct behavior in LLVM.
if self.sess().opts.optimize == config::OptLevel::No {
let llty = cx.val_ty(llval);

// Avoid creating LLVM inline asm with unsupported types,
// usually ZST structs.
let asm_supported = match self.type_kind(llty) {
// Floats.
TypeKind::Half |
TypeKind::Float |
TypeKind::Double |
TypeKind::X86_FP80 |
TypeKind::FP128 |
TypeKind::PPC_FP128 |
// Integers.
TypeKind::Integer |
TypeKind::Pointer |
// Vectors.
TypeKind::Vector |
TypeKind::X86_MMX => true,

TypeKind::Void |
TypeKind::Label |
TypeKind::Function |
TypeKind::Struct |
TypeKind::Array |
TypeKind::Metadata |
TypeKind::Token => false,
};

if asm_supported {
let asm = SmallCStr::new("");
let constraints = SmallCStr::new("X");
let asm = llvm::LLVMRustInlineAsm(
cx.type_func(&[llty], cx.type_void()),
asm.as_ptr(),
constraints.as_ptr(),
llvm::False,
llvm::False,
llvm::AsmDialect::Att,
);
self.call(asm, &[llval], None);
}
}

instr
} else {
// FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`.
llvm::LLVMRustDIBuilderInsertDeclareAtEnd(
DIB(cx),
variable_alloca,
dbg_var,
addr_ops.as_ptr(),
addr_ops.len() as c_uint,
debug_loc,
self.llbb(),
)
};

llvm::LLVMSetInstDebugLocation(self.llbuilder, instr);
}
source_loc::set_debug_location(self, UnknownLocation);
if !is_by_val {
source_loc::set_debug_location(self, UnknownLocation);
}
}

fn set_source_location(
Expand Down Expand Up @@ -569,7 +638,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
file_metadata,
loc.line as c_uint,
type_metadata,
self.sess().opts.optimize != config::OptLevel::No,
true,
DIFlags::FlagZero,
argument_index,
align.bytes() as u32,
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_codegen_llvm/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1730,6 +1730,16 @@ extern "C" {
InsertAtEnd: &'a BasicBlock,
) -> &'a Value;

pub fn LLVMRustDIBuilderInsertDbgValueAtEnd(
Builder: &DIBuilder<'a>,
Val: &'a Value,
VarInfo: &'a DIVariable,
AddrOps: *const i64,
AddrOpsCount: c_uint,
DL: &'a Value,
InsertAtEnd: &'a BasicBlock,
) -> &'a Value;

pub fn LLVMRustDIBuilderCreateEnumerator(
Builder: &DIBuilder<'a>,
Name: *const c_char,
Expand Down
26 changes: 16 additions & 10 deletions src/librustc_codegen_ssa/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
analyzer.visit_body(mir);

for (local, decl) in mir.local_decls.iter_enumerated() {
// FIXME(eddyb): We should figure out how to use llvm.dbg.value instead
// of putting everything in allocas just so we can use llvm.dbg.declare.
if fx.cx.sess().opts.debuginfo == DebugInfo::Full {
if fx.mir.local_kind(local) == mir::LocalKind::Arg {
analyzer.not_ssa(local);
continue;
}
}

let ty = fx.monomorphize(&decl.ty);
debug!("local {:?} has type `{}`", local, ty);
let layout = fx.cx.spanned_layout_of(ty, decl.source_info.span);
Expand All @@ -41,6 +32,15 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// in an Value without an alloca.
} else if fx.cx.is_backend_scalar_pair(layout) {
// We allow pairs and uses of any of their 2 fields.

// FIXME(eddyb): We should figure out how to use llvm.dbg.value instead
// of putting everything in allocas just so we can use llvm.dbg.declare.
if fx.cx.sess().opts.debuginfo == DebugInfo::Full {
if fx.mir.local_kind(local) == mir::LocalKind::Arg {
analyzer.not_ssa(local);
continue;
}
}
} else {
// These sorts of types require an alloca. Note that
// is_llvm_immediate() may *still* be true, particularly
Expand Down Expand Up @@ -286,7 +286,13 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
// FIXME(eddyb): We should figure out how to use `llvm.dbg.value` instead
// of putting everything in allocas just so we can use `llvm.dbg.declare`.
if self.fx.cx.sess().opts.debuginfo == DebugInfo::Full {
self.not_ssa(local);
let decl_span = self.fx.mir.local_decls[local].source_info.span;
let ty = self.fx.mir.local_decls[local].ty;
let ty = self.fx.monomorphize(&ty);
let layout = self.fx.cx.spanned_layout_of(ty, decl_span);
if !self.fx.cx.is_backend_immediate(layout) {
self.not_ssa(local);
}
}
}

Expand Down
57 changes: 39 additions & 18 deletions src/librustc_codegen_ssa/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,16 +215,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
None => return,
};

// FIXME(eddyb) add debuginfo for unsized places too.
let base = match local_ref {
LocalRef::Place(place) => place,
_ => return,
let base_layout = match local_ref {
LocalRef::Operand(None) => return,
LocalRef::Operand(Some(operand)) => operand.layout,
LocalRef::Place(place) => place.layout,

// FIXME(eddyb) add debuginfo for unsized places too.
LocalRef::UnsizedPlace(_) => return,
};

let vars = vars.iter().copied().chain(fallback_var);

for var in vars {
let mut layout = base.layout;
let mut layout = base_layout;
let mut direct_offset = Size::ZERO;
// FIXME(eddyb) use smallvec here.
let mut indirect_offsets = vec![];
Expand Down Expand Up @@ -263,15 +266,36 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let (scope, span) = self.debug_loc(var.source_info);
if let Some(scope) = scope {
if let Some(dbg_var) = var.dbg_var {
bx.dbg_var_addr(
debug_context,
dbg_var,
scope,
base.llval,
direct_offset,
&indirect_offsets,
span,
);
match local_ref {
LocalRef::Place(place) => {
bx.dbg_var_addr(
debug_context,
dbg_var,
scope,
place.llval,
direct_offset,
&indirect_offsets,
span,
false,
);
}
LocalRef::Operand(Some(operand)) => match operand.val {
OperandValue::Immediate(x) => {
bx.dbg_var_addr(
debug_context,
dbg_var,
scope,
x,
direct_offset,
&indirect_offsets,
span,
true,
);
}
OperandValue::Ref(..) | OperandValue::Pair(..) => unreachable!(),
},
LocalRef::UnsizedPlace(_) | LocalRef::Operand(None) => unreachable!(),
}
}
}
}
Expand Down Expand Up @@ -307,11 +331,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let var_ty = self.monomorphized_place_ty(place.as_ref());
let var_kind = if self.mir.local_kind(place.local) == mir::LocalKind::Arg
&& place.projection.is_empty()
&& var.source_info.scope == mir::OUTERMOST_SOURCE_SCOPE
{
// FIXME(eddyb, #67586) take `var.source_info.scope` into
// account to avoid using `ArgumentVariable` more than once
// per argument local.

let arg_index = place.local.index() - 1;

// FIXME(eddyb) shouldn't `ArgumentVariable` indices be
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_ssa/traits/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub trait DebugInfoBuilderMethods: BackendTypes {
// NB: each offset implies a deref (i.e. they're steps in a pointer chain).
indirect_offsets: &[Size],
span: Span,
is_by_value: bool,
);
fn set_source_location(
&mut self,
Expand Down
11 changes: 11 additions & 0 deletions src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,17 @@ extern "C" LLVMValueRef LLVMRustDIBuilderInsertDeclareAtEnd(
unwrap(InsertAtEnd)));
}

extern "C" LLVMValueRef LLVMRustDIBuilderInsertDbgValueAtEnd(
LLVMRustDIBuilderRef Builder, LLVMValueRef V, LLVMMetadataRef VarInfo,
int64_t *AddrOps, unsigned AddrOpsCount, LLVMValueRef DL,
LLVMBasicBlockRef InsertAtEnd) {
return wrap(Builder->insertDbgValueIntrinsic(
unwrap(V), unwrap<DILocalVariable>(VarInfo),
Builder->createExpression(llvm::ArrayRef<int64_t>(AddrOps, AddrOpsCount)),
DebugLoc(cast<MDNode>(unwrap<MetadataAsValue>(DL)->getMetadata())),
unwrap(InsertAtEnd)));
}

extern "C" LLVMMetadataRef
LLVMRustDIBuilderCreateEnumerator(LLVMRustDIBuilderRef Builder,
const char *Name, uint64_t Val) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/debuginfo/borrowed-c-style-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

enum ABC { TheA, TheB, TheC }

fn main() {
pub fn main() {
let the_a = ABC::TheA;
let the_a_ref: &ABC = &the_a;

Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/mir/mir-inlining/var-debuginfo-issue-67586.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// run-pass
// compile-flags: -Z mir-opt-level=2 -C opt-level=0 -C debuginfo=2

#[inline(never)]
pub fn foo(bar: usize) -> usize {
std::convert::identity(bar)
}

fn main() {
foo(0);
}