[llvm] 8367030 - [BranchFolder] Fix missing debug info with tail merging (#94715)

via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 20 10:48:22 PDT 2024


Author: Alan Zhao
Date: 2024-06-20T10:48:18-07:00
New Revision: 836703087d761f9cbf81b6f9593bc5313660f559

URL: https://github.com/llvm/llvm-project/commit/836703087d761f9cbf81b6f9593bc5313660f559
DIFF: https://github.com/llvm/llvm-project/commit/836703087d761f9cbf81b6f9593bc5313660f559.diff

LOG: [BranchFolder] Fix missing debug info with tail merging (#94715)

`BranchFolder::TryTailMergeBlocks(...)` removes unconditional branch
instructions and then recreates them. However, this process loses debug
source location information from the previous branch instruction, even
if tail merging doesn't change IR. This patch preserves the debug
information from the removed instruction and inserts them into the
recreated instruction.

Fixes #94050

Added: 
    llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir

Modified: 
    llvm/lib/CodeGen/BranchFolding.cpp
    llvm/lib/CodeGen/BranchFolding.h
    llvm/test/DebugInfo/COFF/local-variables.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 55aa1d438b2a6..c6c48cfc320c9 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -455,12 +455,14 @@ static unsigned EstimateRuntime(MachineBasicBlock::iterator I,
 // with a conditional branch to the next block, optimize by reversing the
 // test and conditionally branching to SuccMBB instead.
 static void FixTail(MachineBasicBlock *CurMBB, MachineBasicBlock *SuccBB,
-                    const TargetInstrInfo *TII) {
+                    const TargetInstrInfo *TII, const DebugLoc &BranchDL) {
   MachineFunction *MF = CurMBB->getParent();
   MachineFunction::iterator I = std::next(MachineFunction::iterator(CurMBB));
   MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
   SmallVector<MachineOperand, 4> Cond;
   DebugLoc dl = CurMBB->findBranchDebugLoc();
+  if (!dl)
+    dl = BranchDL;
   if (I != MF->end() && !TII->analyzeBranch(*CurMBB, TBB, FBB, Cond, true)) {
     MachineBasicBlock *NextBB = &*I;
     if (TBB == NextBB && !Cond.empty() && !FBB) {
@@ -686,7 +688,8 @@ unsigned BranchFolder::ComputeSameTails(unsigned CurHash,
 
 void BranchFolder::RemoveBlocksWithHash(unsigned CurHash,
                                         MachineBasicBlock *SuccBB,
-                                        MachineBasicBlock *PredBB) {
+                                        MachineBasicBlock *PredBB,
+                                        const DebugLoc &BranchDL) {
   MPIterator CurMPIter, B;
   for (CurMPIter = std::prev(MergePotentials.end()),
       B = MergePotentials.begin();
@@ -694,7 +697,7 @@ void BranchFolder::RemoveBlocksWithHash(unsigned CurHash,
     // Put the unconditional branch back, if we need one.
     MachineBasicBlock *CurMBB = CurMPIter->getBlock();
     if (SuccBB && CurMBB != PredBB)
-      FixTail(CurMBB, SuccBB, TII);
+      FixTail(CurMBB, SuccBB, TII, BranchDL);
     if (CurMPIter == B)
       break;
   }
@@ -908,6 +911,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
   // Walk through equivalence sets looking for actual exact matches.
   while (MergePotentials.size() > 1) {
     unsigned CurHash = MergePotentials.back().getHash();
+    const DebugLoc &BranchDL = MergePotentials.back().getBranchDebugLoc();
 
     // Build SameTails, identifying the set of blocks with this hash code
     // and with the maximum number of instructions in common.
@@ -918,7 +922,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
     // If we didn't find any pair that has at least MinCommonTailLength
     // instructions in common, remove all blocks with this hash code and retry.
     if (SameTails.empty()) {
-      RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
+      RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
       continue;
     }
 
@@ -965,7 +969,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
       // Split a block so that one does.
       if (!CreateCommonTailOnlyBlock(PredBB, SuccBB,
                                      maxCommonTailLength, commonTailIndex)) {
-        RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
+        RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
         continue;
       }
     }
@@ -1013,7 +1017,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
     if (MergePotentials.size() == TailMergeThreshold)
       break;
     if (!TriedMerging.count(&MBB) && MBB.succ_empty())
-      MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB));
+      MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB,
+                                                   MBB.findBranchDebugLoc()));
   }
 
   // If this is a large problem, avoid visiting the same basic blocks
@@ -1115,8 +1120,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
         }
 
         // Remove the unconditional branch at the end, if any.
+        DebugLoc dl = PBB->findBranchDebugLoc();
         if (TBB && (Cond.empty() || FBB)) {
-          DebugLoc dl = PBB->findBranchDebugLoc();
           TII->removeBranch(*PBB);
           if (!Cond.empty())
             // reinsert conditional branch only, for now
@@ -1124,7 +1129,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
                               NewCond, dl);
         }
 
-        MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(*PBB), PBB));
+        MergePotentials.push_back(
+            MergePotentialsElt(HashEndOfMBB(*PBB), PBB, dl));
       }
     }
 
@@ -1142,7 +1148,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
     PredBB = &*std::prev(I); // this may have been changed in TryTailMergeBlocks
     if (MergePotentials.size() == 1 &&
         MergePotentials.begin()->getBlock() != PredBB)
-      FixTail(MergePotentials.begin()->getBlock(), IBB, TII);
+      FixTail(MergePotentials.begin()->getBlock(), IBB, TII,
+              MergePotentials.begin()->getBranchDebugLoc());
   }
 
   return MadeChange;

diff  --git a/llvm/lib/CodeGen/BranchFolding.h b/llvm/lib/CodeGen/BranchFolding.h
index 63b2ef04b21ba..ff2bbe06c0488 100644
--- a/llvm/lib/CodeGen/BranchFolding.h
+++ b/llvm/lib/CodeGen/BranchFolding.h
@@ -50,10 +50,11 @@ class TargetRegisterInfo;
     class MergePotentialsElt {
       unsigned Hash;
       MachineBasicBlock *Block;
+      DebugLoc BranchDebugLoc;
 
     public:
-      MergePotentialsElt(unsigned h, MachineBasicBlock *b)
-        : Hash(h), Block(b) {}
+      MergePotentialsElt(unsigned h, MachineBasicBlock *b, DebugLoc bdl)
+          : Hash(h), Block(b), BranchDebugLoc(std::move(bdl)) {}
 
       unsigned getHash() const { return Hash; }
       MachineBasicBlock *getBlock() const { return Block; }
@@ -62,6 +63,8 @@ class TargetRegisterInfo;
         Block = MBB;
       }
 
+      const DebugLoc &getBranchDebugLoc() { return BranchDebugLoc; }
+
       bool operator<(const MergePotentialsElt &) const;
     };
 
@@ -162,8 +165,9 @@ class TargetRegisterInfo;
 
     /// Remove all blocks with hash CurHash from MergePotentials, restoring
     /// branches at ends of blocks as appropriate.
-    void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock* SuccBB,
-                                                MachineBasicBlock* PredBB);
+    void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock *SuccBB,
+                              MachineBasicBlock *PredBB,
+                              const DebugLoc &BranchDL);
 
     /// None of the blocks to be tail-merged consist only of the common tail.
     /// Create a block that does by splitting one.

diff  --git a/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir b/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir
new file mode 100644
index 0000000000000..6c707223d2aa0
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir
@@ -0,0 +1,99 @@
+# RUN: llc -o - %s -mtriple=x86_64-unknown-linux-gnu --run-pass=branch-folder -enable-tail-merge | FileCheck %s
+#
+# Generated with
+#
+# bin/llc -stop-before=branch-folder test.ll
+# 
+# target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+# target triple = "x86_64-grtev4-linux-gnu"
+#
+# define i32 @main(i1 %0) {
+# entry:
+#   br i1 %0, label %1, label %2
+#
+# 1:                                                ; preds = %entry
+#   store i64 1, ptr null, align 1
+#   br label %3, !dbg !3
+#
+# 2:                                                ; preds = %entry
+#   store i64 0, ptr null, align 1
+#   br label %3
+#
+# 3:                                                ; preds = %2, %1
+#   ret i32 0
+# }
+#
+# !llvm.dbg.cu = !{!0}
+# !llvm.module.flags = !{!2}
+# 
+# !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
+# !1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
+# !2 = !{i32 2, !"Debug Info Version", i32 3}
+# !3 = !DILocation(line: 17, column: 3, scope: !4)
+# !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+# !5 = !DISubroutineType(types: !6)
+# !6 = !{}
+--- |
+  
+  ; ModuleID = '../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll'
+  source_filename = "../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-grtev4-linux-gnu"
+
+  define i32 @main(i1 %0) {
+  entry:
+    br i1 %0, label %1, label %2
+
+  1:                                                ; preds = %entry
+    store i64 1, ptr null, align 1
+    br label %3, !dbg !3
+
+  2:                                                ; preds = %entry
+    store i64 0, ptr null, align 1
+    br label %3
+
+  3:                                                ; preds = %2, %1
+    ret i32 0
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
+  !1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
+  !2 = !{i32 2, !"Debug Info Version", i32 3}
+  !3 = !DILocation(line: 17, column: 3, scope: !4)
+  !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{}
+
+...
+---
+name:            main
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x40000000), %bb.2(0x40000000)
+    liveins: $edi
+
+    TEST8ri renamable $dil, 1, implicit-def $eflags, implicit killed $edi
+    JCC_1 %bb.2, 4, implicit killed $eflags
+    JMP_1 %bb.1
+
+  bb.1 (%ir-block.1):
+    successors: %bb.3(0x80000000)
+
+    MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 1 :: (store (s64) into `ptr null`, align 1)
+    JMP_1 %bb.3, debug-location !3
+
+  bb.2 (%ir-block.2):
+    successors: %bb.3(0x80000000)
+
+    MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 0 :: (store (s64) into `ptr null`, align 1)
+
+  bb.3 (%ir-block.3):
+    $eax = MOV32r0 implicit-def dead $eflags
+    RET 0, killed $eax
+
+...
+# CHECK: [[DEBUGNUM:!.*]] = !DILocation(line: 17, column: 3,
+# CHECK: JMP_1 %bb.3, debug-location [[DEBUGNUM]]

diff  --git a/llvm/test/DebugInfo/COFF/local-variables.ll b/llvm/test/DebugInfo/COFF/local-variables.ll
index 56ece6b794b4b..820f6bd8beaa4 100644
--- a/llvm/test/DebugInfo/COFF/local-variables.ll
+++ b/llvm/test/DebugInfo/COFF/local-variables.ll
@@ -45,6 +45,8 @@
 ; ASM:         .cv_loc 1 1 5 3                 # t.cpp:5:3
 ; ASM:         callq   capture
 ; ASM:         leaq    40(%rsp), %rcx
+; ASM: [[end_inline_1:\.Ltmp.*]]:
+; ASM:         .cv_loc 0 1 11 5                # t.cpp:11:5
 ; ASM:         jmp     .LBB0_3
 ; ASM: [[else_start:\.Ltmp.*]]:
 ; ASM: .LBB0_2:                                # %if.else
@@ -87,7 +89,7 @@
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  0                       # Flags
 ; ASM: .asciz  "v"
-; ASM: .cv_def_range    [[inline_site1]] [[else_start]], frame_ptr_rel, 44
+; ASM: .cv_def_range    [[inline_site1]] [[end_inline_1]], frame_ptr_rel, 44
 ; ASM: .short  4430                    # Record kind: S_INLINESITE_END
 ; ASM: .short  4429                    # Record kind: S_INLINESITE
 ; ASM: .short  4414                    # Record kind: S_LOCAL
@@ -154,7 +156,7 @@
 ; OBJ:        ChangeLineOffset: 1
 ; OBJ:        ChangeCodeOffset: 0x14
 ; OBJ:        ChangeCodeOffsetAndLineOffset: {CodeOffset: 0xD, LineOffset: 1}
-; OBJ:        ChangeCodeLength: 0xC
+; OBJ:        ChangeCodeLength: 0xA
 ; OBJ:      ]
 ; OBJ:    }
 ; OBJ:    LocalSym {
@@ -168,7 +170,7 @@
 ; OBJ:      LocalVariableAddrRange {
 ; OBJ:        OffsetStart: .text+0x14
 ; OBJ:        ISectStart: 0x0
-; OBJ:        Range: 0x19
+; OBJ:        Range: 0x17
 ; OBJ:      }
 ; OBJ:    }
 ; OBJ:    InlineSiteEnd {


        


More information about the llvm-commits mailing list