rustc_codegen_ssa: only "spill" SSA-like values to the stack for debuginfo.

Author: eddyb

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)

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<D> {
@@ -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);

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)]