From ef63e88a9dad54539e1b8510e98e4ed951518c05 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 8 Feb 2020 19:20:45 +0200 Subject: [PATCH 1/2] rustc_codegen_ssa: use `debug_introduce_local` on Operand call results. --- src/librustc_codegen_ssa/mir/block.rs | 3 +++ src/test/codegen/naked-functions.rs | 8 +++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 7f43e66549896..0a4610e9e490d 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -1145,6 +1145,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let op = bx.load_operand(place); place.storage_dead(bx); self.locals[index] = LocalRef::Operand(Some(op)); + self.debug_introduce_local(bx, index); } LocalRef::Operand(Some(op)) => { assert!(op.layout.is_zst(), "assigning to initialized SSAtemp"); @@ -1186,6 +1187,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let op = bx.load_operand(tmp); tmp.storage_dead(bx); self.locals[index] = LocalRef::Operand(Some(op)); + self.debug_introduce_local(bx, index); } DirectOperand(index) => { // If there is a cast, we have to store and reload. @@ -1200,6 +1202,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { OperandRef::from_immediate_or_packed_pair(bx, llval, ret_abi.layout) }; self.locals[index] = LocalRef::Operand(Some(op)); + self.debug_introduce_local(bx, index); } } } diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index 5050ed1499414..493c1b9f0ba6b 100644 --- a/src/test/codegen/naked-functions.rs +++ b/src/test/codegen/naked-functions.rs @@ -1,5 +1,3 @@ -// ignore-tidy-linelength - // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] @@ -61,19 +59,19 @@ pub fn naked_recursive() { naked_empty(); - // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return() + // CHECK-NEXT: %_4 = call i{{[0-9]+}} @naked_with_return() // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb2 // CHECK: bb2: - // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK-NEXT: %_3 = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %_4) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb3 // CHECK: bb3: - // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %_3) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb4 From 1a8f5efab8ccd9d5f6ac794ab1bcf90b5efa536a Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 8 Feb 2020 19:24:31 +0200 Subject: [PATCH 2/2] rustc_codegen_ssa: only "spill" SSA-like values to the stack for debuginfo. --- src/librustc_codegen_ssa/mir/analyze.rs | 19 --------- src/librustc_codegen_ssa/mir/debuginfo.rs | 47 ++++++++++++++++------- src/test/debuginfo/issue-12886.rs | 2 +- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/analyze.rs b/src/librustc_codegen_ssa/mir/analyze.rs index d4b0ab0448a8b..7bf222f4701b7 100644 --- a/src/librustc_codegen_ssa/mir/analyze.rs +++ b/src/librustc_codegen_ssa/mir/analyze.rs @@ -8,7 +8,6 @@ use rustc::mir::visit::{ MutatingUseContext, NonMutatingUseContext, NonUseContext, PlaceContext, Visitor, }; use rustc::mir::{self, Location, TerminatorKind}; -use rustc::session::config::DebugInfo; use rustc::ty; use rustc::ty::layout::{HasTyCtxt, LayoutOf}; use rustc_data_structures::graph::dominators::Dominators; @@ -24,15 +23,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); @@ -281,15 +271,6 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> self.assign(local, location); } - PlaceContext::NonUse(NonUseContext::VarDebugInfo) => { - // We need to keep locals in `alloca`s for debuginfo. - // 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); - } - } - PlaceContext::NonUse(_) | PlaceContext::MutatingUse(MutatingUseContext::Retag) => {} PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index e3e7f91071713..e5f21013ce3e3 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -9,7 +9,8 @@ use rustc_index::vec::IndexVec; use rustc_span::symbol::{kw, Symbol}; use rustc_span::{BytePos, Span}; -use super::OperandValue; +use super::operand::OperandValue; +use super::place::PlaceRef; use super::{FunctionCx, LocalRef}; pub struct FunctionDebugContext { @@ -111,8 +112,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { /// Apply debuginfo and/or name, after creating the `alloca` for a local, /// or initializing the local with an operand (whichever applies). - // FIXME(eddyb) use `llvm.dbg.value` (which would work for operands), - // not just `llvm.dbg.declare` (which requires `alloca`). pub fn debug_introduce_local(&self, bx: &mut Bx, local: mir::Local) { let full_debug_info = bx.sess().opts.debuginfo == DebugInfo::Full; @@ -180,38 +179,60 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let local_ref = &self.locals[local]; - if !bx.sess().fewer_names() { - let name = match whole_local_var.or(fallback_var) { + let name = if bx.sess().fewer_names() { + None + } else { + Some(match whole_local_var.or(fallback_var) { Some(var) if var.name != kw::Invalid => var.name.to_string(), _ => format!("{:?}", local), - }; + }) + }; + + if let Some(name) = &name { match local_ref { LocalRef::Place(place) | LocalRef::UnsizedPlace(place) => { - bx.set_var_name(place.llval, &name); + bx.set_var_name(place.llval, name); } LocalRef::Operand(Some(operand)) => match operand.val { OperandValue::Ref(x, ..) | OperandValue::Immediate(x) => { - bx.set_var_name(x, &name); + bx.set_var_name(x, name); } OperandValue::Pair(a, b) => { // FIXME(eddyb) these are scalar components, // maybe extract the high-level fields? bx.set_var_name(a, &(name.clone() + ".0")); - bx.set_var_name(b, &(name + ".1")); + bx.set_var_name(b, &(name.clone() + ".1")); } }, LocalRef::Operand(None) => {} } } - if !full_debug_info { + if !full_debug_info || vars.is_empty() && fallback_var.is_none() { return; } - // FIXME(eddyb) add debuginfo for unsized places too. let base = match local_ref { - LocalRef::Place(place) => place, - _ => return, + LocalRef::Operand(None) => return, + + LocalRef::Operand(Some(operand)) => { + // "Spill" the value onto the stack, for debuginfo, + // without forcing non-debuginfo uses of the local + // to also load from the stack every single time. + // FIXME(#68817) use `llvm.dbg.value` instead, + // at least for the cases which LLVM handles correctly. + let spill_slot = PlaceRef::alloca(bx, operand.layout); + if let Some(name) = name { + bx.set_var_name(spill_slot.llval, &(name + ".dbg.spill")); + } + operand.val.store(bx, spill_slot); + spill_slot + } + + LocalRef::Place(place) => *place, + + // FIXME(eddyb) add debuginfo for unsized places too. + LocalRef::UnsizedPlace(_) => return, }; let vars = vars.iter().copied().chain(fallback_var); diff --git a/src/test/debuginfo/issue-12886.rs b/src/test/debuginfo/issue-12886.rs index 892f7d9485f9c..389221cbbf151 100644 --- a/src/test/debuginfo/issue-12886.rs +++ b/src/test/debuginfo/issue-12886.rs @@ -6,7 +6,7 @@ // gdb-command:run // gdb-command:next -// gdb-check:[...]25[...]s +// gdb-check:[...]24[...]let s = Some(5).unwrap(); // #break // gdb-command:continue #![feature(omit_gdb_pretty_printer_section)]