[llvm] [CodeGen][RAGreedy] Inform LiveDebugVariables about snippets spilled by InlineSpiller. (PR #109962)

Bevin Hansson via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 25 05:12:48 PDT 2024


https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/109962

>From 7b199e108b4ef0bd5bdb57502ae32511b4dd7224 Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Wed, 25 Sep 2024 12:59:06 +0200
Subject: [PATCH 1/2] [CodeGen][RAGreedy] Inform LiveDebugVariables about
 snippets spilled by InlineSpiller.

RAGreedy invokes InlineSpiller to spill a particular virtreg inline. When the
spiller does this, it also identifies small, adjacent liveranges called
snippets. These are also spilled or rematerialized in the process.

However, the spiller does not inform RA that it has spilled these regs.
This means that debug variable locations referencing these regs/ranges are
lost.

Mark any spilled regs which do not have a stack slot assigned to them as
allocated to the slot being spilled to to tell LDV that those regs are
located in that slot, even though the regs might no longer exist in the
program after regalloc is finished. Also, inform RA about all of the regs
which were replaced (spilled or rematted), not just the one that was
requested so that it can properly manage the ranges of the debug vars.
---
 llvm/include/llvm/CodeGen/Spiller.h           |  2 +
 llvm/lib/CodeGen/InlineSpiller.cpp            | 15 ++++-
 llvm/lib/CodeGen/RegAllocGreedy.cpp           |  3 +-
 .../test/CodeGen/X86/debug-spilled-snippet.ll | 55 +++++++++++++++++++
 4 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/debug-spilled-snippet.ll

diff --git a/llvm/include/llvm/CodeGen/Spiller.h b/llvm/include/llvm/CodeGen/Spiller.h
index b2f5485eba02e7..0731db533ad839 100644
--- a/llvm/include/llvm/CodeGen/Spiller.h
+++ b/llvm/include/llvm/CodeGen/Spiller.h
@@ -30,6 +30,8 @@ class Spiller {
   /// spill - Spill the LRE.getParent() live interval.
   virtual void spill(LiveRangeEdit &LRE) = 0;
 
+  virtual ArrayRef<Register> getReplacedRegs() = 0;
+
   virtual void postOptimization() {}
 };
 
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 409879a8b49bcc..93027678c0652b 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -167,6 +167,10 @@ class InlineSpiller : public Spiller {
   // All registers to spill to StackSlot, including the main register.
   SmallVector<Register, 8> RegsToSpill;
 
+  // All registers that were replaced by the spiller. This includes registers
+  // that were rematerialized; rematted regs are removed from RegsToSpill.
+  SmallVector<Register, 8> RegsReplaced;
+
   // All COPY instructions to/from snippets.
   // They are ignored since both operands refer to the same stack slot.
   // For bundled copies, this will only include the first header copy.
@@ -199,6 +203,7 @@ class InlineSpiller : public Spiller {
         HSpiller(Pass, MF, VRM), VRAI(VRAI) {}
 
   void spill(LiveRangeEdit &) override;
+  ArrayRef<Register> getReplacedRegs() override { return RegsReplaced; }
   void postOptimization() override;
 
 private:
@@ -385,6 +390,7 @@ void InlineSpiller::collectRegsToSpill() {
   // Main register always spills.
   RegsToSpill.assign(1, Reg);
   SnippetCopies.clear();
+  RegsReplaced.assign(RegsToSpill);
 
   // Snippets all have the same original, so there can't be any for an original
   // register.
@@ -405,6 +411,8 @@ void InlineSpiller::collectRegsToSpill() {
     LLVM_DEBUG(dbgs() << "\talso spill snippet " << SnipLI << '\n');
     ++NumSnippets;
   }
+
+  RegsReplaced.assign(RegsToSpill);
 }
 
 bool InlineSpiller::isSibling(Register Reg) {
@@ -1255,8 +1263,13 @@ void InlineSpiller::spillAll() {
   LLVM_DEBUG(dbgs() << "Merged spilled regs: " << *StackInt << '\n');
 
   // Spill around uses of all RegsToSpill.
-  for (Register Reg : RegsToSpill)
+  for (Register Reg : RegsToSpill) {
     spillAroundUses(Reg);
+    // Assign all of the spilled registers to the slot so that
+    // LiveDebugVariables knows about these locations later on.
+    if (VRM.getStackSlot(Reg) == VirtRegMap::NO_STACK_SLOT)
+      VRM.assignVirt2StackSlot(Reg, StackSlot);
+  }
 
   // Hoisted spills may cause dead code.
   if (!DeadDefs.empty()) {
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 1ad70c86d68e3d..60d1c7e53316c9 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2504,7 +2504,8 @@ MCRegister RAGreedy::selectOrSplitImpl(const LiveInterval &VirtReg,
     // Tell LiveDebugVariables about the new ranges. Ranges not being covered by
     // the new regs are kept in LDV (still mapping to the old register), until
     // we rewrite spilled locations in LDV at a later stage.
-    DebugVars->splitRegister(VirtReg.reg(), LRE.regs(), *LIS);
+    for (Register r : spiller().getReplacedRegs())
+      DebugVars->splitRegister(r, LRE.regs(), *LIS);
 
     if (VerifyEnabled)
       MF->verify(this, "After spilling", &errs());
diff --git a/llvm/test/CodeGen/X86/debug-spilled-snippet.ll b/llvm/test/CodeGen/X86/debug-spilled-snippet.ll
new file mode 100644
index 00000000000000..c05e93f5b15d65
--- /dev/null
+++ b/llvm/test/CodeGen/X86/debug-spilled-snippet.ll
@@ -0,0 +1,55 @@
+; RUN: llc -O3 -mtriple i386 %s -stop-after=livedebugvalues -o - | FileCheck %s
+
+; There should be multiple debug values for this variable after regalloc. The
+; value has been spilled, but we shouldn't lose track of the location because
+; of this.
+
+; CHECK-COUNT-4: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+
+define void @main(i32 %call, i32 %xor.i, i1 %tobool4.not, i32 %.pre) #0 !dbg !4 {
+entry:
+  %tobool1.not = icmp ne i32 %call, 0
+  %spec.select = zext i1 %tobool1.not to i32
+  br label %for.body5
+
+for.cond.loopexit.loopexit:                       ; preds = %for.body5
+    #dbg_value(i32 %spec.select, !6, !DIExpression(), !10)
+  %tobool.not.i53 = icmp eq i32 %spec.select, 0
+  br i1 %tobool.not.i53, label %transparent_crc.exit57, label %if.then.i54
+
+for.body5:                                        ; preds = %for.body5, %entry
+  %0 = phi i32 [ 0, %entry ], [ %xor1.i40.i, %for.body5 ]
+  %xor6.i = xor i32 %.pre, %0
+  %shr7.i = ashr i32 %xor6.i, 1
+  %xor17.i = xor i32 %shr7.i, %call
+  %shr18.i = ashr i32 %xor17.i, 1
+  %xor.i.i = xor i32 %shr18.i, %xor.i
+  %arrayidx.i.i = getelementptr [0 x i32], ptr null, i32 0, i32 %xor.i.i
+  %xor1.i40.i = xor i32 %xor.i.i, %call
+  br i1 %tobool4.not, label %for.cond.loopexit.loopexit, label %for.body5
+
+if.then.i54:                                      ; preds = %for.cond.loopexit.loopexit
+  store i64 0, ptr null, align 4
+  br label %transparent_crc.exit57
+
+transparent_crc.exit57:                           ; preds = %if.then.i54, %for.cond.loopexit.loopexit
+  ret void
+}
+
+attributes #0 = { "frame-pointer"="all" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 20.0.0git.prerel", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "xx.c", directory: "/path", checksumkind: CSK_MD5, checksum: "c4b2fc62bca9171ad484c91fb78b8842")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 20, type: !5, scopeLine: 20, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!5 = !DISubroutineType(types: !2)
+!6 = !DILocalVariable(name: "flag", arg: 2, scope: !7, file: !1, line: 8, type: !9)
+!7 = distinct !DISubprogram(name: "transparent_crc", scope: !1, file: !1, line: 8, type: !8, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!8 = distinct !DISubroutineType(types: !2)
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !DILocation(line: 0, scope: !7, inlinedAt: !11)
+!11 = distinct !DILocation(line: 28, column: 3, scope: !4)

>From ca3088e7d14ce6da0d5192cb99dd6b6cc70c294d Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Wed, 25 Sep 2024 14:11:27 +0200
Subject: [PATCH 2/2] Separate spilled and rematted regs, and add a MIR test.

---
 llvm/include/llvm/CodeGen/Spiller.h           |   5 +
 llvm/lib/CodeGen/InlineSpiller.cpp            |  10 +-
 llvm/lib/CodeGen/RegAllocGreedy.cpp           |   2 +
 .../CodeGen/X86/debug-spilled-snippet.mir     | 167 ++++++++++++++++++
 4 files changed, 179 insertions(+), 5 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/debug-spilled-snippet.mir

diff --git a/llvm/include/llvm/CodeGen/Spiller.h b/llvm/include/llvm/CodeGen/Spiller.h
index 0731db533ad839..4568cdfab86288 100644
--- a/llvm/include/llvm/CodeGen/Spiller.h
+++ b/llvm/include/llvm/CodeGen/Spiller.h
@@ -30,6 +30,11 @@ class Spiller {
   /// spill - Spill the LRE.getParent() live interval.
   virtual void spill(LiveRangeEdit &LRE) = 0;
 
+  /// Return the registers that were spilled.
+  virtual ArrayRef<Register> getSpilledRegs() = 0;
+
+  /// Return registers that were not spilled, but otherwise replaced
+  /// (e.g. rematerialized).
   virtual ArrayRef<Register> getReplacedRegs() = 0;
 
   virtual void postOptimization() {}
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 93027678c0652b..836f885c189745 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -167,8 +167,8 @@ class InlineSpiller : public Spiller {
   // All registers to spill to StackSlot, including the main register.
   SmallVector<Register, 8> RegsToSpill;
 
-  // All registers that were replaced by the spiller. This includes registers
-  // that were rematerialized; rematted regs are removed from RegsToSpill.
+  // All registers that were replaced by the spiller through some other method,
+  // e.g. rematerialization.
   SmallVector<Register, 8> RegsReplaced;
 
   // All COPY instructions to/from snippets.
@@ -203,6 +203,7 @@ class InlineSpiller : public Spiller {
         HSpiller(Pass, MF, VRM), VRAI(VRAI) {}
 
   void spill(LiveRangeEdit &) override;
+  ArrayRef<Register> getSpilledRegs() override { return RegsToSpill; }
   ArrayRef<Register> getReplacedRegs() override { return RegsReplaced; }
   void postOptimization() override;
 
@@ -390,7 +391,7 @@ void InlineSpiller::collectRegsToSpill() {
   // Main register always spills.
   RegsToSpill.assign(1, Reg);
   SnippetCopies.clear();
-  RegsReplaced.assign(RegsToSpill);
+  RegsReplaced.clear();
 
   // Snippets all have the same original, so there can't be any for an original
   // register.
@@ -411,8 +412,6 @@ void InlineSpiller::collectRegsToSpill() {
     LLVM_DEBUG(dbgs() << "\talso spill snippet " << SnipLI << '\n');
     ++NumSnippets;
   }
-
-  RegsReplaced.assign(RegsToSpill);
 }
 
 bool InlineSpiller::isSibling(Register Reg) {
@@ -813,6 +812,7 @@ void InlineSpiller::reMaterializeAll() {
 
     RegsToSpill[ResultPos++] = Reg;
   }
+  RegsReplaced.assign(RegsToSpill.begin() + ResultPos, RegsToSpill.end());
   RegsToSpill.erase(RegsToSpill.begin() + ResultPos, RegsToSpill.end());
   LLVM_DEBUG(dbgs() << RegsToSpill.size()
                     << " registers to spill after remat.\n");
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 60d1c7e53316c9..8cfd2192de460e 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2504,6 +2504,8 @@ MCRegister RAGreedy::selectOrSplitImpl(const LiveInterval &VirtReg,
     // Tell LiveDebugVariables about the new ranges. Ranges not being covered by
     // the new regs are kept in LDV (still mapping to the old register), until
     // we rewrite spilled locations in LDV at a later stage.
+    for (Register r : spiller().getSpilledRegs())
+      DebugVars->splitRegister(r, LRE.regs(), *LIS);
     for (Register r : spiller().getReplacedRegs())
       DebugVars->splitRegister(r, LRE.regs(), *LIS);
 
diff --git a/llvm/test/CodeGen/X86/debug-spilled-snippet.mir b/llvm/test/CodeGen/X86/debug-spilled-snippet.mir
new file mode 100644
index 00000000000000..85acd5e4c0f4c4
--- /dev/null
+++ b/llvm/test/CodeGen/X86/debug-spilled-snippet.mir
@@ -0,0 +1,167 @@
+# RUN: llc -O3 -mtriple i386 -start-before=greedy -stop-after=livedebugvars %s -o - | FileCheck %s
+
+# There should be multiple debug values for this variable after regalloc. The
+# value has been spilled, but we shouldn't lose track of the location because
+# of this.
+
+# CHECK-COUNT-4: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+
+--- |
+  
+  define void @main() #0 !dbg !4 {
+  entry:
+  #dbg_value(i32 undef, !6, !DIExpression(), !10)
+    ret void
+  }
+  
+  attributes #0 = { "frame-pointer"="all" }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 20.0.0git.prerel", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !2, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "xx.c", directory: "/path", checksumkind: CSK_MD5, checksum: "c4b2fc62bca9171ad484c91fb78b8842")
+  !2 = !{}
+  !3 = !{i32 2, !"Debug Info Version", i32 3}
+  !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 20, type: !5, scopeLine: 20, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !2)
+  !6 = !DILocalVariable(name: "flag", arg: 2, scope: !7, file: !1, line: 8, type: !9)
+  !7 = distinct !DISubprogram(name: "transparent_crc", scope: !1, file: !1, line: 8, type: !8, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+  !8 = distinct !DISubroutineType(types: !2)
+  !9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !10 = !DILocation(line: 0, scope: !7, inlinedAt: !11)
+  !11 = distinct !DILocation(line: 28, column: 3, scope: !4)
+
+...
+---
+name:            main
+alignment:       16
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+noPhis:          true
+isSSA:           false
+noVRegs:         false
+callsEHReturn:   false
+callsUnwindInit: false
+hasEHCatchret:   false
+hasEHScopes:     false
+hasEHFunclets:   false
+isOutlined:      false
+debugInstrRef:   false
+failsVerification: false
+tracksDebugUserValues: false
+registers:
+  - { id: 0, class: gr32_abcd, preferred-register: '' }
+  - { id: 1, class: gr32, preferred-register: '' }
+  - { id: 2, class: gr32, preferred-register: '' }
+  - { id: 3, class: gr32, preferred-register: '' }
+  - { id: 4, class: gr32, preferred-register: '' }
+  - { id: 5, class: gr8, preferred-register: '' }
+  - { id: 6, class: gr32, preferred-register: '' }
+  - { id: 7, class: gr32, preferred-register: '' }
+  - { id: 8, class: gr8, preferred-register: '' }
+  - { id: 9, class: gr32, preferred-register: '' }
+  - { id: 10, class: gr32, preferred-register: '' }
+  - { id: 11, class: gr32, preferred-register: '' }
+  - { id: 12, class: gr32, preferred-register: '' }
+  - { id: 13, class: gr32, preferred-register: '' }
+  - { id: 14, class: gr32, preferred-register: '' }
+  - { id: 15, class: gr32, preferred-register: '' }
+  - { id: 16, class: gr32, preferred-register: '' }
+  - { id: 17, class: gr32, preferred-register: '' }
+  - { id: 18, class: gr32_abcd, preferred-register: '' }
+  - { id: 19, class: gr32, preferred-register: '' }
+liveins:         []
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  functionContext: ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  hasTailCall:     false
+  isCalleeSavedInfoValid: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+  - { id: 0, type: default, offset: 12, size: 4, alignment: 4, stack-id: default, 
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, type: default, offset: 8, size: 1, alignment: 4, stack-id: default, 
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 2, type: default, offset: 4, size: 4, alignment: 4, stack-id: default, 
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 3, type: default, offset: 0, size: 4, alignment: 4, stack-id: default, 
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+stack:           []
+entry_values:    []
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo:
+  amxProgModel:    None
+body:             |
+  bb.0:
+    successors: %bb.2(0x80000000)
+  
+    %6:gr32 = MOV32rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load (s32) from %fixed-stack.0)
+    %5:gr8 = MOV8rm %fixed-stack.1, 1, $noreg, 0, $noreg :: (load (s8) from %fixed-stack.1, align 4)
+    %3:gr32 = MOV32rm %fixed-stack.3, 1, $noreg, 0, $noreg :: (load (s32) from %fixed-stack.3)
+    %0:gr32_abcd = MOV32r0 implicit-def dead $eflags
+    TEST32rr %3, %3, implicit-def $eflags
+    %0.sub_8bit:gr32_abcd = SETCCr 5, implicit $eflags
+    %11:gr32 = COPY %3
+    %11:gr32 = SAR32ri %11, 1, implicit-def dead $eflags
+    %4:gr32 = MOV32rm %fixed-stack.2, 1, $noreg, 0, $noreg :: (load (s32) from %fixed-stack.2)
+    %19:gr32 = MOV32r0 implicit-def dead $eflags
+    JMP_1 %bb.2
+  
+  bb.1:
+    successors: %bb.4(0x30000000), %bb.3(0x50000000)
+  
+    DBG_VALUE %0, $noreg, !6, !DIExpression(), debug-location !10
+    TEST32rr %0, %0, implicit-def $eflags
+    JCC_1 %bb.4, 4, implicit $eflags
+    JMP_1 %bb.3
+  
+  bb.2:
+    successors: %bb.1(0x04000000), %bb.2(0x7c000000)
+  
+    %19:gr32 = XOR32rr %19, %6, implicit-def dead $eflags
+    %19:gr32 = SAR32ri %19, 2, implicit-def dead $eflags
+    %19:gr32 = XOR32rr %19, %11, implicit-def dead $eflags
+    %19:gr32 = XOR32rr %19, %4, implicit-def dead $eflags
+    %19:gr32 = XOR32rr %19, %3, implicit-def dead $eflags
+    TEST8ri %5, 1, implicit-def $eflags
+    JCC_1 %bb.1, 5, implicit $eflags
+    JMP_1 %bb.2
+  
+  bb.3:
+    successors: %bb.4(0x80000000)
+  
+    MOV32mi $noreg, 1, $noreg, 4, $noreg, 0 :: (store (s32) into `ptr null` + 4)
+    MOV32mi $noreg, 1, $noreg, 0, $noreg, 0 :: (store (s32) into `ptr null`)
+  
+  bb.4:
+    RET 0
+
+...



More information about the llvm-commits mailing list