From 836703087d761f9cbf81b6f9593bc5313660f559 Mon Sep 17 00:00:00 2001 From: Alan Zhao Date: Thu, 20 Jun 2024 10:48:18 -0700 Subject: [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 --- llvm/lib/CodeGen/BranchFolding.cpp | 25 ++++-- llvm/lib/CodeGen/BranchFolding.h | 12 ++- .../MIR/X86/tail-merging-preserve-debugloc.mir | 99 ++++++++++++++++++++++ llvm/test/DebugInfo/COFF/local-variables.ll | 8 +- 4 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 55aa1d4..c6c48cf 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 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 63b2ef0..ff2bbe0 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 0000000..6c70722 --- /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 56ece6b..820f6bd 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 { -- cgit v1.1