[SystemZ, DebugInfo] Instrument SystemZ backend passes for Instr-Ref DebugInfo (#133061)

dominik-steenken · web-flow · commit e9a3ea2218b7 · 2025-03-31T19:30:06.000+02:00
This PR instruments the optimization passes in the SystemZ backend with
calls to `MachineFunction::substituteDebugValuesForInst` where
instruction substitutions are made to instructions that may compute
tracked values.

Tests are also added for each of the substitutions that were inserted.
Details on the individual passes follow.

### systemz-copy-physregs
When a copy targets an access register, we redirect the copy via an
auxiliary register. This leads to the final result being written by a
newly inserted SAR instruction, rather than the original MI, so we need
to update the debug value tracking to account for this.

### systemz-long-branch
This pass relaxes relative branch instructions based on the actual
locations of blocks. Only one of the branch instructions qualifies for
debug value tracking: BRCT, i.e. branch-relative-on-count, which
subtracts 1 from a register and branches if the result is not zero. This
is relaxed into an add-immediate and a conditional branch, so any
`debug-instr-number` present must move to the add-immediate instruction.

### systemz-post-rewrite
This pass replaces `LOCRMux` and `SELRMux` pseudoinstructions with
either the real versions of those instructions, or with branching
programs that implement the intent of the Pseudo. In all these cases,
any `debug-instr-number` attached to the pseudo needs to be reallocated
to the appropriate instruction in the result, either LOCR, SELR, or a
COPY.

### systemz-elim-compare
Similar to systemz-long-branch, for this pass, only few substitutions
are necessary, since it mainly deals with conditional branch
instructions. The only exceptiona are again branch-relative-on-count, as
it modifies a counter as part of the instruction, as well as any of the
load instructions that are affected.
diff --git a/llvm/lib/Target/SystemZ/SystemZCopyPhysRegs.cpp b/llvm/lib/Target/SystemZ/SystemZCopyPhysRegs.cpp
@@ -75,6 +75,7 @@ bool SystemZCopyPhysRegs::visitMBB(MachineBasicBlock &MBB) {
     DebugLoc DL = MI->getDebugLoc();
     Register SrcReg = MI->getOperand(1).getReg();
     Register DstReg = MI->getOperand(0).getReg();
+
     if (DstReg.isVirtual() &&
         (SrcReg == SystemZ::CC || SystemZ::AR32BitRegClass.contains(SrcReg))) {
       Register Tmp = MRI->createVirtualRegister(&SystemZ::GR32BitRegClass);
@@ -89,7 +90,10 @@ bool SystemZCopyPhysRegs::visitMBB(MachineBasicBlock &MBB) {
              SystemZ::AR32BitRegClass.contains(DstReg)) {
       Register Tmp = MRI->createVirtualRegister(&SystemZ::GR32BitRegClass);
       MI->getOperand(0).setReg(Tmp);
-      BuildMI(MBB, MBBI, DL, TII->get(SystemZ::SAR), DstReg).addReg(Tmp);
+      MachineInstr *NMI =
+          BuildMI(MBB, MBBI, DL, TII->get(SystemZ::SAR), DstReg).addReg(Tmp);
+      // SAR now writes the final value to DstReg, so update debug values.
+      MBB.getParent()->substituteDebugValuesForInst(*MI, *NMI);
       Modified = true;
     }
   }
diff --git a/llvm/lib/Target/SystemZ/SystemZElimCompare.cpp b/llvm/lib/Target/SystemZ/SystemZElimCompare.cpp
@@ -227,6 +227,9 @@ bool SystemZElimCompare::convertToBRCT(
   // this is not necessary there.
   if (BRCT != SystemZ::BRCTH)
     MIB.addReg(SystemZ::CC, RegState::ImplicitDefine | RegState::Dead);
+  // The debug instr tracking for the counter now used by BRCT needs to be
+  // updated.
+  MI.getParent()->getParent()->substituteDebugValuesForInst(MI, *MIB);
   MI.eraseFromParent();
   return true;
 }
@@ -268,6 +271,9 @@ bool SystemZElimCompare::convertToLoadAndTrap(
       .add(MI.getOperand(1))
       .add(MI.getOperand(2))
       .add(MI.getOperand(3));
+  // The debug instr tracking for the load target now used by the load-and-trap
+  // needs to be updated.
+  MI.getParent()->getParent()->substituteDebugValuesForInst(MI, *Branch);
   MI.eraseFromParent();
   return true;
 }
@@ -288,6 +294,9 @@ bool SystemZElimCompare::convertToLoadAndTest(
   for (const auto &MO : MI.operands())
     MIB.add(MO);
   MIB.setMemRefs(MI.memoperands());
+  // The debug instr tracking for the load target now needs to be updated
+  // because the load has moved to a new instruction
+  MI.getParent()->getParent()->substituteDebugValuesForInst(MI, *MIB);
   MI.eraseFromParent();
 
   // Mark instruction as not raising an FP exception if applicable.  We already
diff --git a/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp b/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp
@@ -374,16 +374,19 @@ void SystemZLongBranch::splitBranchOnCount(MachineInstr *MI,
                                            unsigned AddOpcode) {
   MachineBasicBlock *MBB = MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
-  BuildMI(*MBB, MI, DL, TII->get(AddOpcode))
-      .add(MI->getOperand(0))
-      .add(MI->getOperand(1))
-      .addImm(-1);
+  MachineInstr *AddImm = BuildMI(*MBB, MI, DL, TII->get(AddOpcode))
+                             .add(MI->getOperand(0))
+                             .add(MI->getOperand(1))
+                             .addImm(-1);
   MachineInstr *BRCL = BuildMI(*MBB, MI, DL, TII->get(SystemZ::BRCL))
                            .addImm(SystemZ::CCMASK_ICMP)
                            .addImm(SystemZ::CCMASK_CMP_NE)
                            .add(MI->getOperand(2));
   // The implicit use of CC is a killing use.
   BRCL->addRegisterKilled(SystemZ::CC, &TII->getRegisterInfo());
+  // The result of the BRANCH ON COUNT MI is the new count in register 0, so the
+  // debug tracking needs to go to the result of the Add immediate.
+  MBB->getParent()->substituteDebugValuesForInst(*MI, *AddImm);
   MI->eraseFromParent();
 }
 
@@ -402,6 +405,8 @@ void SystemZLongBranch::splitCompareBranch(MachineInstr *MI,
                            .add(MI->getOperand(3));
   // The implicit use of CC is a killing use.
   BRCL->addRegisterKilled(SystemZ::CC, &TII->getRegisterInfo());
+  // Since we are replacing branches that did not compute any value, no debug
+  // value substitution is necessary.
   MI->eraseFromParent();
 }
 
diff --git a/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp b/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 using namespace llvm;
 
@@ -108,15 +109,19 @@ void SystemZPostRewrite::selectSELRMux(MachineBasicBlock &MBB,
   bool DestIsHigh = SystemZ::isHighReg(DestReg);
   bool Src1IsHigh = SystemZ::isHighReg(Src1Reg);
   bool Src2IsHigh = SystemZ::isHighReg(Src2Reg);
+  // A copy instruction that we might create, held here for the purpose of
+  // debug instr value tracking.
+  MachineInstr *CopyInst = nullptr;
 
   // In rare cases both sources are the same register (after
   // machine-cse). This must be handled as it may lead to wrong-code (after
   // machine-cp) if the kill flag on Src1 isn't cleared (with
   // expandCondMove()).
   if (Src1Reg == Src2Reg) {
-    BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
-            TII->get(SystemZ::COPY), DestReg)
-        .addReg(Src1Reg, getRegState(Src1MO) & getRegState(Src2MO));
+    CopyInst = BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+                       TII->get(SystemZ::COPY), DestReg)
+                   .addReg(Src1Reg, getRegState(Src1MO) & getRegState(Src2MO));
+    MBB.getParent()->substituteDebugValuesForInst(*MBBI, *CopyInst, 1);
     MBBI->eraseFromParent();
     return;
   }
@@ -126,21 +131,24 @@ void SystemZPostRewrite::selectSELRMux(MachineBasicBlock &MBB,
   // first.  But only if this doesn't clobber the other source.
   if (DestReg != Src1Reg && DestReg != Src2Reg) {
     if (DestIsHigh != Src1IsHigh) {
-      BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
-              TII->get(SystemZ::COPY), DestReg)
-          .addReg(Src1Reg, getRegState(Src1MO));
+      CopyInst = BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+                         TII->get(SystemZ::COPY), DestReg)
+                     .addReg(Src1Reg, getRegState(Src1MO));
       Src1MO.setReg(DestReg);
       Src1Reg = DestReg;
       Src1IsHigh = DestIsHigh;
     } else if (DestIsHigh != Src2IsHigh) {
-      BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
-              TII->get(SystemZ::COPY), DestReg)
-          .addReg(Src2Reg, getRegState(Src2MO));
+      CopyInst = BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+                         TII->get(SystemZ::COPY), DestReg)
+                     .addReg(Src2Reg, getRegState(Src2MO));
       Src2MO.setReg(DestReg);
       Src2Reg = DestReg;
       Src2IsHigh = DestIsHigh;
     }
   }
+  // if a copy instruction was inserted, record the debug value substitution
+  if (CopyInst)
+    MBB.getParent()->substituteDebugValuesForInst(*MBBI, *CopyInst, 1);
 
   // If the destination (now) matches one source, prefer this to be first.
   if (DestReg != Src1Reg && DestReg == Src2Reg) {
@@ -204,8 +212,11 @@ bool SystemZPostRewrite::expandCondMove(MachineBasicBlock &MBB,
 
   // In MoveMBB, emit an instruction to move SrcReg into DestReg,
   // then fall through to RestMBB.
-  BuildMI(*MoveMBB, MoveMBB->end(), DL, TII->get(SystemZ::COPY), DestReg)
-      .addReg(MI.getOperand(2).getReg(), getRegState(MI.getOperand(2)));
+  MachineInstr *CopyInst =
+      BuildMI(*MoveMBB, MoveMBB->end(), DL, TII->get(SystemZ::COPY), DestReg)
+          .addReg(MI.getOperand(2).getReg(), getRegState(MI.getOperand(2)));
+  // record the debug value substitution for CopyInst
+  MBB.getParent()->substituteDebugValuesForInst(*MBBI, *CopyInst, 1);
   MoveMBB->addSuccessor(RestMBB);
 
   NextMBBI = MBB.end();
diff --git a/llvm/test/CodeGen/SystemZ/Large/debug-instrref-brct.py b/llvm/test/CodeGen/SystemZ/Large/debug-instrref-brct.py
@@ -0,0 +1,32 @@
+# RUN: %python %s | llc -mtriple=s390x-linux-gnu -x mir --run-pass=systemz-long-branch \
+# RUN:    | FileCheck %s
+
+# CHECK: debugValueSubstitutions:
+# CHECK:   - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+# CHECK:   - { srcinst: 1, srcop: 3, dstinst: 3, dstop: 3, subreg: 0 }
+# CHECK-NEXT: constants:       []
+# CHECK: $r3l = AHI $r3l, -1
+# CHECK-NEXT: BRCL 14, 6, %bb.2
+print(" name: main")
+print(" alignment: 16")
+print(" tracksRegLiveness: true")
+print(" liveins: ")
+print("   - { reg: '$r1d', virtual-reg: '' }")
+print("   - { reg: '$r2d', virtual-reg: '' }")
+print("   - { reg: '$r3l', virtual-reg: '' }")
+print("   - { reg: '$r4l', virtual-reg: '' }")
+print(" debugValueSubstitutions: []")
+print(" body:            |")
+print("   bb.0:")
+print("     liveins: $r3l, $r4l, $r2d, $r3d")
+print("     $r3l = BRCT $r3l, %bb.2, implicit-def $cc, debug-instr-number 1")
+print("     J %bb.1, debug-instr-number 2")
+print("   bb.1:")
+print("     liveins: $r1d, $r2d")
+for i in range(0, 8192):
+    print("     $r1d = LGR $r2d")
+    print("     $r2d = LGR $r1d")
+print("     Return implicit $r2d")
+print("   bb.2:")
+print("     liveins: $r4l")
+print("     Return implicit $r4l")
diff --git a/llvm/test/CodeGen/SystemZ/debug-instrref-copyphysregs.mir b/llvm/test/CodeGen/SystemZ/debug-instrref-copyphysregs.mir
@@ -0,0 +1,22 @@
+# Check that the backend properly tracks debug-instr-references across the
+# copy-physregs pass.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -run-pass=systemz-copy-physregs \
+# RUN:   -o - 2>&1 | FileCheck %s
+
+# COPY 1: Copy VirtReg to AR
+# COPY 2: Copy AR to VirtReg
+# COPY 3: Copy CC to VirtReg
+# CHECK: name:            foo
+# CHECK: debugValueSubstitutions:
+# these are the correct substitutions
+# CHECK-NEXT:  - { srcinst: 1, srcop: 0, dstinst: 4, dstop: 0, subreg: 0 }
+# we also need to make sure that these are the only substitutions
+# CHECK-NEXT: constants:       []
+name: foo
+body:               |
+  bb.0:
+    liveins: $a1
+    COPY def $a1, %1:gr32bit, debug-instr-number 1
+    COPY def %2:gr32bit, $a1, debug-instr-number 2
+    COPY def %3:gr32bit, $cc, debug-instr-number 3
diff --git a/llvm/test/CodeGen/SystemZ/debug-instrref-elimcompare.mir b/llvm/test/CodeGen/SystemZ/debug-instrref-elimcompare.mir
@@ -0,0 +1,65 @@
+# Check that the backend properly tracks debug-instr-references across the
+# elim-compare pass.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z14 -run-pass=systemz-elim-compare \
+# RUN:   -o - 2>&1 | FileCheck %s
+
+# bb.0 - elimination of CHI, modification of BRC, no substitutions
+# bb.1 - elimination of CHI, replacement of LR with LTR, one substitution
+# bb.2 - elimination of L and CHI, modification of CondTrap into LAT, one substitution
+# CHECK: name:            foo
+# CHECK: debugValueSubstitutions:
+# these are the correct substitutions
+# CHECK-NEXT:  - { srcinst: 5, srcop: 0, dstinst: 13, dstop: 0, subreg: 0 }
+# CHECK-NEXT:  - { srcinst: 7, srcop: 0, dstinst: 9, dstop: 0, subreg: 0 }
+# CHECK-NEXT:  - { srcinst: 10, srcop: 0, dstinst: 14, dstop: 0, subreg: 0 }
+# we also need to make sure that these are the only substitutions
+# CHECK-NEXT: constants:       []
+---
+name:            foo
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r2l', virtual-reg: '' }
+  - { reg: '$r3l', virtual-reg: '' }
+  - { reg: '$r4l', virtual-reg: '' }
+  - { reg: '$r5d', virtual-reg: '' }
+debugValueSubstitutions: []
+body:             |
+  bb.0:
+    successors: %bb.1(0x80000000)
+    liveins: $r2l, $r3l, $r4l, $r5d
+  
+    renamable $r3l = nsw AR killed renamable $r3l, renamable $r2l, implicit-def dead $cc, debug-instr-number 1
+    CHI renamable $r3l, 0, implicit-def $cc, debug-instr-number 2
+    BRC 14, 12, %bb.1, implicit $cc, debug-instr-number 3
+
+  bb.1:
+    successors: %bb.2(0x80000000)
+    liveins: $r2l, $r3l, $r4l, $r5d
+    
+    CHI renamable $r2l, 0, implicit-def $cc, debug-instr-number 4
+    renamable $r3l = LR renamable $r2l, debug-instr-number 5
+    BRC 14, 8, %bb.2, implicit killed $cc, debug-instr-number 6
+
+  bb.2:
+    successors: %bb.3(0x80000000)
+    liveins: $r2l, $r3l, $r4l, $r5d
+
+    renamable $r2l = L killed renamable $r5d, 0, $noreg, debug-instr-number 7
+    CHI renamable $r2l, 0, implicit-def $cc, debug-instr-number 8
+    CondTrap 14, 8, implicit killed $cc, debug-instr-number 9
+    J %bb.3
+
+  bb.3:
+    successors: %bb.4(080000000)
+    liveins: $r2l, $r3l, $r4l, $r5d
+
+    renamable $r3l = L renamable $r5d, 0, $noreg, debug-instr-number 10
+    CHI renamable $r3l, 0, implicit-def $cc, debug-instr-number 11
+    BRC 14, 8, %bb.4, implicit killed $cc, debug-instr-number 12
+  
+  bb.4:
+    $r2l = LHI 2
+    Return implicit $r2l
+
+...
diff --git a/llvm/test/CodeGen/SystemZ/debug-instrref-postrewrite.mir b/llvm/test/CodeGen/SystemZ/debug-instrref-postrewrite.mir
@@ -0,0 +1,24 @@
+# Check that the backend properly tracks debug-instr-references across the
+# post-rewrite pass.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -run-pass=systemz-post-rewrite \
+# RUN:   -o - 2>&1 | FileCheck %s
+
+# SELRMux 1: simple replace with copy
+# SELRMux 2: simple mutation into selfhr
+# SELRMux 3: replace with if-then-else without prior copy
+# SELRMux 4: replace with if-then-else with prior copy
+# CHECK: name:            foo
+# CHECK: debugValueSubstitutions:
+# CHECK-NEXT:  - { srcinst: 1, srcop: 0, dstinst: 5, dstop: 0, subreg: 0 }
+# CHECK-NEXT:  - { srcinst: 3, srcop: 0, dstinst: 6, dstop: 0, subreg: 0 }
+# CHECK-NEXT:  - { srcinst: 4, srcop: 0, dstinst: 7, dstop: 0, subreg: 0 }
+# CHECK-NEXT:  - { srcinst: 4, srcop: 0, dstinst: 8, dstop: 0, subreg: 0 }
+name: foo
+body:               |
+  bb.0:
+    liveins: $r2h, $r3h, $r2l, $r3l, $cc
+    SELRMux def $r2h, renamable $r3l, renamable $r3l, 1, 2, implicit $cc, debug-instr-number 1
+    SELRMux def $r1h, renamable $r2h, renamable $r3h, 1, 2, implicit $cc, debug-instr-number 2
+    SELRMux def $r2h, renamable $r2h, renamable $r3l, 1, 2, implicit $cc, debug-instr-number 3
+    SELRMux def $r1h, renamable $r2l, renamable $r3l, 1, 2, implicit $cc, debug-instr-number 4
