Skip to content

Commit

Permalink
[CodeGen] Make the parameter TRI required in some functions. (#85968)
Browse files Browse the repository at this point in the history
Fixes #82659

There are some functions, such as `findRegisterDefOperandIdx` and  `findRegisterDefOperand`, that have too many default parameters. As a result, we have encountered some issues due to the lack of TRI  parameters, as shown in issue #82411.

Following @RKSimon 's suggestion, this patch refactors 9 functions, including `{reads, kills, defines, modifies}Register`,  `registerDefIsDead`, and `findRegister{UseOperandIdx, UseOperand, DefOperandIdx, DefOperand}`, adjusting the order of the TRI parameter and making it required. In addition, all the places that call these functions have also been updated correctly to ensure no additional impact.

After this, the caller of these functions should explicitly know whether to pass the `TargetRegisterInfo` or just a `nullptr`.
  • Loading branch information
simonzgx committed Apr 24, 2024
1 parent 07e6c16 commit f6d431f
Show file tree
Hide file tree
Showing 82 changed files with 402 additions and 317 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,8 @@ class LegalizationArtifactCombiner {
unsigned &DefOperandIdx) {
if (Register Def = findValueFromDefImpl(Reg, 0, Size)) {
if (auto *Unmerge = dyn_cast<GUnmerge>(MRI.getVRegDef(Def))) {
DefOperandIdx = Unmerge->findRegisterDefOperandIdx(Def);
DefOperandIdx =
Unmerge->findRegisterDefOperandIdx(Def, /*TRI=*/nullptr);
return Unmerge;
}
}
Expand Down
72 changes: 34 additions & 38 deletions llvm/include/llvm/CodeGen/MachineInstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -1466,9 +1466,8 @@ class MachineInstr
/// is a read of a super-register.
/// This does not count partial redefines of virtual registers as reads:
/// %reg1024:6 = OP.
bool readsRegister(Register Reg,
const TargetRegisterInfo *TRI = nullptr) const {
return findRegisterUseOperandIdx(Reg, false, TRI) != -1;
bool readsRegister(Register Reg, const TargetRegisterInfo *TRI) const {
return findRegisterUseOperandIdx(Reg, TRI, false) != -1;
}

/// Return true if the MachineInstr reads the specified virtual register.
Expand All @@ -1487,34 +1486,30 @@ class MachineInstr
/// Return true if the MachineInstr kills the specified register.
/// If TargetRegisterInfo is non-null, then it also checks if there is
/// a kill of a super-register.
bool killsRegister(Register Reg,
const TargetRegisterInfo *TRI = nullptr) const {
return findRegisterUseOperandIdx(Reg, true, TRI) != -1;
bool killsRegister(Register Reg, const TargetRegisterInfo *TRI) const {
return findRegisterUseOperandIdx(Reg, TRI, true) != -1;
}

/// Return true if the MachineInstr fully defines the specified register.
/// If TargetRegisterInfo is non-null, then it also checks
/// if there is a def of a super-register.
/// NOTE: It's ignoring subreg indices on virtual registers.
bool definesRegister(Register Reg,
const TargetRegisterInfo *TRI = nullptr) const {
return findRegisterDefOperandIdx(Reg, false, false, TRI) != -1;
bool definesRegister(Register Reg, const TargetRegisterInfo *TRI) const {
return findRegisterDefOperandIdx(Reg, TRI, false, false) != -1;
}

/// Return true if the MachineInstr modifies (fully define or partially
/// define) the specified register.
/// NOTE: It's ignoring subreg indices on virtual registers.
bool modifiesRegister(Register Reg,
const TargetRegisterInfo *TRI = nullptr) const {
return findRegisterDefOperandIdx(Reg, false, true, TRI) != -1;
bool modifiesRegister(Register Reg, const TargetRegisterInfo *TRI) const {
return findRegisterDefOperandIdx(Reg, TRI, false, true) != -1;
}

/// Returns true if the register is dead in this machine instruction.
/// If TargetRegisterInfo is non-null, then it also checks
/// if there is a dead def of a super-register.
bool registerDefIsDead(Register Reg,
const TargetRegisterInfo *TRI = nullptr) const {
return findRegisterDefOperandIdx(Reg, true, false, TRI) != -1;
bool registerDefIsDead(Register Reg, const TargetRegisterInfo *TRI) const {
return findRegisterDefOperandIdx(Reg, TRI, true, false) != -1;
}

/// Returns true if the MachineInstr has an implicit-use operand of exactly
Expand All @@ -1524,22 +1519,23 @@ class MachineInstr
/// Returns the operand index that is a use of the specific register or -1
/// if it is not found. It further tightens the search criteria to a use
/// that kills the register if isKill is true.
int findRegisterUseOperandIdx(Register Reg, bool isKill = false,
const TargetRegisterInfo *TRI = nullptr) const;
int findRegisterUseOperandIdx(Register Reg, const TargetRegisterInfo *TRI,
bool isKill = false) const;

/// Wrapper for findRegisterUseOperandIdx, it returns
/// a pointer to the MachineOperand rather than an index.
MachineOperand *findRegisterUseOperand(Register Reg, bool isKill = false,
const TargetRegisterInfo *TRI = nullptr) {
int Idx = findRegisterUseOperandIdx(Reg, isKill, TRI);
MachineOperand *findRegisterUseOperand(Register Reg,
const TargetRegisterInfo *TRI,
bool isKill = false) {
int Idx = findRegisterUseOperandIdx(Reg, TRI, isKill);
return (Idx == -1) ? nullptr : &getOperand(Idx);
}

const MachineOperand *findRegisterUseOperand(
Register Reg, bool isKill = false,
const TargetRegisterInfo *TRI = nullptr) const {
return const_cast<MachineInstr *>(this)->
findRegisterUseOperand(Reg, isKill, TRI);
const MachineOperand *findRegisterUseOperand(Register Reg,
const TargetRegisterInfo *TRI,
bool isKill = false) const {
return const_cast<MachineInstr *>(this)->findRegisterUseOperand(Reg, TRI,
isKill);
}

/// Returns the operand index that is a def of the specified register or
Expand All @@ -1548,26 +1544,26 @@ class MachineInstr
/// overlap the specified register. If TargetRegisterInfo is non-null,
/// then it also checks if there is a def of a super-register.
/// This may also return a register mask operand when Overlap is true.
int findRegisterDefOperandIdx(Register Reg,
bool isDead = false, bool Overlap = false,
const TargetRegisterInfo *TRI = nullptr) const;
int findRegisterDefOperandIdx(Register Reg, const TargetRegisterInfo *TRI,
bool isDead = false,
bool Overlap = false) const;

/// Wrapper for findRegisterDefOperandIdx, it returns
/// a pointer to the MachineOperand rather than an index.
MachineOperand *
findRegisterDefOperand(Register Reg, bool isDead = false,
bool Overlap = false,
const TargetRegisterInfo *TRI = nullptr) {
int Idx = findRegisterDefOperandIdx(Reg, isDead, Overlap, TRI);
MachineOperand *findRegisterDefOperand(Register Reg,
const TargetRegisterInfo *TRI,
bool isDead = false,
bool Overlap = false) {
int Idx = findRegisterDefOperandIdx(Reg, TRI, isDead, Overlap);
return (Idx == -1) ? nullptr : &getOperand(Idx);
}

const MachineOperand *
findRegisterDefOperand(Register Reg, bool isDead = false,
bool Overlap = false,
const TargetRegisterInfo *TRI = nullptr) const {
const MachineOperand *findRegisterDefOperand(Register Reg,
const TargetRegisterInfo *TRI,
bool isDead = false,
bool Overlap = false) const {
return const_cast<MachineInstr *>(this)->findRegisterDefOperand(
Reg, isDead, Overlap, TRI);
Reg, TRI, isDead, Overlap);
}

/// Find the index of the first operand in the
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ bool AggressiveAntiDepBreaker::IsImplicitDefUse(MachineInstr &MI,

MachineOperand *Op = nullptr;
if (MO.isDef())
Op = MI.findRegisterUseOperand(Reg, true);
Op = MI.findRegisterUseOperand(Reg, /*TRI=*/nullptr, true);
else
Op = MI.findRegisterDefOperand(Reg);
Op = MI.findRegisterDefOperand(Reg, /*TRI=*/nullptr);

return(Op && Op->isImplicit());
}
Expand Down Expand Up @@ -679,7 +679,7 @@ bool AggressiveAntiDepBreaker::FindSuitableFreeRegisters(
// defines 'NewReg' via an early-clobber operand.
for (const auto &Q : make_range(RegRefs.equal_range(Reg))) {
MachineInstr *UseMI = Q.second.Operand->getParent();
int Idx = UseMI->findRegisterDefOperandIdx(NewReg, false, true, TRI);
int Idx = UseMI->findRegisterDefOperandIdx(NewReg, TRI, false, true);
if (Idx == -1)
continue;

Expand Down Expand Up @@ -846,7 +846,8 @@ unsigned AggressiveAntiDepBreaker::BreakAntiDependencies(
continue;
} else {
// No anti-dep breaking for implicit deps
MachineOperand *AntiDepOp = MI.findRegisterDefOperand(AntiDepReg);
MachineOperand *AntiDepOp =
MI.findRegisterDefOperand(AntiDepReg, /*TRI=*/nullptr);
assert(AntiDepOp && "Can't find index for defined register operand");
if (!AntiDepOp || AntiDepOp->isImplicit()) {
LLVM_DEBUG(dbgs() << " (implicit)\n");
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/CalcSpillWeights.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ float VirtRegAuxInfo::weightCalcHelper(LiveInterval &LI, SlotIndex *Start,

// For terminators that produce values, ask the backend if the register is
// not spillable.
if (TII.isUnspillableTerminator(MI) && MI->definesRegister(LI.reg())) {
if (TII.isUnspillableTerminator(MI) &&
MI->definesRegister(LI.reg(), /*TRI=*/nullptr)) {
LI.markNotSpillable();
return -1.0f;
}
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/CodeGenCommonISel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ void llvm::salvageDebugInfoForDbgValue(const MachineRegisterInfo &MRI,
continue;
}

int UseMOIdx = DbgMI->findRegisterUseOperandIdx(DefMO->getReg());
int UseMOIdx =
DbgMI->findRegisterUseOperandIdx(DefMO->getReg(), /*TRI=*/nullptr);
assert(UseMOIdx != -1 && DbgMI->hasDebugOperandForReg(DefMO->getReg()) &&
"Must use salvaged instruction as its location");

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/EarlyIfConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,8 @@ static bool hasSameValue(const MachineRegisterInfo &MRI,
return false;

// Further, check that the two defs come from corresponding operands.
int TIdx = TDef->findRegisterDefOperandIdx(TReg);
int FIdx = FDef->findRegisterDefOperandIdx(FReg);
int TIdx = TDef->findRegisterDefOperandIdx(TReg, /*TRI=*/nullptr);
int FIdx = FDef->findRegisterDefOperandIdx(FReg, /*TRI=*/nullptr);
if (TIdx == -1 || FIdx == -1)
return false;

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static Register performCopyPropagation(Register Reg,
bool &IsKill, const TargetInstrInfo &TII,
const TargetRegisterInfo &TRI) {
// First check if statepoint itself uses Reg in non-meta operands.
int Idx = RI->findRegisterUseOperandIdx(Reg, false, &TRI);
int Idx = RI->findRegisterUseOperandIdx(Reg, &TRI, false);
if (Idx >= 0 && (unsigned)Idx < StatepointOpers(&*RI).getNumDeoptArgsIdx()) {
IsKill = false;
return Reg;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2800,8 +2800,8 @@ bool CombinerHelper::matchEqualDefs(const MachineOperand &MOP1,
// %5:_(s8), %6:_(s8), %7:_(s8), %8:_(s8) = G_UNMERGE_VALUES %4:_(<4 x s8>)
// I1 and I2 are different instructions but produce same values,
// %1 and %6 are same, %1 and %7 are not the same value.
return I1->findRegisterDefOperandIdx(InstAndDef1->Reg) ==
I2->findRegisterDefOperandIdx(InstAndDef2->Reg);
return I1->findRegisterDefOperandIdx(InstAndDef1->Reg, /*TRI=*/nullptr) ==
I2->findRegisterDefOperandIdx(InstAndDef2->Reg, /*TRI=*/nullptr);
}
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,8 @@ void RegBankSelect::tryAvoidingSplit(
// If the next terminator uses Reg, this means we have
// to split right after MI and thus we need a way to ask
// which outgoing edges are affected.
assert(!Next->readsRegister(Reg) && "Need to split between terminators");
assert(!Next->readsRegister(Reg, /*TRI=*/nullptr) &&
"Need to split between terminators");
// We will split all the edges and repair there.
} else {
// This is a virtual register defined by a terminator.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/InlineSpiller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ static void dumpMachineInstrRangeWithSlotIndex(MachineBasicBlock::iterator B,
// destination that is marked as an early clobber, print the
// early-clobber slot index.
if (VReg) {
MachineOperand *MO = I->findRegisterDefOperand(VReg);
MachineOperand *MO = I->findRegisterDefOperand(VReg, /*TRI=*/nullptr);
if (MO && MO->isEarlyClobber())
Idx = Idx.getRegSlot(true);
}
Expand Down
11 changes: 6 additions & 5 deletions llvm/lib/CodeGen/LiveVariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ void LiveVariables::HandlePhysRegUse(Register Reg, MachineInstr &MI) {
}
}
} else if (LastDef && !PhysRegUse[Reg] &&
!LastDef->findRegisterDefOperand(Reg))
!LastDef->findRegisterDefOperand(Reg, /*TRI=*/nullptr))
// Last def defines the super register, add an implicit def of reg.
LastDef->addOperand(MachineOperand::CreateReg(Reg, true/*IsDef*/,
true/*IsImp*/));
Expand Down Expand Up @@ -361,7 +361,8 @@ bool LiveVariables::HandlePhysRegKill(Register Reg, MachineInstr *MI) {
continue;
bool NeedDef = true;
if (PhysRegDef[Reg] == PhysRegDef[SubReg]) {
MachineOperand *MO = PhysRegDef[Reg]->findRegisterDefOperand(SubReg);
MachineOperand *MO =
PhysRegDef[Reg]->findRegisterDefOperand(SubReg, /*TRI=*/nullptr);
if (MO) {
NeedDef = false;
assert(!MO->isDead());
Expand All @@ -388,15 +389,15 @@ bool LiveVariables::HandlePhysRegKill(Register Reg, MachineInstr *MI) {
true/*IsImp*/, true/*IsKill*/));
else {
MachineOperand *MO =
LastRefOrPartRef->findRegisterDefOperand(Reg, false, false, TRI);
LastRefOrPartRef->findRegisterDefOperand(Reg, TRI, false, false);
bool NeedEC = MO->isEarlyClobber() && MO->getReg() != Reg;
// If the last reference is the last def, then it's not used at all.
// That is, unless we are currently processing the last reference itself.
LastRefOrPartRef->addRegisterDead(Reg, TRI, true);
if (NeedEC) {
// If we are adding a subreg def and the superreg def is marked early
// clobber, add an early clobber marker to the subreg def.
MO = LastRefOrPartRef->findRegisterDefOperand(Reg);
MO = LastRefOrPartRef->findRegisterDefOperand(Reg, /*TRI=*/nullptr);
if (MO)
MO->setIsEarlyClobber();
}
Expand Down Expand Up @@ -727,7 +728,7 @@ void LiveVariables::recomputeForSingleDefVirtReg(Register Reg) {
if (MI.isPHI())
break;
if (MI.readsVirtualRegister(Reg)) {
assert(!MI.killsRegister(Reg));
assert(!MI.killsRegister(Reg, /*TRI=*/nullptr));
MI.addRegisterKilled(Reg, nullptr);
VI.Kills.push_back(&MI);
break;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MachineCSE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ bool MachineCSE::ProcessBlockCSE(MachineBasicBlock *MBB) {
for (MachineBasicBlock::iterator II = CSMI, IE = &MI; II != IE; ++II)
for (auto ImplicitDef : ImplicitDefs)
if (MachineOperand *MO = II->findRegisterUseOperand(
ImplicitDef, /*isKill=*/true, TRI))
ImplicitDef, TRI, /*isKill=*/true))
MO->setIsKill(false);
} else {
// If the instructions aren't in the same BB, bail out and clear the
Expand Down
20 changes: 14 additions & 6 deletions llvm/lib/CodeGen/MachineCombiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,10 @@ MachineCombiner::getDepth(SmallVectorImpl<MachineInstr *> &InsInstrs,
assert(DefInstr &&
"There must be a definition for a new virtual register");
DepthOp = InstrDepth[II->second];
int DefIdx = DefInstr->findRegisterDefOperandIdx(MO.getReg());
int UseIdx = InstrPtr->findRegisterUseOperandIdx(MO.getReg());
int DefIdx =
DefInstr->findRegisterDefOperandIdx(MO.getReg(), /*TRI=*/nullptr);
int UseIdx =
InstrPtr->findRegisterUseOperandIdx(MO.getReg(), /*TRI=*/nullptr);
LatencyOp = TSchedModel.computeOperandLatency(DefInstr, DefIdx,
InstrPtr, UseIdx);
} else {
Expand All @@ -241,8 +243,12 @@ MachineCombiner::getDepth(SmallVectorImpl<MachineInstr *> &InsInstrs,
DepthOp = BlockTrace.getInstrCycles(*DefInstr).Depth;
if (!isTransientMI(DefInstr))
LatencyOp = TSchedModel.computeOperandLatency(
DefInstr, DefInstr->findRegisterDefOperandIdx(MO.getReg()),
InstrPtr, InstrPtr->findRegisterUseOperandIdx(MO.getReg()));
DefInstr,
DefInstr->findRegisterDefOperandIdx(MO.getReg(),
/*TRI=*/nullptr),
InstrPtr,
InstrPtr->findRegisterUseOperandIdx(MO.getReg(),
/*TRI=*/nullptr));
}
}
IDepth = std::max(IDepth, DepthOp + LatencyOp);
Expand Down Expand Up @@ -280,8 +286,10 @@ unsigned MachineCombiner::getLatency(MachineInstr *Root, MachineInstr *NewRoot,
unsigned LatencyOp = 0;
if (UseMO && BlockTrace.isDepInTrace(*Root, *UseMO)) {
LatencyOp = TSchedModel.computeOperandLatency(
NewRoot, NewRoot->findRegisterDefOperandIdx(MO.getReg()), UseMO,
UseMO->findRegisterUseOperandIdx(MO.getReg()));
NewRoot,
NewRoot->findRegisterDefOperandIdx(MO.getReg(), /*TRI=*/nullptr),
UseMO,
UseMO->findRegisterUseOperandIdx(MO.getReg(), /*TRI=*/nullptr));
} else {
LatencyOp = TSchedModel.computeInstrLatency(NewRoot);
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MachineCopyPropagation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
// cannot cope with that.
if (isCopyInstr(MI, *TII, UseCopyInstr) &&
MI.modifiesRegister(CopySrcReg, TRI) &&
!MI.definesRegister(CopySrcReg)) {
!MI.definesRegister(CopySrcReg, /*TRI=*/nullptr)) {
LLVM_DEBUG(dbgs() << "MCP: Copy source overlap with dest in " << MI);
continue;
}
Expand Down
13 changes: 7 additions & 6 deletions llvm/lib/CodeGen/MachineInstr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1045,8 +1045,9 @@ bool MachineInstr::hasRegisterImplicitUseOperand(Register Reg) const {
/// findRegisterUseOperandIdx() - Returns the MachineOperand that is a use of
/// the specific register or -1 if it is not found. It further tightens
/// the search criteria to a use that kills the register if isKill is true.
int MachineInstr::findRegisterUseOperandIdx(
Register Reg, bool isKill, const TargetRegisterInfo *TRI) const {
int MachineInstr::findRegisterUseOperandIdx(Register Reg,
const TargetRegisterInfo *TRI,
bool isKill) const {
for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
const MachineOperand &MO = getOperand(i);
if (!MO.isReg() || !MO.isUse())
Expand Down Expand Up @@ -1093,9 +1094,9 @@ MachineInstr::readsWritesVirtualRegister(Register Reg,
/// the specified register or -1 if it is not found. If isDead is true, defs
/// that are not dead are skipped. If TargetRegisterInfo is non-null, then it
/// also checks if there is a def of a super-register.
int
MachineInstr::findRegisterDefOperandIdx(Register Reg, bool isDead, bool Overlap,
const TargetRegisterInfo *TRI) const {
int MachineInstr::findRegisterDefOperandIdx(Register Reg,
const TargetRegisterInfo *TRI,
bool isDead, bool Overlap) const {
bool isPhys = Reg.isPhysical();
for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
const MachineOperand &MO = getOperand(i);
Expand Down Expand Up @@ -2136,7 +2137,7 @@ void MachineInstr::setRegisterDefReadUndef(Register Reg, bool IsUndef) {
void MachineInstr::addRegisterDefined(Register Reg,
const TargetRegisterInfo *RegInfo) {
if (Reg.isPhysical()) {
MachineOperand *MO = findRegisterDefOperand(Reg, false, false, RegInfo);
MachineOperand *MO = findRegisterDefOperand(Reg, RegInfo, false, false);
if (MO)
return;
} else {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
if (MI.modifiesRegister(Reg, TRI)) {
MBBDefs.erase(Reg);
MBBKills.erase(Reg);
} else if (MI.findRegisterUseOperandIdx(Reg, true /*isKill*/, TRI) != -1)
} else if (MI.findRegisterUseOperandIdx(Reg, TRI, true /*isKill*/) != -1)
// Keep track of register kills.
MBBKills[Reg] = &MI;
}
Expand Down
Loading

0 comments on commit f6d431f

Please sign in to comment.