[DebugInfo][InstrRef] Avoid a debug-info-affects-codegen scenario in MCP

[jmorse]

Summary

The machine-copy-propagation pass collects all debug uses of instructions and then tries to propagate copies through them -- this includes DBG_PHIs, which can refer to COPYs in rare circumstances where tail duplication simplifies control flow. Such DBG_PHIs should also have copies propagated through them: update the helper utility to do so. I've added a DBG_PHI to an ARM test that tests this behaviour, i.e. that copies get propagated in debug instructions.

In addition, this patch also correctly sets the "debug-use" flag on DBG_PHIs when they're read from MIR files, which was previously being omitted. This is covered by the test, as copy propagation won't happen if there's a non-debug-use of the $r4.

Finally, call setIsDebug when DBG_PHIs are created in LiveDebugVariables. I, uh, don't know of a way to test for this, because "debug-use" doesn't seem to be printed by the MIR printer. Ouch.

[djtodoro]

Thanks, looks good to me!

Finally, call setIsDebug when DBG_PHIs are created in LiveDebugVariables. I, uh, don't know of a way to test for this, because "debug-use" doesn't seem to be printed by the MIR printer. Ouch.

unittest ?

[StephenTozer]

llvm/include/llvm/CodeGen/MachineRegisterInfo.h

847

[jmorse]

Rebase, fix comment, remove setIsDebug now that a different approach is used for that after D110105.

On the unit test front, I think D110105 avoids this problem now, any register operand added to a debug instruction will now get the isDebug flag set on it.

llvm/include/llvm/CodeGen/MachineRegisterInfo.h

&getRegAllocationHints(Register VReg) const {
     return RegAllocHints[VReg];
   }
 
   /// markUsesInDebugValueAsUndef - Mark every DBG_VALUE referencing the
   /// specified register as undefined which causes the DBG_VALUE to be
   /// deleted during LiveDebugVariables analysis.
   void markUsesInDebugValueAsUndef(Register Reg) const;
 
-  /// updateDbgUsersToReg - Update a collection of DBG_VALUE instructions
+  /// updateDbgUsersToReg - Update a collection of debug instructions
   /// to refer to the designated register.
   void updateDbgUsersToReg(MCRegister OldReg, MCRegister NewReg,
                            ArrayRef<MachineInstr *> Users) const {
     SmallSet<MCRegister, 4> OldRegUnits;
     for (MCRegUnitIterator RUI(OldReg, getTargetRegisterInfo()); RUI.isValid();
          ++RUI)
       OldRegUnits.insert(*RUI);
-    for (MachineInstr *MI : Users) {
-      assert(MI->isDebugValue());
-      for (auto &Op : MI->debug_operands()) {
-        if (Op.isReg()) {
-          for (MCRegUnitIterator RUI(OldReg, getTargetRegisterInfo());
-               RUI.isValid(); ++RUI) {
-            if (OldRegUnits.contains(*RUI)) {
-              Op.setReg(NewReg);
-              break;
-            }
-          }
-        }
-      }
+
+    // If this operand is a register, check whether it overlaps with OldReg.
+    // If it does, replace with NewReg.
+    auto UpdateOp = [this, &NewReg, &OldReg, &OldRegUnits](MachineOperand &Op) {
+    };
+
+    // Iterate through (possibly several) operands to DBG_VALUEs and update
+    // each. For DBG_PHIs, only one operand will be present.
+    for (MachineInstr *MI : Users) {
+      if (MI->isDebugValue()) {
+        for (auto &Op : MI->debug_operands())
+          UpdateOp(Op);
         assert(MI->hasDebugOperandForReg(NewReg) &&
                "Expected debug value to have some overlap with OldReg");
+      } else if (MI->isDebugPHI()) {
+        UpdateOp(MI->getOperand(0));
+      } else {
+        llvm_unreachable("Non-DBG_VALUE, Non-DBG_PHI debug instr updated");
+      }
     }
   }
 
   /// Return true if the specified register is modified in this function.
   /// This checks that no defining machine operands exist for the register or
   /// any of its aliases. Definitions found on functions marked noreturn are
   /// ignored, to consider them pass 'true' for optional parameter
   /// SkipNoReturnDef. The register is also considered modified when it is set

llvm/test/DebugInfo/ARM/machine-cp-updates-dbg-reg.mir

 # RUN: llc %s -run-pass=machine-cp -o - | FileCheck %s --implicit-check-not=DBG_VALUE
 
 ## Ensure that when the destination register of a copy instruction is used by a
 ## DBG_VALUE/DBG_VALUE_LIST, and then that instruction is deleted during copy
 ## propagation, the debug use is updated to the source register.
+##
+## Do the same for DBG_PHI instructions.
 
 # CHECK: ![[VAR_V:[0-9]+]] = !DILocalVariable(name: "v"
 # CHECK-LABEL: body:
 
+# CHECK: DBG_PHI $r0, 1
 # CHECK: DBG_VALUE $r0, $noreg, ![[VAR_V]], !DIExpression()
 # CHECK: DBG_VALUE_LIST ![[VAR_V]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 4, DW_OP_mul, DW_OP_plus, DW_OP_stack_value), $r0, 0
 
 --- |
   target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
   target triple = "arm-unknown-linux-gnueabi"
 
   %struct.atomic_t.0.6.15.24.48.51.54.57 = type { i32 }
bb.1.if.end:
     CMPri renamable $r4, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
     Bcc %bb.3, 1 /* CC::ne */, killed $cpsr
 
   bb.2.if.end3:
     liveins: $r0
 
     dead renamable $r1 = LDRi12 undef renamable $r0, 0, 14 /* CC::al */, $noreg :: (volatile load (s32) from `i32* undef`)
     renamable $r4 = COPY killed renamable $r0
+    DBG_PHI $r4, 1
     DBG_VALUE $r4, $noreg, !30, !DIExpression(), debug-location !40
     DBG_VALUE_LIST !30, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 4, DW_OP_mul, DW_OP_plus, DW_OP_stack_value), $r4, 0, debug-location !40
     $r0 = COPY killed renamable $r4
     $sp = frame-destroy LDMIA_RET $sp, 14 /* CC::al */, $noreg, def $r4, def $pc, implicit $r0
 
   bb.3.free_pt:
     successors: %bb.4(0x80000000)
     liveins: $r4