Skip to content

Commit a8f6243

Browse files
nhaehnleLukacma
nhaehnle
authored and
Lukacma
committed
AMDGPU: Refactor three-address conversion (NFC) (llvm#162558)
Extract the core of the instruction rewriting into an implementation method, and unify the update of live variables / intervals updates in its caller. This is intended to help make future changes to three-address conversion more robust.
1 parent 911e1a2 commit a8f6243

File tree

2 files changed

+76
-69
lines changed

2 files changed

+76
-69
lines changed

‎llvm/lib/Target/AMDGPU/SIInstrInfo.cpp‎

Lines changed: 71 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -4032,28 +4032,31 @@ static unsigned getNewFMAInst(const GCNSubtarget &ST, unsigned Opc) {
40324032
}
40334033
}
40344034

4035+
/// Helper struct for the implementation of 3-address conversion to communicate
4036+
/// updates made to instruction operands.
4037+
struct SIInstrInfo::ThreeAddressUpdates {
4038+
/// Other instruction whose def is no longer used by the converted
4039+
/// instruction.
4040+
MachineInstr *RemoveMIUse = nullptr;
4041+
};
4042+
40354043
MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
40364044
LiveVariables *LV,
40374045
LiveIntervals *LIS) const {
40384046
MachineBasicBlock &MBB = *MI.getParent();
4039-
unsigned Opc = MI.getOpcode();
4047+
ThreeAddressUpdates U;
4048+
MachineInstr *NewMI = convertToThreeAddressImpl(MI, U);
40404049

4041-
// Handle MFMA.
4042-
int NewMFMAOpc = AMDGPU::getMFMAEarlyClobberOp(Opc);
4043-
if (NewMFMAOpc != -1) {
4044-
MachineInstrBuilder MIB =
4045-
BuildMI(MBB, MI, MI.getDebugLoc(), get(NewMFMAOpc));
4046-
for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I)
4047-
MIB.add(MI.getOperand(I));
4048-
updateLiveVariables(LV, MI, *MIB);
4050+
if (NewMI) {
4051+
updateLiveVariables(LV, MI, *NewMI);
40494052
if (LIS) {
4050-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
4053+
LIS->ReplaceMachineInstrInMaps(MI, *NewMI);
40514054
// SlotIndex of defs needs to be updated when converting to early-clobber
4052-
MachineOperand &Def = MIB->getOperand(0);
4055+
MachineOperand &Def = NewMI->getOperand(0);
40534056
if (Def.isEarlyClobber() && Def.isReg() &&
40544057
LIS->hasInterval(Def.getReg())) {
4055-
SlotIndex OldIndex = LIS->getInstructionIndex(*MIB).getRegSlot(false);
4056-
SlotIndex NewIndex = LIS->getInstructionIndex(*MIB).getRegSlot(true);
4058+
SlotIndex OldIndex = LIS->getInstructionIndex(*NewMI).getRegSlot(false);
4059+
SlotIndex NewIndex = LIS->getInstructionIndex(*NewMI).getRegSlot(true);
40574060
auto &LI = LIS->getInterval(Def.getReg());
40584061
auto UpdateDefIndex = [&](LiveRange &LR) {
40594062
auto *S = LR.find(OldIndex);
@@ -4068,6 +4071,58 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
40684071
UpdateDefIndex(SR);
40694072
}
40704073
}
4074+
}
4075+
4076+
if (U.RemoveMIUse) {
4077+
MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
4078+
// The only user is the instruction which will be killed.
4079+
Register DefReg = U.RemoveMIUse->getOperand(0).getReg();
4080+
4081+
if (MRI.hasOneNonDBGUse(DefReg)) {
4082+
// We cannot just remove the DefMI here, calling pass will crash.
4083+
U.RemoveMIUse->setDesc(get(AMDGPU::IMPLICIT_DEF));
4084+
U.RemoveMIUse->getOperand(0).setIsDead(true);
4085+
for (unsigned I = U.RemoveMIUse->getNumOperands() - 1; I != 0; --I)
4086+
U.RemoveMIUse->removeOperand(I);
4087+
if (LV)
4088+
LV->getVarInfo(DefReg).AliveBlocks.clear();
4089+
}
4090+
4091+
if (LIS) {
4092+
LiveInterval &DefLI = LIS->getInterval(DefReg);
4093+
4094+
// We cannot delete the original instruction here, so hack out the use
4095+
// in the original instruction with a dummy register so we can use
4096+
// shrinkToUses to deal with any multi-use edge cases. Other targets do
4097+
// not have the complexity of deleting a use to consider here.
4098+
Register DummyReg = MRI.cloneVirtualRegister(DefReg);
4099+
for (MachineOperand &MIOp : MI.uses()) {
4100+
if (MIOp.isReg() && MIOp.getReg() == DefReg) {
4101+
MIOp.setIsUndef(true);
4102+
MIOp.setReg(DummyReg);
4103+
}
4104+
}
4105+
4106+
LIS->shrinkToUses(&DefLI);
4107+
}
4108+
}
4109+
4110+
return NewMI;
4111+
}
4112+
4113+
MachineInstr *
4114+
SIInstrInfo::convertToThreeAddressImpl(MachineInstr &MI,
4115+
ThreeAddressUpdates &U) const {
4116+
MachineBasicBlock &MBB = *MI.getParent();
4117+
unsigned Opc = MI.getOpcode();
4118+
4119+
// Handle MFMA.
4120+
int NewMFMAOpc = AMDGPU::getMFMAEarlyClobberOp(Opc);
4121+
if (NewMFMAOpc != -1) {
4122+
MachineInstrBuilder MIB =
4123+
BuildMI(MBB, MI, MI.getDebugLoc(), get(NewMFMAOpc));
4124+
for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I)
4125+
MIB.add(MI.getOperand(I));
40714126
return MIB;
40724127
}
40734128

@@ -4077,11 +4132,6 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
40774132
.setMIFlags(MI.getFlags());
40784133
for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I)
40794134
MIB->addOperand(MI.getOperand(I));
4080-
4081-
updateLiveVariables(LV, MI, *MIB);
4082-
if (LIS)
4083-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
4084-
40854135
return MIB;
40864136
}
40874137

@@ -4152,39 +4202,6 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
41524202
(ST.getConstantBusLimit(Opc) > 1 || !Src0->isReg() ||
41534203
!RI.isSGPRReg(MBB.getParent()->getRegInfo(), Src0->getReg()))) {
41544204
MachineInstr *DefMI;
4155-
const auto killDef = [&]() -> void {
4156-
MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
4157-
// The only user is the instruction which will be killed.
4158-
Register DefReg = DefMI->getOperand(0).getReg();
4159-
4160-
if (MRI.hasOneNonDBGUse(DefReg)) {
4161-
// We cannot just remove the DefMI here, calling pass will crash.
4162-
DefMI->setDesc(get(AMDGPU::IMPLICIT_DEF));
4163-
DefMI->getOperand(0).setIsDead(true);
4164-
for (unsigned I = DefMI->getNumOperands() - 1; I != 0; --I)
4165-
DefMI->removeOperand(I);
4166-
if (LV)
4167-
LV->getVarInfo(DefReg).AliveBlocks.clear();
4168-
}
4169-
4170-
if (LIS) {
4171-
LiveInterval &DefLI = LIS->getInterval(DefReg);
4172-
4173-
// We cannot delete the original instruction here, so hack out the use
4174-
// in the original instruction with a dummy register so we can use
4175-
// shrinkToUses to deal with any multi-use edge cases. Other targets do
4176-
// not have the complexity of deleting a use to consider here.
4177-
Register DummyReg = MRI.cloneVirtualRegister(DefReg);
4178-
for (MachineOperand &MIOp : MI.uses()) {
4179-
if (MIOp.isReg() && MIOp.getReg() == DefReg) {
4180-
MIOp.setIsUndef(true);
4181-
MIOp.setReg(DummyReg);
4182-
}
4183-
}
4184-
4185-
LIS->shrinkToUses(&DefLI);
4186-
}
4187-
};
41884205

41894206
int64_t Imm;
41904207
if (!Src0Literal && getFoldableImm(Src2, Imm, &DefMI)) {
@@ -4196,10 +4213,7 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
41964213
.add(*Src1)
41974214
.addImm(Imm)
41984215
.setMIFlags(MI.getFlags());
4199-
updateLiveVariables(LV, MI, *MIB);
4200-
if (LIS)
4201-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
4202-
killDef();
4216+
U.RemoveMIUse = DefMI;
42034217
return MIB;
42044218
}
42054219
}
@@ -4212,11 +4226,7 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
42124226
.addImm(Imm)
42134227
.add(*Src2)
42144228
.setMIFlags(MI.getFlags());
4215-
updateLiveVariables(LV, MI, *MIB);
4216-
4217-
if (LIS)
4218-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
4219-
killDef();
4229+
U.RemoveMIUse = DefMI;
42204230
return MIB;
42214231
}
42224232
}
@@ -4235,12 +4245,7 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
42354245
.addImm(Imm)
42364246
.add(*Src2)
42374247
.setMIFlags(MI.getFlags());
4238-
updateLiveVariables(LV, MI, *MIB);
4239-
4240-
if (LIS)
4241-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
4242-
if (DefMI)
4243-
killDef();
4248+
U.RemoveMIUse = DefMI;
42444249
return MIB;
42454250
}
42464251
}
@@ -4269,9 +4274,6 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineInstr &MI,
42694274
.setMIFlags(MI.getFlags());
42704275
if (AMDGPU::hasNamedOperand(NewOpc, AMDGPU::OpName::op_sel))
42714276
MIB.addImm(OpSel ? OpSel->getImm() : 0);
4272-
updateLiveVariables(LV, MI, *MIB);
4273-
if (LIS)
4274-
LIS->ReplaceMachineInstrInMaps(MI, *MIB);
42754277
return MIB;
42764278
}
42774279

‎llvm/lib/Target/AMDGPU/SIInstrInfo.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ struct SIInstrWorklist {
8888
};
8989

9090
class SIInstrInfo final : public AMDGPUGenInstrInfo {
91+
struct ThreeAddressUpdates;
92+
9193
private:
9294
const SIRegisterInfo RI;
9395
const GCNSubtarget &ST;
@@ -190,6 +192,9 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
190192

191193
bool resultDependsOnExec(const MachineInstr &MI) const;
192194

195+
MachineInstr *convertToThreeAddressImpl(MachineInstr &MI,
196+
ThreeAddressUpdates &Updates) const;
197+
193198
protected:
194199
/// If the specific machine instruction is a instruction that moves/copies
195200
/// value from one register to another register return destination and source

0 commit comments

Comments
 (0)