Auto merge of rust-lang#130329 - khuey:reorder-constant-spills, r=dav…

…idtwco

Reorder stack spills so that constants come later.

Currently constants are "pulled forward" and have their stack spills emitted first. This confuses LLVM as to where to place breakpoints at function entry, and results in argument values being wrong in the debugger. It's straightforward to avoid emitting the stack spills for constants until arguments/etc have been introduced in debug_introduce_locals, so do that.

Example LLVM IR (irrelevant IR elided):
Before:
```
define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start:
  %c.dbg.spill = alloca [8 x i8], align 8
  %b.dbg.spill = alloca [8 x i8], align 8
  %a.dbg.spill = alloca [8 x i8], align 8
  %x.dbg.spill = alloca [4 x i8], align 4
  store i32 0, ptr %x.dbg.spill, align 4, !dbg !192            ; LLVM places breakpoint here.
    #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !192)
  store i64 %a, ptr %a.dbg.spill, align 8
    #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !193)
  store i64 %b, ptr %b.dbg.spill, align 8
    #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !194)
  store double %c, ptr %c.dbg.spill, align 8
    #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !195)
  ret void, !dbg !196
}
```
After:
```
define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start:
  %x.dbg.spill = alloca [4 x i8], align 4
  %c.dbg.spill = alloca [8 x i8], align 8
  %b.dbg.spill = alloca [8 x i8], align 8
  %a.dbg.spill = alloca [8 x i8], align 8
  store i64 %a, ptr %a.dbg.spill, align 8
    #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !192)
  store i64 %b, ptr %b.dbg.spill, align 8
    #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !193)
  store double %c, ptr %c.dbg.spill, align 8
    #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !194)
  store i32 0, ptr %x.dbg.spill, align 4, !dbg !195            ; LLVM places breakpoint here.
    #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !195)
  ret void, !dbg !196
}
```
Note in particular the position of the "LLVM places breakpoint here" comment relative to the stack spills for the function arguments. LLVM assumes that the first instruction with with a debug location is the end of the prologue. As LLVM does not currently offer front ends any direct control over the placement of the prologue end reordering the IR is the only mechanism available to fix argument values at function entry in the presence of MIR optimizations like SingleUseConsts. Fixes rust-lang#128945

r? `@michaelwoerister`

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -1,4 +1,5 @@
 use std::collections::hash_map::Entry;
+use std::marker::PhantomData;
 use std::ops::Range;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -14,7 +15,7 @@ use rustc_target::abi::{Abi, FieldIdx, FieldsShape, Size, VariantIdx};
 
 use super::operand::{OperandRef, OperandValue};
 use super::place::{PlaceRef, PlaceValue};
-use super::{FunctionCx, LocalRef};
+use super::{FunctionCx, LocalRef, PerLocalVarDebugInfoIndexVec};
 use crate::traits::*;
 
 pub struct FunctionDebugContext<'tcx, S, L> {
@@ -48,6 +49,17 @@ pub struct PerLocalVarDebugInfo<'tcx, D> {
     pub projection: &'tcx ty::List<mir::PlaceElem<'tcx>>,
 }
 
+/// Information needed to emit a constant.
+pub struct ConstDebugInfo<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
+    pub name: String,
+    pub source_info: mir::SourceInfo,
+    pub operand: OperandRef<'tcx, Bx::Value>,
+    pub dbg_var: Bx::DIVariable,
+    pub dbg_loc: Bx::DILocation,
+    pub fragment: Option<Range<Size>>,
+    pub _phantom: PhantomData<&'a ()>,
+}
+
 #[derive(Clone, Copy, Debug)]
 pub struct DebugScope<S, L> {
     pub dbg_scope: S,
@@ -427,19 +439,36 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         }
     }
 
-    pub(crate) fn debug_introduce_locals(&self, bx: &mut Bx) {
+    pub(crate) fn debug_introduce_locals(
+        &self,
+        bx: &mut Bx,
+        consts: Vec<ConstDebugInfo<'a, 'tcx, Bx>>,
+    ) {
         if bx.sess().opts.debuginfo == DebugInfo::Full || !bx.sess().fewer_names() {
             for local in self.locals.indices() {
                 self.debug_introduce_local(bx, local);
             }
+
+            for ConstDebugInfo { name, source_info, operand, dbg_var, dbg_loc, fragment, .. } in
+                consts.into_iter()
+            {
+                self.set_debug_loc(bx, source_info);
+                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);
+            }
         }
     }
 
     /// Partition all `VarDebugInfo` in `self.mir`, by their base `Local`.
     pub(crate) fn compute_per_local_var_debug_info(
         &self,
         bx: &mut Bx,
-    ) -> Option<IndexVec<mir::Local, Vec<PerLocalVarDebugInfo<'tcx, Bx::DIVariable>>>> {
+    ) -> Option<(
+        PerLocalVarDebugInfoIndexVec<'tcx, Bx::DIVariable>,
+        Vec<ConstDebugInfo<'a, 'tcx, Bx>>,
+    )> {
         let full_debug_info = self.cx.sess().opts.debuginfo == DebugInfo::Full;
 
         let target_is_msvc = self.cx.sess().target.is_like_msvc;
@@ -449,6 +478,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         }
 
         let mut per_local = IndexVec::from_elem(vec![], &self.mir.local_decls);
+        let mut constants = vec![];
         let mut params_seen: FxHashMap<_, Bx::DIVariable> = Default::default();
         for var in &self.mir.var_debug_info {
             let dbg_scope_and_span = if full_debug_info {
@@ -545,23 +575,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         let Some(dbg_loc) = self.dbg_loc(var.source_info) else { continue };
 
                         let operand = self.eval_mir_constant_to_operand(bx, &c);
-                        self.set_debug_loc(bx, var.source_info);
-                        let base =
-                            Self::spill_operand_to_stack(operand, Some(var.name.to_string()), bx);
-                        bx.clear_dbg_loc();
-
-                        bx.dbg_var_addr(
+                        constants.push(ConstDebugInfo {
+                            name: var.name.to_string(),
+                            source_info: var.source_info,
+                            operand,
                             dbg_var,
                             dbg_loc,
-                            base.val.llval,
-                            Size::ZERO,
-                            &[],
                             fragment,
-                        );
+                            _phantom: PhantomData,
+                        });
                     }
                 }
             }
         }
-        Some(per_local)
+        Some((per_local, constants))
     }
 }

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -41,6 +41,9 @@ enum CachedLlbb<T> {
     Skip,
 }
 
+type PerLocalVarDebugInfoIndexVec<'tcx, V> =
+    IndexVec<mir::Local, Vec<PerLocalVarDebugInfo<'tcx, V>>>;
+
 /// Master context for codegenning from MIR.
 pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
     instance: Instance<'tcx>,
@@ -107,8 +110,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
 
     /// All `VarDebugInfo` from the MIR body, partitioned by `Local`.
     /// This is `None` if no variable debuginfo/names are needed.
-    per_local_var_debug_info:
-        Option<IndexVec<mir::Local, Vec<PerLocalVarDebugInfo<'tcx, Bx::DIVariable>>>>,
+    per_local_var_debug_info: Option<PerLocalVarDebugInfoIndexVec<'tcx, Bx::DIVariable>>,
 
     /// Caller location propagated if this function has `#[track_caller]`.
     caller_location: Option<OperandRef<'tcx, Bx::Value>>,
@@ -216,7 +218,9 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     // monomorphization, and if there is an error during collection then codegen never starts -- so
     // we don't have to do it again.
 
-    fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);
+    let (per_local_var_debug_info, consts_debug_info) =
+        fx.compute_per_local_var_debug_info(&mut start_bx).unzip();
+    fx.per_local_var_debug_info = per_local_var_debug_info;
 
     let traversal_order = traversal::mono_reachable_reverse_postorder(mir, cx.tcx(), instance);
     let memory_locals = analyze::non_ssa_locals(&fx, &traversal_order);
@@ -268,7 +272,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     fx.initialize_locals(local_values);
 
     // Apply debuginfo to the newly allocated locals.
-    fx.debug_introduce_locals(&mut start_bx);
+    fx.debug_introduce_locals(&mut start_bx, consts_debug_info.unwrap_or_default());
 
     // If the backend supports coverage, and coverage is enabled for this function,
     // do any necessary start-of-function codegen (e.g. locals for MC/DC bitmaps).

tests/debuginfo/constant-ordering-prologue.rs

@@ -0,0 +1,35 @@
+//@ min-lldb-version: 310
+
+//@ compile-flags:-g
+
+// === GDB TESTS ===================================================================================
+
+// gdb-command:break constant_ordering_prologue::binding
+// gdb-command:run
+
+// gdb-command:print a
+// gdb-check: = 19
+// gdb-command:print b
+// gdb-check: = 20
+// gdb-command:print c
+// gdb-check: = 21.5
+
+// === LLDB TESTS ==================================================================================
+
+// lldb-command:b "constant_ordering_prologue::binding"
+// lldb-command:run
+
+// lldb-command:print a
+// lldb-check: = 19
+// lldb-command:print b
+// lldb-check: = 20
+// lldb-command:print c
+// lldb-check: = 21.5
+
+fn binding(a: i64, b: u64, c: f64) {
+    let x = 0;
+}
+
+fn main() {
+    binding(19, 20, 21.5);
+}