[BranchFolding] Kill common hoisted debug instructions

llvm/lib/CodeGen/BranchFolding.cpp

@@ -47,6 +47,8 @@
 #include "llvm/IR/Function.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/MC/LaneBitmask.h"
+#include "llvm/MC/MCInstrDesc.h"
+#include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/BlockFrequency.h"
@@ -2076,19 +2078,54 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
   if (TBB == FBB) {
     MBB->splice(Loc, TBB, TBB->begin(), TIB);
   } else {
+    // Merge the debug locations, and hoist and kill the debug instructions from
+    // both branches. FIXME: We could probably try harder to preserve some debug
+    // instructions (but at least this isn't producing wrong locations).
+    MachineInstrBuilder MIRBuilder(*MBB->getParent(), Loc);
+    auto HoistAndKillDbgInstr = [MBB, Loc](MachineBasicBlock::iterator DI) {
+      assert(DI->isDebugInstr() && "Expected a debug instruction");
+      if (DI->isDebugRef()) {
+        const TargetInstrInfo *TII =
+            MBB->getParent()->getSubtarget().getInstrInfo();
+        const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE);
+        DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0,
+                     DI->getDebugVariable(), DI->getDebugExpression());
+        MBB->insert(Loc, &*DI);
+        return;
+      }
+
+      DI->setDebugValueUndef();
+      DI->moveBefore(&*Loc);
+    };
+
     // TIB and FIB point to the end of the regions to hoist/merge in TBB and
     // FBB.
     MachineBasicBlock::iterator FE = FIB;
     MachineBasicBlock::iterator FI = FBB->begin();
     for (MachineBasicBlock::iterator TI :
          make_early_inc_range(make_range(TBB->begin(), TIB))) {
-      // Move debug instructions and pseudo probes without modifying them.
-      // FIXME: This is the wrong thing to do for debug locations, which
-      // should at least be killed (and hoisted from BOTH blocks).
-      if (TI->isDebugOrPseudoInstr()) {
+
+      // Hoist and kill debug instructions from FBB. Skip over pseudo
+      // instructions. After this loop FI points to the next non-debug
+      // instruction to hoist (checked in assert after the TBB debug
+      // instruction handling code).
+      while (FI->isDebugOrPseudoInstr()) {
+        assert(FI != FE && "Unexpected end of FBB range");
+        MachineBasicBlock::iterator FINext = std::next(FI);
+        HoistAndKillDbgInstr(FI);
+        FI = FINext;
+      }
+
+      // Move pseudo probes without modifying them.
+      if (TI->isPseudoProbe()) {
         TI->moveBefore(&*Loc);
         continue;
       }
+      // Kill debug instructions before moving.
+      if (TI->isDebugInstr()) {
+        HoistAndKillDbgInstr(TI);
+        continue;
+      }
 
       // Get the next non-meta instruction in FBB.
       FI = skipDebugInstructionsForward(FI, FE, false);
@@ -2104,6 +2141,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
       ++FI;
     }
   }
+  assert(FIB->getParent() == FBB && "Unexpectedly moved FIB?");
   FBB->erase(FBB->begin(), FIB);
 
   if (UpdateLiveIns)

llvm/test/DebugInfo/X86/branch-folder-dbg.mir

@@ -1,16 +1,21 @@
 # RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - | FileCheck %s
 
 ## Check that common instructions hoisted from `if.then` and `if.else` into
-## common pred `entry` get merged debug locations.
-
-## FIXME: The debug instructions handling here is wrong.
+## common pred `entry` get merged debug locations. The debug instructions from
+## both branches should get hoisted and killed.
+##
+## The MIR debug instructions have been modified by hand in order to check they
+## can be killed.
 
 # CHECK: bb.0
 # CHECK:      CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
-## --- Start splice from bb.2.if.else ---
-# CHECK-NEXT: DBG_VALUE 2, $noreg, ![[#]], !DIExpression(),  debug-location ![[#]]
-# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags,  debug-location !DILocation(line: 0, scope: ![[#]])
-## --- End splice --------------
+## --- Start splice from bb.2.if.else (and debug instructions from bb.1.if.then) ---
+# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(),  debug-location ![[#]]
+# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(),  debug-location ![[#]]
+# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !DILocation(line: 0, scope: ![[#]])
+# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0),  debug-location ![[#]]
+# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0),  debug-location ![[#]]
+## --- End splice ------------------------------------------------------------------
 # CHECK-NEXT: TEST64rr killed renamable $rax, renamable $rax, implicit-def $eflags
 # CHECK-NEXT: JCC_1 %bb.2, 9, implicit killed $eflags
 # CHECK: bb.1
@@ -73,6 +78,8 @@
 ...
 ---
 name:            g
+tracksRegLiveness: true
+isSSA: false
 body:             |
   bb.0 (%ir-block.0):
     successors: %bb.1(0x40000000), %bb.2(0x40000000)
@@ -86,22 +93,21 @@ body:             |
 
   bb.1.if.then:
     successors: %bb.3(0x80000000)
-
-    DBG_VALUE 0, $noreg, !11, !DIExpression(),  debug-location !13
-    $edi = MOV32r0 implicit-def dead $eflags,  debug-location !14
+    DBG_VALUE $esi, $noreg, !11, !DIExpression(),  debug-location !13
+    $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 1, debug-location !14
+    DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0),  debug-location !13
     $esi = MOV32r0 implicit-def dead $eflags,  debug-location !14
     CALL64pcrel32 target-flags(x86-plt) @_Z3fooii, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit killed $esi, implicit-def $rsp, implicit-def $ssp,  debug-location !14
-    DBG_VALUE 1, $noreg, !11, !DIExpression(),  debug-location !13
     JMP_1 %bb.3,  debug-location !15
 
   bb.2.if.else:
     successors: %bb.3(0x80000000)
 
-    DBG_VALUE 2, $noreg, !11, !DIExpression(),  debug-location !13
-    $edi = MOV32r0 implicit-def dead $eflags,  debug-location !16
+    DBG_VALUE $esp, $noreg, !11, !DIExpression(), debug-location !13
+    $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !16
+    DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(2, 0),  debug-location !13
     $esi = MOV32ri 1,  debug-location !16
     CALL64pcrel32 target-flags(x86-plt) @_Z3barii, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit killed $esi, implicit-def $rsp, implicit-def $ssp,  debug-location !16
-    DBG_VALUE 3, $noreg, !11, !DIExpression(),  debug-location !13
 
   bb.3.if.end:
     $eax = MOV32ri 2