Skip to content

[RegAllocFast] NFC cleanups #74860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2023
Merged

[RegAllocFast] NFC cleanups #74860

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 30 additions & 35 deletions llvm/lib/CodeGen/RegAllocFast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class RegAllocFast : public MachineFunctionPass {
Register VirtReg);
bool defineVirtReg(MachineInstr &MI, unsigned OpNum, Register VirtReg,
bool LookAtPhysRegUses = false);
bool useVirtReg(MachineInstr &MI, unsigned OpNum, Register VirtReg);
bool useVirtReg(MachineInstr &MI, MachineOperand &MO, Register VirtReg);

MachineBasicBlock::iterator
getMBBBeginInsertionPoint(MachineBasicBlock &MBB,
Expand Down Expand Up @@ -984,17 +984,15 @@ bool RegAllocFast::defineVirtReg(MachineInstr &MI, unsigned OpNum,

/// Allocates a register for a VirtReg use.
/// \return true if MI's MachineOperands were re-arranged/invalidated.
bool RegAllocFast::useVirtReg(MachineInstr &MI, unsigned OpNum,
bool RegAllocFast::useVirtReg(MachineInstr &MI, MachineOperand &MO,
Register VirtReg) {
assert(VirtReg.isVirtual() && "Not a virtual register");
if (!shouldAllocateRegister(VirtReg))
return false;
MachineOperand &MO = MI.getOperand(OpNum);
LiveRegMap::iterator LRI;
bool New;
std::tie(LRI, New) = LiveVirtRegs.insert(LiveReg(VirtReg));
if (New) {
MachineOperand &MO = MI.getOperand(OpNum);
if (!MO.isKill()) {
if (mayLiveOut(VirtReg)) {
LRI->LiveOut = true;
Expand Down Expand Up @@ -1224,6 +1222,17 @@ void RegAllocFast::findAndSortDefOperandIndexes(const MachineInstr &MI) {
});
}

// Returns true if MO is tied and the operand it's tied to is not Undef (not
// Undef is not the same thing as Def).
static bool isTiedToNotUndef(const MachineOperand &MO) {
if (!MO.isTied())
return false;
const MachineInstr &MI = *MO.getParent();
unsigned TiedIdx = MI.findTiedOperandIdx(MI.getOperandNo(&MO));
const MachineOperand &TiedMO = MI.getOperand(TiedIdx);
return !TiedMO.isUndef();
}

void RegAllocFast::allocateInstruction(MachineInstr &MI) {
// The basic algorithm here is:
// 1. Mark registers of def operands as free
Expand All @@ -1241,21 +1250,14 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
RegMasks.clear();
BundleVirtRegsMap.clear();

auto TiedOpIsUndef = [&](const MachineOperand &MO, unsigned Idx) {
assert(MO.isTied());
unsigned TiedIdx = MI.findTiedOperandIdx(Idx);
const MachineOperand &TiedMO = MI.getOperand(TiedIdx);
return TiedMO.isUndef();
};
// Scan for special cases; Apply pre-assigned register defs to state.
bool HasPhysRegUse = false;
bool HasRegMask = false;
bool HasVRegDef = false;
bool HasDef = false;
bool HasEarlyClobber = false;
bool NeedToAssignLiveThroughs = false;
for (unsigned I = 0; I < MI.getNumOperands(); ++I) {
MachineOperand &MO = MI.getOperand(I);
for (MachineOperand &MO : MI.operands()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it was intentional, especially given it is not even storing MI.getNumOperands() in a variable. And in fact git blame points to this: https://reviews.llvm.org/D124834

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was done to pass I to TiedOpIsUndef, not because MI's number of operands change in the loop. I've eliminated the need to pass I, since it can be rematerialized from the MachineOperand itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, LGTM then.

if (MO.isReg()) {
Register Reg = MO.getReg();
if (Reg.isVirtual()) {
Expand All @@ -1268,8 +1270,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
HasEarlyClobber = true;
NeedToAssignLiveThroughs = true;
}
if ((MO.isTied() && !TiedOpIsUndef(MO, I)) ||
(MO.getSubReg() != 0 && !MO.isUndef()))
if (isTiedToNotUndef(MO) || (MO.getSubReg() != 0 && !MO.isUndef()))
NeedToAssignLiveThroughs = true;
}
} else if (Reg.isPhysical()) {
Expand Down Expand Up @@ -1314,35 +1315,32 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
for (uint16_t OpIdx : DefOperandIndexes) {
MachineOperand &MO = MI.getOperand(OpIdx);
LLVM_DEBUG(dbgs() << "Allocating " << MO << '\n');
unsigned Reg = MO.getReg();
if (MO.isEarlyClobber() ||
(MO.isTied() && !TiedOpIsUndef(MO, OpIdx)) ||
Register Reg = MO.getReg();
if (MO.isEarlyClobber() || isTiedToNotUndef(MO) ||
(MO.getSubReg() && !MO.isUndef())) {
ReArrangedImplicitOps = defineLiveThroughVirtReg(MI, OpIdx, Reg);
} else {
ReArrangedImplicitOps = defineVirtReg(MI, OpIdx, Reg);
}
if (ReArrangedImplicitOps) {
// Implicit operands of MI were re-arranged,
// re-compute DefOperandIndexes.
// Implicit operands of MI were re-arranged,
// re-compute DefOperandIndexes.
if (ReArrangedImplicitOps)
break;
}
}
}
} else {
// Assign virtual register defs.
while (ReArrangedImplicitOps) {
ReArrangedImplicitOps = false;
for (unsigned I = 0, E = MI.getNumOperands(); I < E; ++I) {
MachineOperand &MO = MI.getOperand(I);
for (MachineOperand &MO : MI.operands()) {
if (!MO.isReg() || !MO.isDef())
continue;
Register Reg = MO.getReg();
if (Reg.isVirtual()) {
ReArrangedImplicitOps = defineVirtReg(MI, I, Reg);
if (ReArrangedImplicitOps) {
ReArrangedImplicitOps =
defineVirtReg(MI, MI.getOperandNo(&MO), Reg);
if (ReArrangedImplicitOps)
break;
}
}
}
}
Expand All @@ -1352,8 +1350,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
// Free registers occupied by defs.
// Iterate operands in reverse order, so we see the implicit super register
// defs first (we added them earlier in case of <def,read-undef>).
for (signed I = MI.getNumOperands() - 1; I >= 0; --I) {
MachineOperand &MO = MI.getOperand(I);
for (MachineOperand &MO : reverse(MI.operands())) {
if (!MO.isReg() || !MO.isDef())
continue;

Expand All @@ -1370,7 +1367,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
"tied def assigned to clobbered register");

// Do not free tied operands and early clobbers.
if ((MO.isTied() && !TiedOpIsUndef(MO, I)) || MO.isEarlyClobber())
if (isTiedToNotUndef(MO) || MO.isEarlyClobber())
continue;
if (!Reg)
continue;
Expand Down Expand Up @@ -1411,8 +1408,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
continue;
if (MRI->isReserved(Reg))
continue;
bool displacedAny = usePhysReg(MI, Reg);
if (!displacedAny)
if (!usePhysReg(MI, Reg))
MO.setIsKill(true);
}
}
Expand All @@ -1424,8 +1420,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {
bool ReArrangedImplicitMOs = true;
while (ReArrangedImplicitMOs) {
ReArrangedImplicitMOs = false;
for (unsigned I = 0; I < MI.getNumOperands(); ++I) {
MachineOperand &MO = MI.getOperand(I);
for (MachineOperand &MO : MI.operands()) {
if (!MO.isReg() || !MO.isUse())
continue;
Register Reg = MO.getReg();
Expand All @@ -1443,7 +1438,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {

assert(!MO.isInternalRead() && "Bundles not supported");
assert(MO.readsReg() && "reading use");
ReArrangedImplicitMOs = useVirtReg(MI, I, Reg);
ReArrangedImplicitMOs = useVirtReg(MI, MO, Reg);
if (ReArrangedImplicitMOs)
break;
}
Expand All @@ -1465,7 +1460,7 @@ void RegAllocFast::allocateInstruction(MachineInstr &MI) {

// Free early clobbers.
if (HasEarlyClobber) {
for (MachineOperand &MO : llvm::reverse(MI.all_defs())) {
for (MachineOperand &MO : reverse(MI.all_defs())) {
if (!MO.isEarlyClobber())
continue;
assert(!MO.getSubReg() && "should be already handled in def processing");
Expand Down