[DebugInfo] Keep parameter DBG_VALUEs before prologue code

[dstenb]

Summary

This is a preparatory change for removing the code from
DebugHandlerBase::beginFunction() which changes the starting label for
the first non-overlapping DBG_VALUEs of parameters to the beginning of
the function. It does that to be able to show parameters when entering a
function. However, that code does not consider what defines the values,
which can result in the ranges for the debug values starting before
their defining instructions. That code is removed in a follow-up patch.

When prologue code is inserted, it leads to DBG_VALUEs that start
directly in the entry block being moved down after the prologue
instructions. This patch fixes that by stashing away DBG_VALUEs for
parameters before emitting the prologue, and then reinserts them at the
start of the block. This assumes that there is no target that somehow
clobbers parameter registers in the frame setup; there is no such case
in the lit tests at least.

See PR40188 for more information.

[jmorse]

LGTM, although I'll wait for someone else to approve.

[bjope]

test/DebugInfo/X86/prolog-params.mir

1 ↗

(On Diff #184494)

Add -mtriple=x86_64-unknown-linux-gnu to the RUN line.

[dstenb]

Add -mtriple to RUN line.

[aprantl]

lib/CodeGen/PrologEpilogInserter.cpp

174 ↗

(On Diff #185264)

Can you add a sentence explaining what stashing is for, something like "... the stashed DBG_VALUEs will be processed later by ..."?

183 ↗

(On Diff #185264)

What kind of DebugInstr in MIR are not also a DebugValue?
Is this condition equivalent to

assert(MI.isDebugValue()) ?

184 ↗

(On Diff #185264)

Nevermind. The new labels probably.

[dstenb]

Amend comment for stashEntryDbgValues().

[dstenb]

lib/CodeGen/PrologEpilogInserter.cpp

184 ↗

(On Diff #185264)

Yes, this is for labels. In the cases I have looked at, the DBG_LABEL instructions are placed after the parameter DBG_VALUE instructions, but I guess there might cases where they are intermixed, or it could change in the future so that the labels are placed before.

[dstenb]

Thanks for the reviews! I'm intending on landing this tomorrow.

llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp

void PEI::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addPreserved<MachineDominatorTree>();
   AU.addRequired<MachineOptimizationRemarkEmitterPass>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
 /// StackObjSet - A set of stack object indexes
 using StackObjSet = SmallSetVector<int, 8>;
 
+using SavedDbgValuesMap =
+    SmallDenseMap<MachineBasicBlock *, SmallVector<MachineInstr *, 4>, 4>;
+
+/// Stash DBG_VALUEs that describe parameters and which are placed at the start
+/// of the block. Later on, after the prologue code has been emitted, the
+/// stashed DBG_VALUEs will be reinserted at the start of the block.
+static void stashEntryDbgValues(MachineBasicBlock &MBB,
+                                SavedDbgValuesMap &EntryDbgValues) {
+  SmallVector<const MachineInstr *, 4> FrameIndexValues;
+
+  for (auto &MI : MBB) {
+    if (!MI.isDebugInstr())
+      break;
+    if (!MI.isDebugValue() || !MI.getDebugVariable()->isParameter())
+      continue;
+    if (MI.getOperand(0).isFI()) {
+      // We can only emit valid locations for frame indices after the frame
+      // setup, so do not stash away them.
+      FrameIndexValues.push_back(&MI);
+      continue;
+    }
+    const DILocalVariable *Var = MI.getDebugVariable();
+    const DIExpression *Expr = MI.getDebugExpression();
+    auto Overlaps = [Var, Expr](const MachineInstr *DV) {
+      return Var == DV->getDebugVariable() &&
+             Expr->fragmentsOverlap(DV->getDebugExpression());
+    };
+    // See if the debug value overlaps with any preceding debug value that will
+    // not be stashed. If that is the case, then we can't stash this value, as
+    // we would then reorder the values at reinsertion.
+    if (llvm::none_of(FrameIndexValues, Overlaps))
+      EntryDbgValues[&MBB].push_back(&MI);
+  }
+
+  // Remove stashed debug values from the block.
+  if (EntryDbgValues.count(&MBB))
+    for (auto *MI : EntryDbgValues[&MBB])
+      MI->removeFromParent();
+}
+
 /// runOnMachineFunction - Insert prolog/epilog code and replace abstract
 /// frame indexes with appropriate references.
 bool PEI::runOnMachineFunction(MachineFunction &MF) {
   NumFuncSeen++;
   const Function &F = MF.getFunction();
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
   const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
 
   RS = TRI->requiresRegisterScavenging(MF) ? new RegScavenger() : nullptr;
   FrameIndexVirtualScavenging = TRI->requiresFrameIndexScavenging(MF);
   FrameIndexEliminationScavenging = (RS && !FrameIndexVirtualScavenging) ||
     TRI->requiresFrameIndexReplacementScavenging(MF);
   ORE = &getAnalysis<MachineOptimizationRemarkEmitterPass>().getORE();
 
   // Calculate the MaxCallFrameSize and AdjustsStack variables for the
   // function's frame information. Also eliminates call frame pseudo
   // instructions.
   calculateCallFrameInfo(MF);
 
   // Determine placement of CSR spill/restore code and prolog/epilog code:
   // place all spills in the entry block, all restores in return blocks.
   calculateSaveRestoreBlocks(MF);
 
+  // Stash away DBG_VALUEs that should not be moved by insertion of prolog code.
+  SavedDbgValuesMap EntryDbgValues;
+  for (MachineBasicBlock *SaveBlock : SaveBlocks)
+    stashEntryDbgValues(*SaveBlock, EntryDbgValues);
+
   // Handle CSR spilling and restoring, for targets that need it.
   if (MF.getTarget().usesPhysRegsForPEI())
     spillCalleeSavedRegs(MF);
 
   // Allow the target machine to make final modifications to the function
   // before the frame layout is finalized.
   TFI->processFunctionBeforeFrameFinalized(MF, RS);
 
   // Calculate actual frame offsets for all abstract stack objects...
   calculateFrameObjectOffsets(MF);
 
   // Add prolog and epilog code to the function.  This function is required
   // to align the stack frame as necessary for any stack variables or
   // called functions.  Because of this, calculateCalleeSavedRegisters()
   // must be called before this function in order to set the AdjustsStack
   // and MaxCallFrameSize variables.
   if (!F.hasFnAttribute(Attribute::Naked))
     insertPrologEpilogCode(MF);
 
+  // Reinsert stashed debug values at the start of the entry blocks.
+  for (auto &I : EntryDbgValues)
+    I.first->insert(I.first->begin(), I.second.begin(), I.second.end());
+
   // Replace all MO_FrameIndex operands with physical register references
   // and actual offsets.
   //
   replaceFrameIndices(MF);
 
   // If register scavenging is needed, as we've enabled doing it as a
   // post-pass, scavenge the virtual registers that frame index elimination
   // inserted.

llvm/trunk/test/DebugInfo/COFF/register-variables.ll

 ;     int c = getint();
 ;     putint(c);
 ;   }
 ; }
 
 ; ASM: f:                                      # @f
 ; ASM: .Lfunc_begin0:
 ; ASM: # %bb.0:                                 # %entry
+; ASM:         #DEBUG_VALUE: f:p <- $ecx
 ; ASM:         pushq   %rsi
 ; ASM:         subq    $32, %rsp
-; ASM:         #DEBUG_VALUE: f:p <- $ecx
 ; ASM:         movl    %ecx, %esi
 ; ASM: [[p_ecx_esi:\.Ltmp.*]]:
 ; ASM:         #DEBUG_VALUE: f:p <- $esi
 ; ASM:         callq   getint
 ; ASM: [[after_getint:\.Ltmp.*]]:
 ; ASM:         #DEBUG_VALUE: a <- $eax
 ; ASM:         #DEBUG_VALUE: inlineinc:a <- $eax
 ; ASM:         #DEBUG_VALUE: c <- $eax

llvm/trunk/test/DebugInfo/X86/prolog-params.mir

+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=prologepilog %s -o - | FileCheck %s
+
+# MIR test case produced by compiling:
+#
+# typedef struct { int a; long b; } S1;
+#
+# extern void bar(int *);
+#
+# void foo(int p1, S1 p2, int p3, int p4, int p5, int p6, int p7, int p8) {
+#   int local1 = p1;
+#   int local2 = p6;
+#   int arr[2];
+#   bar(arr);
+# }
+#
+# using the following commands (with some manual stripping of metadata):
+#
+# $ clang -O1 -g -S -emit-llvm
+# $ llc -O1 -stop-before=prologepilog
+
+--- |
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+
+  ; Function Attrs: nounwind uwtable
+  define dso_local void @foo(i32 %p1, i32 %p2.coerce0, i64 %p2.coerce1, i32 %p3, i32 %p4, i32 %p5, i32 %p6, i32 %p7, i32 %p8) local_unnamed_addr #0 !dbg !7 {
+  entry:
+    %arr = alloca [2 x i32], align 4
+    call void @llvm.dbg.value(metadata i32 %p1, metadata !18, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p2.coerce0, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !28
+    call void @llvm.dbg.value(metadata i64 %p2.coerce1, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p3, metadata !20, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p4, metadata !21, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p5, metadata !22, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p6, metadata !23, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p7, metadata !24, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p8, metadata !25, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p1, metadata !26, metadata !DIExpression()), !dbg !28
+    call void @llvm.dbg.value(metadata i32 %p6, metadata !27, metadata !DIExpression()), !dbg !28
+    %arraydecay1 = bitcast [2 x i32]* %arr to i32*, !dbg !28
+    call void @bar(i32* nonnull %arraydecay1), !dbg !28
+    ret void, !dbg !28
+  }
+
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+  declare dso_local void @bar(i32*) local_unnamed_addr
+
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+  attributes #0 = { nounwind uwtable }
+  attributes #1 = { nounwind readnone speculatable }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5}
+  !llvm.ident = !{!6}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+  !1 = !DIFile(filename: "params.c", directory: "/")
+  !2 = !{}
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 4}
+  !6 = !{!"clang version 9.0.0"}
+  !7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 5, type: !8, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !17)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{null, !10, !11, !10, !10, !10, !10, !10, !10}
+  !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !11 = !DIDerivedType(tag: DW_TAG_typedef, name: "S1", file: !1, line: 1, baseType: !12)
+  !12 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !1, line: 1, size: 128, elements: !13)
+  !13 = !{!14, !15}
+  !14 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !12, file: !1, line: 1, baseType: !10, size: 32)
+  !15 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !12, file: !1, line: 1, baseType: !16, size: 64, offset: 64)
+  !16 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+  !17 = !{!18, !19, !20, !21, !22, !23, !24, !25, !26, !27}
+  !18 = !DILocalVariable(name: "p1", arg: 1, scope: !7, file: !1, line: 5, type: !10)
+  !19 = !DILocalVariable(name: "p2", arg: 2, scope: !7, file: !1, line: 5, type: !11)
+  !20 = !DILocalVariable(name: "p3", arg: 3, scope: !7, file: !1, line: 5, type: !10)
+  !21 = !DILocalVariable(name: "p4", arg: 4, scope: !7, file: !1, line: 5, type: !10)
+  !22 = !DILocalVariable(name: "p5", arg: 5, scope: !7, file: !1, line: 5, type: !10)
+  !23 = !DILocalVariable(name: "p6", arg: 6, scope: !7, file: !1, line: 5, type: !10)
+  !24 = !DILocalVariable(name: "p7", arg: 7, scope: !7, file: !1, line: 5, type: !10)
+  !25 = !DILocalVariable(name: "p8", arg: 8, scope: !7, file: !1, line: 5, type: !10)
+  !26 = !DILocalVariable(name: "local1", scope: !7, file: !1, line: 6, type: !10)
+  !27 = !DILocalVariable(name: "local2", scope: !7, file: !1, line: 7, type: !10)
+  !28 = !DILocation(line: 5, column: 21, scope: !7)
+
+...
+---
+name:            foo
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, type: default, offset: 16, size: 4, alignment: 16, stack-id: 0,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true }
+  - { id: 1, type: default, offset: 8, size: 4, alignment: 8, stack-id: 0,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true }
+  - { id: 2, type: default, offset: 0, size: 4, alignment: 16, stack-id: 0,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true }
+stack:
+  - { id: 0, name: arr, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true }
+body:             |
+  bb.0.entry:
+    DBG_VALUE $edi, $noreg, !18, !DIExpression(), debug-location !28
+    DBG_VALUE $esi, $noreg, !19, !DIExpression(DW_OP_LLVM_fragment, 0, 32), debug-location !28
+    DBG_VALUE $rdx, $noreg, !19, !DIExpression(DW_OP_LLVM_fragment, 64, 64), debug-location !28
+    DBG_VALUE $ecx, $noreg, !20, !DIExpression(), debug-location !28
+    DBG_VALUE $r8d, $noreg, !21, !DIExpression(), debug-location !28
+    DBG_VALUE %fixed-stack.2, 0, !23, !DIExpression(), debug-location !28
+    DBG_VALUE $r9d, $noreg, !22, !DIExpression(), debug-location !28
+    DBG_VALUE $r9d, $noreg, !23, !DIExpression(), debug-location !28
+    DBG_VALUE %fixed-stack.1, 0, !24, !DIExpression(), debug-location !28
+    DBG_VALUE %fixed-stack.0, 0, !25, !DIExpression(), debug-location !28
+    DBG_VALUE $edi, $noreg, !26, !DIExpression(), debug-location !28
+    DBG_VALUE %fixed-stack.2, 0, !27, !DIExpression(), debug-location !28
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !28
+    renamable $rdi = LEA64r %stack.0.arr, 1, $noreg, 0, $noreg
+    CALL64pcrel32 @bar, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, debug-location !28
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !28
+    RET 0, debug-location !28
+
+...
+
+# CHECK: ![[P1:.*]] = !DILocalVariable(name: "p1"
+# CHECK: ![[P2:.*]] = !DILocalVariable(name: "p2"
+# CHECK: ![[P3:.*]] = !DILocalVariable(name: "p3"
+# CHECK: ![[P4:.*]] = !DILocalVariable(name: "p4"
+# CHECK: ![[P5:.*]] = !DILocalVariable(name: "p5"
+# CHECK: ![[P6:.*]] = !DILocalVariable(name: "p6"
+# CHECK: ![[P7:.*]] = !DILocalVariable(name: "p7"
+# CHECK: ![[P8:.*]] = !DILocalVariable(name: "p8"
+# CHECK: ![[LOCAL1:.*]] = !DILocalVariable(name: "local1"
+# CHECK: ![[LOCAL2:.*]] = !DILocalVariable(name: "local2"
+
+# Please note that an additional DBG_VALUE has been added for p6 to see that
+# FI- and register-described DBG_VALUEs for the same variable are not reordered.
+
+# Parameters passed in registers are emitted before the prologue code.
+# CHECK: bb.0.entry:
+# CHECK-NEXT: DBG_VALUE $edi, $noreg, ![[P1]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $esi, $noreg, ![[P2]], !DIExpression(DW_OP_LLVM_fragment, 0, 32)
+# CHECK-NEXT: DBG_VALUE $rdx, $noreg, ![[P2]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+# CHECK-NEXT: DBG_VALUE $ecx, $noreg, ![[P3]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $r8d, $noreg, ![[P4]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $r9d, $noreg, ![[P5]], !DIExpression()
+# CHECK-NEXT: frame-setup PUSH64r
+
+# Locals and stack parameters are emitted after the prologue code.
+# CHECK: DBG_VALUE $rsp, 0, ![[P6]], !DIExpression(DW_OP_plus_uconst, 16)
+# CHECK-NEXT: DBG_VALUE $r9d, $noreg, ![[P6]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $rsp, 0, ![[P7]], !DIExpression(DW_OP_plus_uconst, 24)
+# CHECK-NEXT: DBG_VALUE $rsp, 0, ![[P8]], !DIExpression(DW_OP_plus_uconst, 32)
+# CHECK-NEXT: DBG_VALUE $edi, $noreg, ![[LOCAL1]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $rsp, 0, ![[LOCAL2]], !DIExpression(DW_OP_plus_uconst, 16)