[MachineOperand][Target] MachineOperand::isRenamable semantics changes
Summary:
Add a target option AllowRegisterRenaming that is used to opt in to
post-register-allocation renaming of registers. This is set to 0 by
default, which causes the hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq
fields of all opcodes to be set to 1, causing
MachineOperand::isRenamable to always return false.
Set the AllowRegisterRenaming flag to 1 for all in-tree targets that
have lit tests that were effected by enabling COPY forwarding in
MachineCopyPropagation (AArch64, AMDGPU, ARM, Hexagon, Mips, PowerPC,
RISCV, Sparc, SystemZ and X86).
Add some more comments describing the semantics of the
MachineOperand::isRenamable function and how it is set and maintained.
Change isRenamable to check the operand's opcode
hasExtraSrcRegAllocReq/hasExtraDstRegAllocReq bit directly instead of
relying on it being consistently reflected in the IsRenamable bit
setting.
Clear the IsRenamable bit when changing an operand's register value.
Remove target code that was clearing the IsRenamable bit when changing
registers/opcodes now that this is done conservatively by default.
Change setting of hasExtraSrcRegAllocReq in AMDGPU target to be done in
one place covering all opcodes that have constant pipe read limit
restrictions.
Reviewers: qcolombet, MatzeB
Subscribers: aemerson, arsenm, jyknight, mcrosier, sdardis, nhaehnle, javed.absar, tpr, arichardson, kristof.beyls, kbarton, fedor.sergeev, asb, rbar, johnrusso, simoncook, jordy.potman.lists, apazos, sabuasal, niosHD, escha, nemanjai, llvm-commits
Differential Revision: https://reviews.llvm.org/D43042
llvm-svn: 325931
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index d2d3bc7..98993b2 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -930,8 +930,7 @@
void MachineInstr::substituteRegister(unsigned FromReg, unsigned ToReg,
unsigned SubIdx,
- const TargetRegisterInfo &RegInfo,
- bool ClearIsRenamable) {
+ const TargetRegisterInfo &RegInfo) {
if (TargetRegisterInfo::isPhysicalRegister(ToReg)) {
if (SubIdx)
ToReg = RegInfo.getSubReg(ToReg, SubIdx);
@@ -939,11 +938,8 @@
if (!MO.isReg() || MO.getReg() != FromReg)
continue;
MO.substPhysReg(ToReg, RegInfo);
- if (ClearIsRenamable)
- MO.setIsRenamable(false);
}
} else {
- assert(!ClearIsRenamable && "IsRenamable invalid for virtual registers");
for (MachineOperand &MO : operands()) {
if (!MO.isReg() || MO.getReg() != FromReg)
continue;
diff --git a/llvm/lib/CodeGen/MachineOperand.cpp b/llvm/lib/CodeGen/MachineOperand.cpp
index 9122ede..409dd07 100644
--- a/llvm/lib/CodeGen/MachineOperand.cpp
+++ b/llvm/lib/CodeGen/MachineOperand.cpp
@@ -50,6 +50,9 @@
if (getReg() == Reg)
return; // No change.
+ // Clear the IsRenamable bit to keep it conservatively correct.
+ IsRenamable = false;
+
// Otherwise, we have to change the register. If this operand is embedded
// into a machine function, we need to update the old and new register's
// use/def lists.
@@ -110,30 +113,27 @@
assert(isReg() && "Wrong MachineOperand accessor");
assert(TargetRegisterInfo::isPhysicalRegister(getReg()) &&
"isRenamable should only be checked on physical registers");
- return IsRenamable;
+ if (!IsRenamable)
+ return false;
+
+ const MachineInstr *MI = getParent();
+ if (!MI)
+ return true;
+
+ if (isDef())
+ return !MI->hasExtraDefRegAllocReq(MachineInstr::IgnoreBundle);
+
+ assert(isUse() && "Reg is not def or use");
+ return !MI->hasExtraSrcRegAllocReq(MachineInstr::IgnoreBundle);
}
void MachineOperand::setIsRenamable(bool Val) {
assert(isReg() && "Wrong MachineOperand accessor");
assert(TargetRegisterInfo::isPhysicalRegister(getReg()) &&
"setIsRenamable should only be called on physical registers");
- if (const MachineInstr *MI = getParent())
- if ((isDef() && MI->hasExtraDefRegAllocReq()) ||
- (isUse() && MI->hasExtraSrcRegAllocReq()))
- assert(!Val && "isRenamable should be false for "
- "hasExtraDefRegAllocReq/hasExtraSrcRegAllocReq opcodes");
IsRenamable = Val;
}
-void MachineOperand::setIsRenamableIfNoExtraRegAllocReq() {
- if (const MachineInstr *MI = getParent())
- if ((isDef() && MI->hasExtraDefRegAllocReq()) ||
- (isUse() && MI->hasExtraSrcRegAllocReq()))
- return;
-
- setIsRenamable(true);
-}
-
// If this operand is currently a register operand, and if this is in a
// function, deregister the operand from the register's use/def list.
void MachineOperand::removeRegFromUses() {
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 3e87dd6..3b38bbf 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1132,14 +1132,10 @@
}
}
if (MO->isRenamable()) {
- if ((MO->isDef() && MI->hasExtraDefRegAllocReq()) ||
- (MO->isUse() && MI->hasExtraSrcRegAllocReq()))
- report("Illegal isRenamable setting for opcode with extra regalloc "
- "requirements",
- MO, MONum);
- if (MRI->isReserved(Reg))
+ if (MRI->isReserved(Reg)) {
report("isRenamable set on reserved register", MO, MONum);
- return;
+ return;
+ }
}
} else {
// Virtual register.
diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 17d9492..7a8d422 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -699,13 +699,13 @@
bool Dead = MO.isDead();
if (!MO.getSubReg()) {
MO.setReg(PhysReg);
- MO.setIsRenamableIfNoExtraRegAllocReq();
+ MO.setIsRenamable(true);
return MO.isKill() || Dead;
}
// Handle subregister index.
MO.setReg(PhysReg ? TRI->getSubReg(PhysReg, MO.getSubReg()) : 0);
- MO.setIsRenamableIfNoExtraRegAllocReq();
+ MO.setIsRenamable(true);
MO.setSubReg(0);
// A kill flag implies killing the full register. Add corresponding super
diff --git a/llvm/lib/CodeGen/VirtRegMap.cpp b/llvm/lib/CodeGen/VirtRegMap.cpp
index 13f7e83..cb3b204 100644
--- a/llvm/lib/CodeGen/VirtRegMap.cpp
+++ b/llvm/lib/CodeGen/VirtRegMap.cpp
@@ -530,7 +530,7 @@
// Rewrite. Note we could have used MachineOperand::substPhysReg(), but
// we need the inlining here.
MO.setReg(PhysReg);
- MO.setIsRenamableIfNoExtraRegAllocReq();
+ MO.setIsRenamable(true);
}
// Add any missing super-register kills after rewriting the whole