Skip to content

Commit dc4ff43

Browse files
JonPsson1JonPsson1
committed
Updates per review.
1 parent 138859d commit dc4ff43

File tree

2 files changed

+36
-46
lines changed

2 files changed

+36
-46
lines changed

‎llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp‎

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,25 +39,24 @@ namespace {
3939
class MachineLateInstrsCleanup : public MachineFunctionPass {
4040
const TargetRegisterInfo *TRI = nullptr;
4141
const TargetInstrInfo *TII = nullptr;
42+
const MachineRegisterInfo *MRI = nullptr;
4243

43-
// Data structures to map regs to their definitions and kills per MBB.
44+
// Data structure to map regs to their definitions per MBB.
4445
struct Reg2MIMap : public SmallDenseMap<Register, MachineInstr *> {
4546
bool hasIdentical(Register Reg, MachineInstr *ArgMI) {
4647
MachineInstr *MI = lookup(Reg);
4748
return MI && MI->isIdenticalTo(*ArgMI);
4849
}
4950
};
50-
typedef SmallDenseMap<Register, TinyPtrVector<MachineInstr *>> Reg2MIVecMap;
5151
std::vector<Reg2MIMap> RegDefs;
52-
std::vector<Reg2MIVecMap> RegKills;
5352

5453
// Walk through the instructions in MBB and remove any redundant
5554
// instructions.
5655
bool processBlock(MachineBasicBlock *MBB);
5756

5857
void removeRedundantDef(MachineInstr *MI);
59-
void clearKillsForDef(Register Reg, MachineBasicBlock *MBB,
60-
BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
58+
void updateLiveInLists(Register Reg, MachineBasicBlock *MBB,
59+
BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
6160

6261
public:
6362
static char ID; // Pass identification, replacement for typeid
@@ -94,11 +93,10 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
9493

9594
TRI = MF.getSubtarget().getRegisterInfo();
9695
TII = MF.getSubtarget().getInstrInfo();
96+
MRI = &MF.getRegInfo();
9797

9898
RegDefs.clear();
9999
RegDefs.resize(MF.getNumBlockIDs());
100-
RegKills.clear();
101-
RegKills.resize(MF.getNumBlockIDs());
102100

103101
// Visit all MBBs in an order that maximises the reuse from predecessors.
104102
bool Changed = false;
@@ -111,41 +109,36 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
111109

112110
// Clear any preceding kill flag on Reg after removing a redundant
113111
// definition.
114-
void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
112+
void MachineLateInstrsCleanup::updateLiveInLists(Register Reg,
115113
MachineBasicBlock *MBB,
116114
BitVector &VisitedPreds,
117115
MachineInstr *ToRemoveMI) {
118116
VisitedPreds.set(MBB->getNumber());
119117

120-
// Clear kill flag(s) in MBB, that have been seen after the preceding
121-
// definition. If Reg or one of its subregs was killed, it would actually
122-
// be ok to stop after removing that (and any other) kill-flag, but it
123-
// doesn't seem noticeably faster while it would be a bit more complicated.
124-
Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
125-
if (MBBKills.contains(Reg))
126-
for (auto *KillMI : MBBKills[Reg])
127-
KillMI->clearRegisterKills(Reg, TRI);
128-
129118
// Definition in current MBB: done.
130119
Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
131120
MachineInstr *DefMI = MBBDefs[Reg];
132121
assert(DefMI->isIdenticalTo(*ToRemoveMI) && "Previous def not identical?");
133122
if (DefMI->getParent() == MBB)
134123
return;
135124

136-
// If an earlier def is not in MBB, continue in predecessors.
125+
// If the earlier def is not in MBB, it has now become live in. Continue in
126+
// predecessors until the defining MBB has been reached.
137127
if (!MBB->isLiveIn(Reg))
138128
MBB->addLiveIn(Reg);
139129
assert(!MBB->pred_empty() && "Predecessor def not found!");
140130
for (MachineBasicBlock *Pred : MBB->predecessors())
141131
if (!VisitedPreds.test(Pred->getNumber()))
142-
clearKillsForDef(Reg, Pred, VisitedPreds, ToRemoveMI);
132+
updateLiveInLists(Reg, Pred, VisitedPreds, ToRemoveMI);
143133
}
144134

145135
void MachineLateInstrsCleanup::removeRedundantDef(MachineInstr *MI) {
146136
Register Reg = MI->getOperand(0).getReg();
137+
// Clear any and all kill flags.
138+
for (MCPhysReg SReg : TRI->superregs_inclusive(Reg))
139+
MRI->clearKillFlags(SReg);
147140
BitVector VisitedPreds(MI->getMF()->getNumBlockIDs());
148-
clearKillsForDef(Reg, MI->getParent(), VisitedPreds, MI);
141+
updateLiveInLists(Reg, MI->getParent(), VisitedPreds, MI);
149142
MI->eraseFromParent();
150143
++NumRemoved;
151144
}
@@ -181,7 +174,6 @@ static bool isCandidate(const MachineInstr *MI, Register &DefedReg,
181174
bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
182175
bool Changed = false;
183176
Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
184-
Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
185177

186178
// Find reusable definitions in the predecessor(s).
187179
if (!MBB->pred_empty() && !MBB->isEHPad() &&
@@ -208,7 +200,6 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
208200
// it) are valid.
209201
if (MI.modifiesRegister(FrameReg, TRI)) {
210202
MBBDefs.clear();
211-
MBBKills.clear();
212203
continue;
213204
}
214205

@@ -227,20 +218,15 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
227218
// Clear any entries in map that MI clobbers.
228219
for (auto DefI : llvm::make_early_inc_range(MBBDefs)) {
229220
Register Reg = DefI.first;
230-
if (MI.modifiesRegister(Reg, TRI)) {
221+
if (MI.modifiesRegister(Reg, TRI))
231222
MBBDefs.erase(Reg);
232-
MBBKills.erase(Reg);
233-
} else if (MI.findRegisterUseOperandIdx(Reg, TRI, true /*isKill*/) != -1)
234-
// Keep track of all instructions that fully or partially kills Reg.
235-
MBBKills[Reg].push_back(&MI);
236223
}
237224

238225
// Record this MI for potential later reuse.
239226
if (IsCandidate) {
240227
LLVM_DEBUG(dbgs() << "Found interesting instruction in "
241228
<< printMBBReference(*MBB) << ": " << MI;);
242229
MBBDefs[DefedReg] = &MI;
243-
assert(!MBBKills.count(DefedReg) && "Should already have been removed.");
244230
}
245231
}
246232

‎llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,26 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
12
# RUN: llc -mtriple=s390x-linux-gnu -run-pass=machine-latecleanup %s -o - -mcpu=z16 \
23
# RUN: -verify-machineinstrs 2>&1 | FileCheck %s
34

45
# Kill flag of $r0q (super-reg) needs to be removed, and $r0l needs to be added as live-in.
5-
# CHECK-LABEL: name: fun0
6-
# CHECK-LABEL: bb.0:
7-
# CHECK: renamable $r0l = LHIMux -1
8-
# CHECK-NEXT: J %bb.1
9-
# CHECK-LABEL: bb.1:
10-
# CHECK-NEXT: liveins: $r0l
11-
# CHECK: renamable $r1d = LGFI 0
12-
# CHECK-NEXT: ST128 renamable $r0q, $r15d, 168, $noreg
13-
# CHECK-NEXT: ST killed renamable $r0l, $r15d, 160, $noreg
14-
# CHECK-NEXT: Return
156
---
167
name: fun0
178
tracksRegLiveness: true
189
body: |
10+
; CHECK-LABEL: name: fun0
11+
; CHECK: bb.0:
12+
; CHECK-NEXT: successors: %bb.1(0x80000000)
13+
; CHECK-NEXT: {{ $}}
14+
; CHECK-NEXT: renamable $r0l = LHIMux -1
15+
; CHECK-NEXT: J %bb.1
16+
; CHECK-NEXT: {{ $}}
17+
; CHECK-NEXT: bb.1:
18+
; CHECK-NEXT: liveins: $r0l
19+
; CHECK-NEXT: {{ $}}
20+
; CHECK-NEXT: renamable $r1d = LGFI 0
21+
; CHECK-NEXT: ST128 renamable $r0q, $r15d, 168, $noreg
22+
; CHECK-NEXT: ST renamable $r0l, $r15d, 160, $noreg
23+
; CHECK-NEXT: Return
1924
bb.0:
2025
renamable $r0l = LHIMux -1
2126
J %bb.1
@@ -29,19 +34,18 @@ body: |
2934
...
3035

3136
# Kill flags of both $r1d and $r0q (super-reg) need to be removed.
32-
# CHECK-LABEL: name: fun1
33-
# CHECK-LABEL: bb.0:
34-
# CHECK-NEXT: renamable $r1d = LLILL 1
35-
# CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
36-
# CHECK-NEXT: renamable $r0d = LLILL 0
37-
# CHECK-NEXT: ST128 renamable $r0q, $r15d, 0, $noreg
38-
# CHECK-NEXT: STG killed renamable $r1d, killed $r15d, 8, $noreg
39-
# CHECK-NEXT: Return
4037
---
4138
name: fun1
4239
tracksRegLiveness: true
4340
body: |
4441
bb.0:
42+
; CHECK-LABEL: name: fun1
43+
; CHECK: renamable $r1d = LLILL 1
44+
; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
45+
; CHECK-NEXT: renamable $r0d = LLILL 0
46+
; CHECK-NEXT: ST128 renamable $r0q, $r15d, 0, $noreg
47+
; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
48+
; CHECK-NEXT: Return
4549
renamable $r1d = LLILL 1
4650
STG killed renamable $r1d, killed $r15d, 8, $noreg
4751
renamable $r0d = LLILL 0

0 commit comments

Comments
 (0)