[SystemZ, DebugInfo] Instrument SystemZ backend passes for Instr-Ref DebugInfo #133061
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-systemz Author: Dominik Steenken (dominik-steenken) ChangesThis PR instruments the optimization passes in the SystemZ backend with calls to Tests are also added for each of the substitutions that were inserted. Details on the individual passes follow. systemz-copy-physregsWhen 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-branchThis 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 systemz-post-rewriteThis pass replaces systemz-elim-compareSimilar 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. Full diff: https://github.com/llvm/llvm-project/pull/133061.diff 8 Files Affected:
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
|
dominik-steenken
force-pushed
the
debug-instruction-tracking
branch
from
5f27d3e to
61633e1
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
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.
dominik-steenken
force-pushed
the
debug-instruction-tracking
branch
from
61633e1 to
e6e0fb3
Compare
dominik-steenken
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR instruments the optimization passes in the SystemZ backend with calls to
MachineFunction::substituteDebugValuesForInstwhere 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-numberpresent must move to the add-immediate instruction.systemz-post-rewrite
This pass replaces
LOCRMuxandSELRMuxpseudoinstructions with either the real versions of those instructions, or with branching programs that implement the intent of the Pseudo. In all these cases, anydebug-instr-numberattached 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.