[llvm] [SystemZ] Instrument SystemZ backend passes for Instr-Ref DebugInfo (PR #133061)
Dominik Steenken via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 26 02:57:21 PDT 2025
https://github.com/dominik-steenken updated https://github.com/llvm/llvm-project/pull/133061
>From 61633e114b7a12a87eae593bc220d9456c02cd88 Mon Sep 17 00:00:00 2001
From: Dominik Steenken <dost at de.ibm.com>
Date: Mon, 3 Feb 2025 13:20:31 +0100
Subject: [PATCH] [SystemZ] Instrument SystemZ backend passes for Instr-Ref
DebugInfo
This commit 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 exceptions 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.
---
.../Target/SystemZ/SystemZCopyPhysRegs.cpp | 6 +-
.../lib/Target/SystemZ/SystemZElimCompare.cpp | 9 +++
llvm/lib/Target/SystemZ/SystemZLongBranch.cpp | 13 ++--
.../lib/Target/SystemZ/SystemZPostRewrite.cpp | 33 ++++++----
.../SystemZ/Large/debug-instrref-brct.py | 33 ++++++++++
.../SystemZ/debug-instrref-copyphysregs.mir | 22 +++++++
.../SystemZ/debug-instrref-elimcompare.mir | 65 +++++++++++++++++++
.../SystemZ/debug-instrref-postrewrite.mir | 24 +++++++
8 files changed, 189 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/CodeGen/SystemZ/Large/debug-instrref-brct.py
create mode 100644 llvm/test/CodeGen/SystemZ/debug-instrref-copyphysregs.mir
create mode 100644 llvm/test/CodeGen/SystemZ/debug-instrref-elimcompare.mir
create mode 100644 llvm/test/CodeGen/SystemZ/debug-instrref-postrewrite.mir
diff --git a/llvm/lib/Target/SystemZ/SystemZCopyPhysRegs.cpp b/llvm/lib/Target/SystemZ/SystemZCopyPhysRegs.cpp
index 8979ce4386607..a6cf0f57aaf06 100644
--- 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
index 9f4d4aaa68fa3..789365fb9e311 100644
--- 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
index 36d76235398ed..f19b932f3c731 100644
--- 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
index 4b16bcf95d51c..ffeba87795625 100644
--- 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
new file mode 100644
index 0000000000000..05593a672bf05
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/Large/debug-instrref-brct.py
@@ -0,0 +1,33 @@
+# 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
new file mode 100644
index 0000000000000..ef0c4810731d6
--- /dev/null
+++ 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
new file mode 100644
index 0000000000000..9382b7ad18fca
--- /dev/null
+++ 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
new file mode 100644
index 0000000000000..a0bb2c1b9ed83
--- /dev/null
+++ 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
More information about the llvm-commits
mailing list