[llvm-branch-commits] [llvm] [mcp-frameinst: 2/3]: [MCP][NFC] Opinionated refactoring (PR #186239)
Scott Linder via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Apr 7 14:05:15 PDT 2026
https://github.com/slinder1 updated https://github.com/llvm/llvm-project/pull/186239
>From 4ef126d01fe4e249fefc487d83d779f48a4c367c Mon Sep 17 00:00:00 2001
From: Scott Linder <Scott.Linder at amd.com>
Date: Thu, 12 Mar 2026 16:29:22 +0000
Subject: [PATCH] [MCP][NFC] Opinionated refactoring
There are a few minor inconsistencies across the pass which I found mildly
distracting:
* The use of `Def`/`Dest`/`Dst` to refer to the same thing
* Inconsistent declaration order of `Dst`/`Src` vs `Src`/`Dst`
* Lots of `->getReg()->asMCReg()`, and uses of `Register` when the pass
is always running after RA anyway.
* Some places explicitly `assert(isCopyInstr)` while others just deref
the `optional`.
Standardize on `Dst`/`Src` to match the metaphor and ordering of
`DestSourcePair`.
Assume `std::optional::operator*` will assert in any reasonable
implementation, even though this may technically be undefined behavior.
When asserts are disabled it would be anyway.
The refactor uses structured bindings for a couple reasons:
* Naturally enforces consistent order of `Dst`-then-`Src`
* Requires the use of `auto`, which ensures the declaration is not
implicitly converting from `MCRegister` back to `Register`.
In both cases the explicitness of the name `getDstSrcMCRegs` hopefully
makes the meaning at the callsite clear (`Dst, Src = DstSrc`, and
explicitly mentioning `MCReg`).
Change-Id: Ic58f555e03535d726cdad38dbe3f9c6df1b86460
---
llvm/lib/CodeGen/MachineCopyPropagation.cpp | 357 +++++++++-----------
1 file changed, 163 insertions(+), 194 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index 41ab530bfb208..eb7e22366050f 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -94,9 +94,26 @@ static cl::opt<cl::boolOrDefault>
namespace {
-static std::optional<DestSourcePair> isCopyInstr(const MachineInstr &MI,
- const TargetInstrInfo &TII,
- bool UseCopyInstr) {
+MCRegister asPhysMCReg(const MachineOperand *Operand) {
+ Register Reg = Operand->getReg();
+ assert(Reg.isPhysical() &&
+ "MachineCopyPropagation should be run after register allocation!");
+ return Reg;
+}
+
+MCRegister getDstMCReg(const DestSourcePair &DSP) {
+ return asPhysMCReg(DSP.Destination);
+}
+MCRegister getSrcMCReg(const DestSourcePair &DSP) {
+ return asPhysMCReg(DSP.Source);
+}
+std::pair<MCRegister, MCRegister> getDstSrcMCRegs(const DestSourcePair &DSP) {
+ return {getDstMCReg(DSP), getSrcMCReg(DSP)};
+}
+
+std::optional<DestSourcePair> isCopyInstr(const MachineInstr &MI,
+ const TargetInstrInfo &TII,
+ bool UseCopyInstr) {
if (UseCopyInstr)
return TII.isCopyInstr(MI);
@@ -164,14 +181,12 @@ class CopyTracker {
// the subregisters used in the source of the COPY.
SmallSet<MCRegUnit, 8> RegUnitsToInvalidate;
auto InvalidateCopy = [&](MachineInstr *MI) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*MI, TII, UseCopyInstr);
- assert(CopyOperands && "Expect copy");
-
- auto Dest = TRI.regunits(CopyOperands->Destination->getReg().asMCReg());
- auto Src = TRI.regunits(CopyOperands->Source->getReg().asMCReg());
- RegUnitsToInvalidate.insert_range(Dest);
- RegUnitsToInvalidate.insert_range(Src);
+ DestSourcePair CopyOperands = *isCopyInstr(*MI, TII, UseCopyInstr);
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
+ auto DstUnits = TRI.regunits(Dst);
+ auto SrcUnits = TRI.regunits(Src);
+ RegUnitsToInvalidate.insert_range(DstUnits);
+ RegUnitsToInvalidate.insert_range(SrcUnits);
};
for (MCRegUnit Unit : TRI.regunits(Reg)) {
@@ -198,13 +213,10 @@ class CopyTracker {
// When we clobber the destination of a copy, we need to clobber the
// whole register it defined.
if (MachineInstr *MI = I->second.MI) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*MI, TII, UseCopyInstr);
-
- MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
- MCRegister Src = CopyOperands->Source->getReg().asMCReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*MI, TII, UseCopyInstr);
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
- markRegsUnavailable(Def, TRI);
+ markRegsUnavailable(Dst, TRI);
// Since we clobber the destination of a copy, the semantic of Src's
// "DefRegs" to contain Def is no longer effectual. We will also need
@@ -226,7 +238,7 @@ class CopyTracker {
// NOLINTNEXTLINE(llvm-qualified-auto)
for (auto Itr = SrcCopy->second.DefRegs.begin();
Itr != SrcCopy->second.DefRegs.end(); Itr++) {
- if (*Itr == Def) {
+ if (*Itr == Dst) {
SrcCopy->second.DefRegs.erase(Itr);
// If DefReg becomes empty after removal, we can remove the
// SrcCopy from the tracker's copy maps. We only remove those
@@ -266,9 +278,8 @@ class CopyTracker {
if (!AvailCopy)
return false;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*AvailCopy, TII, UseCopyInstr);
- Register Src = CopyOperands->Source->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*AvailCopy, TII, UseCopyInstr);
+ MCRegister Src = getSrcMCReg(CopyOperands);
// Bail out, if the source of the copy is not the same as the Reg.
if (Src != Reg)
@@ -295,23 +306,19 @@ class CopyTracker {
/// Add this copy's registers into the tracker's copy maps.
void trackCopy(MachineInstr *MI, const TargetRegisterInfo &TRI,
const TargetInstrInfo &TII, bool UseCopyInstr) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*MI, TII, UseCopyInstr);
- assert(CopyOperands && "Tracking non-copy?");
-
- MCRegister Src = CopyOperands->Source->getReg().asMCReg();
- MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*MI, TII, UseCopyInstr);
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
- // Remember Def is defined by the copy.
- for (MCRegUnit Unit : TRI.regunits(Def))
+ // Remember Dst is defined by the copy.
+ for (MCRegUnit Unit : TRI.regunits(Dst))
Copies[Unit] = {MI, nullptr, {}, {}, true};
- // Remember source that's copied to Def. Once it's clobbered, then
+ // Remember source that's copied to Dst. Once it's clobbered, then
// it's no longer available for copy propagation.
for (MCRegUnit Unit : TRI.regunits(Src)) {
auto &Copy = Copies[Unit];
- if (!is_contained(Copy.DefRegs, Def))
- Copy.DefRegs.push_back(Def);
+ if (!is_contained(Copy.DefRegs, Dst))
+ Copy.DefRegs.push_back(Dst);
Copy.LastSeenUseInCopy = MI;
}
}
@@ -352,10 +359,8 @@ class CopyTracker {
if (!AvailCopy)
return nullptr;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*AvailCopy, TII, UseCopyInstr);
- Register AvailSrc = CopyOperands->Source->getReg();
- Register AvailDef = CopyOperands->Destination->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*AvailCopy, TII, UseCopyInstr);
+ auto [AvailDst, AvailSrc] = getDstSrcMCRegs(CopyOperands);
if (!TRI.isSubRegisterEq(AvailSrc, Reg))
return nullptr;
@@ -363,8 +368,8 @@ class CopyTracker {
make_range(AvailCopy->getReverseIterator(), I.getReverseIterator()))
for (const MachineOperand &MO : MI.operands())
if (MO.isRegMask())
- // FIXME: Shall we simultaneously invalidate AvailSrc or AvailDef?
- if (MO.clobbersPhysReg(AvailSrc) || MO.clobbersPhysReg(AvailDef))
+ // FIXME: Shall we simultaneously invalidate AvailSrc or AvailDst?
+ if (MO.clobbersPhysReg(AvailSrc) || MO.clobbersPhysReg(AvailDst))
return nullptr;
return AvailCopy;
@@ -382,11 +387,9 @@ class CopyTracker {
if (!AvailCopy)
return nullptr;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*AvailCopy, TII, UseCopyInstr);
- Register AvailSrc = CopyOperands->Source->getReg();
- Register AvailDef = CopyOperands->Destination->getReg();
- if (!TRI.isSubRegisterEq(AvailDef, Reg))
+ DestSourcePair CopyOperands = *isCopyInstr(*AvailCopy, TII, UseCopyInstr);
+ auto [AvailDst, AvailSrc] = getDstSrcMCRegs(CopyOperands);
+ if (!TRI.isSubRegisterEq(AvailDst, Reg))
return nullptr;
// Check that the available copy isn't clobbered by any regmasks between
@@ -395,7 +398,7 @@ class CopyTracker {
make_range(AvailCopy->getIterator(), DestCopy.getIterator()))
for (const MachineOperand &MO : MI.operands())
if (MO.isRegMask())
- if (MO.clobbersPhysReg(AvailSrc) || MO.clobbersPhysReg(AvailDef))
+ if (MO.clobbersPhysReg(AvailSrc) || MO.clobbersPhysReg(AvailDst))
return nullptr;
return AvailCopy;
@@ -413,10 +416,9 @@ class CopyTracker {
return nullptr;
MachineInstr *DefCopy = CI->second.MI;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*DefCopy, TII, UseCopyInstr);
- Register Def = CopyOperands->Destination->getReg();
- if (!TRI.isSubRegisterEq(Def, Reg))
+ DestSourcePair CopyOperands = *isCopyInstr(*DefCopy, TII, UseCopyInstr);
+ MCRegister Dst = getDstMCReg(CopyOperands);
+ if (!TRI.isSubRegisterEq(Dst, Reg))
return nullptr;
for (const MachineInstr &MI :
@@ -424,9 +426,9 @@ class CopyTracker {
Current.getIterator()))
for (const MachineOperand &MO : MI.operands())
if (MO.isRegMask())
- if (MO.clobbersPhysReg(Def)) {
+ if (MO.clobbersPhysReg(Dst)) {
LLVM_DEBUG(dbgs() << "MCP: Removed tracking of "
- << printReg(Def, &TRI) << "\n");
+ << printReg(Dst, &TRI) << "\n");
return nullptr;
}
@@ -470,7 +472,7 @@ class MachineCopyPropagation {
void forwardCopyPropagateBlock(MachineBasicBlock &MBB);
void backwardCopyPropagateBlock(MachineBasicBlock &MBB);
void eliminateSpillageCopies(MachineBasicBlock &MBB);
- bool eraseIfRedundant(MachineInstr &Copy, MCRegister Src, MCRegister Def);
+ bool eraseIfRedundant(MachineInstr &Copy, MCRegister Dst, MCRegister Src);
void forwardUses(MachineInstr &MI);
void propagateDefs(MachineInstr &MI);
bool isForwardableRegClassCopy(const MachineInstr &Copy,
@@ -571,66 +573,62 @@ void MachineCopyPropagation::readSuccessorLiveIns(
}
}
-/// Return true if \p PreviousCopy did copy register \p Src to register \p Def.
+/// Return true if \p PreviousCopy did copy register \p Src to register \p Dst.
/// This fact may have been obscured by sub register usage or may not be true at
-/// all even though Src and Def are subregisters of the registers used in
+/// all even though Src and Dst are subregisters of the registers used in
/// PreviousCopy. e.g.
/// isNopCopy("ecx = COPY eax", AX, CX) == true
/// isNopCopy("ecx = COPY eax", AH, CL) == false
static bool isNopCopy(const MachineInstr &PreviousCopy, MCRegister Src,
- MCRegister Def, const TargetRegisterInfo *TRI,
+ MCRegister Dst, const TargetRegisterInfo *TRI,
const TargetInstrInfo *TII, bool UseCopyInstr) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(PreviousCopy, *TII, UseCopyInstr);
- MCRegister PreviousSrc = CopyOperands->Source->getReg().asMCReg();
- MCRegister PreviousDef = CopyOperands->Destination->getReg().asMCReg();
- if (Src == PreviousSrc && Def == PreviousDef)
+ DestSourcePair CopyOperands = *isCopyInstr(PreviousCopy, *TII, UseCopyInstr);
+ auto [PreviousDst, PreviousSrc] = getDstSrcMCRegs(CopyOperands);
+ if (Src == PreviousSrc && Dst == PreviousDst)
return true;
if (!TRI->isSubRegister(PreviousSrc, Src))
return false;
unsigned SubIdx = TRI->getSubRegIndex(PreviousSrc, Src);
- return SubIdx == TRI->getSubRegIndex(PreviousDef, Def);
+ return SubIdx == TRI->getSubRegIndex(PreviousDst, Dst);
}
/// Remove instruction \p Copy if there exists a previous copy that copies the
-/// register \p Src to the register \p Def; This may happen indirectly by
+/// register \p Src to the register \p Dst; This may happen indirectly by
/// copying the super registers.
bool MachineCopyPropagation::eraseIfRedundant(MachineInstr &Copy,
- MCRegister Src, MCRegister Def) {
- if (isNeverRedundant(Src) || isNeverRedundant(Def))
+ MCRegister Dst, MCRegister Src) {
+ if (isNeverRedundant(Src) || isNeverRedundant(Dst))
return false;
// Search for an existing copy.
MachineInstr *PrevCopy =
- Tracker.findAvailCopy(Copy, Def, *TRI, *TII, UseCopyInstr);
+ Tracker.findAvailCopy(Copy, Dst, *TRI, *TII, UseCopyInstr);
if (!PrevCopy)
return false;
- auto PrevCopyOperands = isCopyInstr(*PrevCopy, *TII, UseCopyInstr);
+ DestSourcePair PrevCopyOperands = *isCopyInstr(*PrevCopy, *TII, UseCopyInstr);
// Check that the existing copy uses the correct sub registers.
- if (PrevCopyOperands->Destination->isDead())
+ if (PrevCopyOperands.Destination->isDead())
return false;
- if (!isNopCopy(*PrevCopy, Src, Def, TRI, TII, UseCopyInstr))
+ if (!isNopCopy(*PrevCopy, Src, Dst, TRI, TII, UseCopyInstr))
return false;
LLVM_DEBUG(dbgs() << "MCP: copy is a NOP, removing: "; Copy.dump());
- // Copy was redundantly redefining either Src or Def. Remove earlier kill
+ // Copy was redundantly redefining either Src or Dst. Remove earlier kill
// flags between Copy and PrevCopy because the value will be reused now.
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(Copy, *TII, UseCopyInstr);
- assert(CopyOperands);
+ DestSourcePair CopyOperands = *isCopyInstr(Copy, *TII, UseCopyInstr);
- MCRegister CopyDef = CopyOperands->Destination->getReg().asMCReg();
- assert(CopyDef == Src || CopyDef == Def);
+ MCRegister CopyDst = getDstMCReg(CopyOperands);
+ assert(CopyDst == Src || CopyDst == Dst);
for (MachineInstr &MI :
make_range(PrevCopy->getIterator(), Copy.getIterator()))
- MI.clearRegisterKills(CopyDef, TRI);
+ MI.clearRegisterKills(CopyDst, TRI);
// Clear undef flag from remaining copy if needed.
- if (!CopyOperands->Source->isUndef()) {
- PrevCopy->getOperand(PrevCopyOperands->Source->getOperandNo())
+ if (!CopyOperands.Source->isUndef()) {
+ PrevCopy->getOperand(PrevCopyOperands.Source->getOperandNo())
.setIsUndef(false);
}
@@ -642,13 +640,12 @@ bool MachineCopyPropagation::eraseIfRedundant(MachineInstr &Copy,
bool MachineCopyPropagation::isBackwardPropagatableRegClassCopy(
const MachineInstr &Copy, const MachineInstr &UseI, unsigned UseIdx) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(Copy, *TII, UseCopyInstr);
- Register Def = CopyOperands->Destination->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(Copy, *TII, UseCopyInstr);
+ MCRegister Dst = getDstMCReg(CopyOperands);
if (const TargetRegisterClass *URC =
UseI.getRegClassConstraint(UseIdx, TII, TRI))
- return URC->contains(Def);
+ return URC->contains(Dst);
// We don't process further if UseI is a COPY, since forward copy propagation
// should handle that.
@@ -657,13 +654,12 @@ bool MachineCopyPropagation::isBackwardPropagatableRegClassCopy(
bool MachineCopyPropagation::isBackwardPropagatableCopy(
const MachineInstr &Copy, const DestSourcePair &CopyOperands) {
- MCRegister Def = CopyOperands.Destination->getReg().asMCReg();
- MCRegister Src = CopyOperands.Source->getReg().asMCReg();
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
- if (!Def || !Src)
+ if (!Dst || !Src)
return false;
- if (isNeverRedundant(Def) || isNeverRedundant(Src))
+ if (isNeverRedundant(Src))
return false;
return CopyOperands.Source->isRenamable() && CopyOperands.Source->isKill();
@@ -675,17 +671,17 @@ bool MachineCopyPropagation::isBackwardPropagatableCopy(
bool MachineCopyPropagation::isForwardableRegClassCopy(const MachineInstr &Copy,
const MachineInstr &UseI,
unsigned UseIdx) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(Copy, *TII, UseCopyInstr);
- Register CopySrcReg = CopyOperands->Source->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(Copy, *TII, UseCopyInstr);
+ MCRegister CopySrc = getSrcMCReg(CopyOperands);
// If the new register meets the opcode register constraints, then allow
// forwarding.
if (const TargetRegisterClass *URC =
UseI.getRegClassConstraint(UseIdx, TII, TRI))
- return URC->contains(CopySrcReg);
+ return URC->contains(CopySrc);
- auto UseICopyOperands = isCopyInstr(UseI, *TII, UseCopyInstr);
+ std::optional<DestSourcePair> UseICopyOperands =
+ isCopyInstr(UseI, *TII, UseCopyInstr);
if (!UseICopyOperands)
return false;
@@ -709,11 +705,11 @@ bool MachineCopyPropagation::isForwardableRegClassCopy(const MachineInstr &Copy,
// Allow forwarding if src and dst belong to any common class, so long as they
// don't belong to any (possibly smaller) common class that requires copies to
// go via a different class.
- Register UseDstReg = UseICopyOperands->Destination->getReg();
+ MCRegister UseDst = getDstMCReg(*UseICopyOperands);
bool Found = false;
bool IsCrossClass = false;
for (const TargetRegisterClass *RC : TRI->regclasses()) {
- if (RC->contains(CopySrcReg) && RC->contains(UseDstReg)) {
+ if (RC->contains(CopySrc) && RC->contains(UseDst)) {
Found = true;
if (TRI->getCrossCopyRegClass(RC) != RC) {
IsCrossClass = true;
@@ -727,9 +723,9 @@ bool MachineCopyPropagation::isForwardableRegClassCopy(const MachineInstr &Copy,
return true;
// The forwarded copy would be cross-class. Only do this if the original copy
// was also cross-class.
- Register CopyDstReg = CopyOperands->Destination->getReg();
+ MCRegister CopyDst = getDstMCReg(CopyOperands);
for (const TargetRegisterClass *RC : TRI->regclasses()) {
- if (RC->contains(CopySrcReg) && RC->contains(CopyDstReg) &&
+ if (RC->contains(CopySrc) && RC->contains(CopyDst) &&
TRI->getCrossCopyRegClass(RC) != RC)
return true;
}
@@ -825,20 +821,18 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
if (!Copy)
continue;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*Copy, *TII, UseCopyInstr);
- MCRegister CopyDstReg = CopyOperands->Destination->getReg().asMCReg();
- const MachineOperand &CopySrc = *CopyOperands->Source;
- MCRegister CopySrcReg = CopySrc.getReg().asMCReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*Copy, *TII, UseCopyInstr);
+ auto [CopyDst, CopySrc] = getDstSrcMCRegs(CopyOperands);
+ const MachineOperand &CopySrcOperand = *CopyOperands.Source;
- MCRegister ForwardedReg = CopySrcReg;
+ MCRegister ForwardedReg = CopySrc;
// MI might use a sub-register of the Copy destination, in which case the
// forwarded register is the matching sub-register of the Copy source.
- if (MOUse.getReg() != CopyDstReg) {
- unsigned SubRegIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
+ if (MOUse.getReg() != CopyDst) {
+ unsigned SubRegIdx = TRI->getSubRegIndex(CopyDst, MOUse.getReg());
assert(SubRegIdx &&
"MI source is not a sub-register of Copy destination");
- ForwardedReg = TRI->getSubReg(CopySrcReg, SubRegIdx);
+ ForwardedReg = TRI->getSubReg(CopySrc, SubRegIdx);
if (!ForwardedReg || TRI->isArtificial(ForwardedReg)) {
LLVM_DEBUG(dbgs() << "MCP: Copy source does not have sub-register "
<< TRI->getSubRegIndexName(SubRegIdx) << '\n');
@@ -847,7 +841,7 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
}
// Don't forward COPYs of reserved regs unless they are constant.
- if (MRI->isReserved(CopySrcReg) && !MRI->isConstantPhysReg(CopySrcReg))
+ if (MRI->isReserved(CopySrc) && !MRI->isConstantPhysReg(CopySrc))
continue;
if (!isForwardableRegClassCopy(*Copy, MI, OpIdx))
@@ -860,8 +854,8 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
// original copy source that we are about to use. The tracker mechanism
// cannot cope with that.
if (isCopyInstr(MI, *TII, UseCopyInstr) &&
- MI.modifiesRegister(CopySrcReg, TRI) &&
- !MI.definesRegister(CopySrcReg, /*TRI=*/nullptr)) {
+ MI.modifiesRegister(CopySrc, TRI) &&
+ !MI.definesRegister(CopySrc, /*TRI=*/nullptr)) {
LLVM_DEBUG(dbgs() << "MCP: Copy source overlap with dest in " << MI);
continue;
}
@@ -878,16 +872,16 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
MOUse.setReg(ForwardedReg);
- if (!CopySrc.isRenamable())
+ if (!CopySrcOperand.isRenamable())
MOUse.setIsRenamable(false);
- MOUse.setIsUndef(CopySrc.isUndef());
+ MOUse.setIsUndef(CopySrcOperand.isUndef());
LLVM_DEBUG(dbgs() << "MCP: After replacement: " << MI << "\n");
// Clear kill markers that may have been invalidated.
for (MachineInstr &KMI :
make_range(Copy->getIterator(), std::next(MI.getIterator())))
- KMI.clearRegisterKills(CopySrcReg, TRI);
+ KMI.clearRegisterKills(CopySrc, TRI);
++NumCopyForwards;
Changed = true;
@@ -903,15 +897,8 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
std::optional<DestSourcePair> CopyOperands =
isCopyInstr(MI, *TII, UseCopyInstr);
if (CopyOperands) {
- Register RegSrc = CopyOperands->Source->getReg();
- Register RegDef = CopyOperands->Destination->getReg();
- if (!TRI->regsOverlap(RegDef, RegSrc)) {
- assert(RegDef.isPhysical() && RegSrc.isPhysical() &&
- "MachineCopyPropagation should be run after register allocation!");
-
- MCRegister Def = RegDef.asMCReg();
- MCRegister Src = RegSrc.asMCReg();
-
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
+ if (!TRI->regsOverlap(Dst, Src)) {
// The two copies cancel out and the source of the first copy
// hasn't been overridden, eliminate the second one. e.g.
// %ecx = COPY %eax
@@ -927,7 +914,7 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
// %ecx = COPY %eax
// =>
// %ecx = COPY %eax
- if (eraseIfRedundant(MI, Def, Src) || eraseIfRedundant(MI, Src, Def))
+ if (eraseIfRedundant(MI, Dst, Src) || eraseIfRedundant(MI, Src, Dst))
continue;
}
}
@@ -955,15 +942,11 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
if (CopyOperands) {
- Register RegSrc = CopyOperands->Source->getReg();
- Register RegDef = CopyOperands->Destination->getReg();
-
- if (!TRI->regsOverlap(RegDef, RegSrc)) {
- // Copy is now a candidate for deletion.
- MCRegister Def = RegDef.asMCReg();
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
+ if (!TRI->regsOverlap(Dst, Src)) {
// FIXME: Document why this does not consider `RegSrc`, similar to how
// `backwardCopyPropagateBlock` does.
- if (!isNeverRedundant(Def))
+ if (!isNeverRedundant(Dst))
MaybeDeadCopies.insert(&MI);
}
}
@@ -979,7 +962,7 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
if (!Reg)
continue;
- assert(!Reg.isVirtual() &&
+ assert(Reg.isPhysical() &&
"MachineCopyPropagation should be run after register allocation!");
if (MO.isDef() && !MO.isEarlyClobber()) {
@@ -1050,9 +1033,8 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
if (CopyOperands) {
- Register RegSrc = CopyOperands->Source->getReg();
- Register RegDef = CopyOperands->Destination->getReg();
- if (!TRI->regsOverlap(RegDef, RegSrc)) {
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
+ if (!TRI->regsOverlap(Dst, Src)) {
Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
}
}
@@ -1073,20 +1055,17 @@ void MachineCopyPropagation::forwardCopyPropagateBlock(MachineBasicBlock &MBB) {
LLVM_DEBUG(dbgs() << "MCP: Removing copy due to no live-out succ: ";
MaybeDead->dump());
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*MaybeDead, *TII, UseCopyInstr);
- assert(CopyOperands);
+ DestSourcePair CopyOperands =
+ *isCopyInstr(*MaybeDead, *TII, UseCopyInstr);
- Register SrcReg = CopyOperands->Source->getReg();
- Register DestReg = CopyOperands->Destination->getReg();
- assert(!isNeverRedundant(DestReg));
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
+ assert(!isNeverRedundant(Dst));
// Update matching debug values, if any.
const auto &DbgUsers = CopyDbgUsers[MaybeDead];
SmallVector<MachineInstr *> MaybeDeadDbgUsers(DbgUsers.begin(),
DbgUsers.end());
- MRI->updateDbgUsersToReg(DestReg.asMCReg(), SrcReg.asMCReg(),
- MaybeDeadDbgUsers);
+ MRI->updateDbgUsersToReg(Dst, Src, MaybeDeadDbgUsers);
MaybeDead->eraseFromParent();
Changed = true;
@@ -1126,10 +1105,8 @@ void MachineCopyPropagation::propagateDefs(MachineInstr &MI) {
if (!Copy)
continue;
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*Copy, *TII, UseCopyInstr);
- Register Def = CopyOperands->Destination->getReg();
- Register Src = CopyOperands->Source->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*Copy, *TII, UseCopyInstr);
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
if (MODef.getReg() != Src)
continue;
@@ -1140,25 +1117,25 @@ void MachineCopyPropagation::propagateDefs(MachineInstr &MI) {
if (hasImplicitOverlap(MI, MODef))
continue;
- if (hasOverlappingMultipleDef(MI, MODef, Def))
+ if (hasOverlappingMultipleDef(MI, MODef, Dst))
continue;
- if (!canUpdateSrcUsers(*Copy, *CopyOperands->Source))
+ if (!canUpdateSrcUsers(*Copy, *CopyOperands.Source))
continue;
LLVM_DEBUG(dbgs() << "MCP: Replacing " << printReg(MODef.getReg(), TRI)
- << "\n with " << printReg(Def, TRI) << "\n in "
+ << "\n with " << printReg(Dst, TRI) << "\n in "
<< MI << " from " << *Copy);
- MODef.setReg(Def);
- MODef.setIsRenamable(CopyOperands->Destination->isRenamable());
+ MODef.setReg(Dst);
+ MODef.setIsRenamable(CopyOperands.Destination->isRenamable());
for (auto *SrcUser : Tracker.getSrcUsers(Src, *TRI)) {
for (MachineOperand &MO : SrcUser->uses()) {
if (!MO.isReg() || !MO.isUse() || MO.getReg() != Src)
continue;
- MO.setReg(Def);
- MO.setIsRenamable(CopyOperands->Destination->isRenamable());
+ MO.setReg(Dst);
+ MO.setIsRenamable(CopyOperands.Destination->isRenamable());
}
}
@@ -1179,17 +1156,14 @@ void MachineCopyPropagation::backwardCopyPropagateBlock(
std::optional<DestSourcePair> CopyOperands =
isCopyInstr(MI, *TII, UseCopyInstr);
if (CopyOperands && MI.getNumImplicitOperands() == 0) {
- Register DefReg = CopyOperands->Destination->getReg();
- Register SrcReg = CopyOperands->Source->getReg();
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
- if (!TRI->regsOverlap(DefReg, SrcReg)) {
+ if (!TRI->regsOverlap(Dst, Src)) {
// Unlike forward cp, we don't invoke propagateDefs here,
// just let forward cp do COPY-to-COPY propagation.
if (isBackwardPropagatableCopy(MI, *CopyOperands)) {
- Tracker.invalidateRegister(SrcReg.asMCReg(), *TRI, *TII,
- UseCopyInstr);
- Tracker.invalidateRegister(DefReg.asMCReg(), *TRI, *TII,
- UseCopyInstr);
+ Tracker.invalidateRegister(Src, *TRI, *TII, UseCopyInstr);
+ Tracker.invalidateRegister(Dst, *TRI, *TII, UseCopyInstr);
Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
continue;
}
@@ -1238,15 +1212,13 @@ void MachineCopyPropagation::backwardCopyPropagateBlock(
}
for (auto *Copy : MaybeDeadCopies) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*Copy, *TII, UseCopyInstr);
- Register Src = CopyOperands->Source->getReg();
- Register Def = CopyOperands->Destination->getReg();
+ DestSourcePair CopyOperands = *isCopyInstr(*Copy, *TII, UseCopyInstr);
+ auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
const auto &DbgUsers = CopyDbgUsers[Copy];
SmallVector<MachineInstr *> MaybeDeadDbgUsers(DbgUsers.begin(),
DbgUsers.end());
- MRI->updateDbgUsersToReg(Src.asMCReg(), Def.asMCReg(), MaybeDeadDbgUsers);
+ MRI->updateDbgUsersToReg(Src, Dst, MaybeDeadDbgUsers);
Copy->eraseFromParent();
++NumDeletes;
}
@@ -1288,8 +1260,8 @@ void MachineCopyPropagation::backwardCopyPropagateBlock(
// conservatively keep its value as it was before the rewrite.
//
// The algorithm is trying to keep
-// property#1: No Def of spill COPY in the chain is used or defined until the
-// paired reload COPY in the chain uses the Def.
+// property#1: No Dst of spill COPY in the chain is used or defined until the
+// paired reload COPY in the chain uses the Dst.
//
// property#2: NO Source of COPY in the chain is used or defined until the next
// COPY in the chain defines the Source, except the innermost spill-reload
@@ -1299,7 +1271,7 @@ void MachineCopyPropagation::backwardCopyPropagateBlock(
// the COPY is a reload COPY, then try to find paired spill COPY by searching
// the COPY defines the Src of the reload COPY backward. If such pair is found,
// it either belongs to an existing chain or a new chain depends on
-// last available COPY uses the Def of the reload COPY.
+// last available COPY uses the Dst of the reload COPY.
// Implementation notes, we use CopyTracker::findLastDefCopy(Reg, ...) to find
// out last COPY that defines Reg; we use CopyTracker::findLastUseCopy(Reg, ...)
// to find out last COPY that uses Reg. When we are encountered with a Non-COPY
@@ -1345,9 +1317,9 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
if (CopySourceInvalid.count(Reload))
return;
- auto CheckCopyConstraint = [this](Register Def, Register Src) {
+ auto CheckCopyConstraint = [this](Register Dst, Register Src) {
for (const TargetRegisterClass *RC : TRI->regclasses()) {
- if (RC->contains(Def) && RC->contains(Src))
+ if (RC->contains(Dst) && RC->contains(Src))
return true;
}
return false;
@@ -1361,26 +1333,26 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
}
};
- std::optional<DestSourcePair> InnerMostSpillCopy =
- isCopyInstr(*SC[0], *TII, UseCopyInstr);
- std::optional<DestSourcePair> OuterMostSpillCopy =
- isCopyInstr(*SC.back(), *TII, UseCopyInstr);
- std::optional<DestSourcePair> InnerMostReloadCopy =
- isCopyInstr(*RC[0], *TII, UseCopyInstr);
- std::optional<DestSourcePair> OuterMostReloadCopy =
- isCopyInstr(*RC.back(), *TII, UseCopyInstr);
- if (!CheckCopyConstraint(OuterMostSpillCopy->Source->getReg(),
- InnerMostSpillCopy->Source->getReg()) ||
- !CheckCopyConstraint(InnerMostReloadCopy->Destination->getReg(),
- OuterMostReloadCopy->Destination->getReg()))
+ DestSourcePair InnerMostSpillCopy =
+ *isCopyInstr(*SC[0], *TII, UseCopyInstr);
+ DestSourcePair OuterMostSpillCopy =
+ *isCopyInstr(*SC.back(), *TII, UseCopyInstr);
+ DestSourcePair InnerMostReloadCopy =
+ *isCopyInstr(*RC[0], *TII, UseCopyInstr);
+ DestSourcePair OuterMostReloadCopy =
+ *isCopyInstr(*RC.back(), *TII, UseCopyInstr);
+ if (!CheckCopyConstraint(getSrcMCReg(OuterMostSpillCopy),
+ getSrcMCReg(InnerMostSpillCopy)) ||
+ !CheckCopyConstraint(getDstMCReg(InnerMostReloadCopy),
+ getDstMCReg(OuterMostReloadCopy)))
return;
SpillageChainsLength += SC.size() + RC.size();
NumSpillageChains += 1;
- UpdateReg(SC[0], InnerMostSpillCopy->Destination,
- OuterMostSpillCopy->Source);
- UpdateReg(RC[0], InnerMostReloadCopy->Source,
- OuterMostReloadCopy->Destination);
+ UpdateReg(SC[0], InnerMostSpillCopy.Destination,
+ OuterMostSpillCopy.Source);
+ UpdateReg(RC[0], InnerMostReloadCopy.Source,
+ OuterMostReloadCopy.Destination);
for (size_t I = 1; I < SC.size() - 1; ++I) {
SC[I]->eraseFromParent();
@@ -1396,9 +1368,8 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
isCopyInstr(MaybeCopy, *TII, UseCopyInstr);
if (!CopyOperands)
return false;
- Register Src = CopyOperands->Source->getReg();
- Register Def = CopyOperands->Destination->getReg();
- return Src && Def && !TRI->regsOverlap(Src, Def) &&
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
+ return Src && Dst && !TRI->regsOverlap(Src, Dst) &&
CopyOperands->Source->isRenamable() &&
CopyOperands->Destination->isRenamable();
};
@@ -1413,8 +1384,8 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
isCopyInstr(Reload, *TII, UseCopyInstr);
if (!SpillCopy || !ReloadCopy)
return false;
- return SpillCopy->Source->getReg() == ReloadCopy->Destination->getReg() &&
- SpillCopy->Destination->getReg() == ReloadCopy->Source->getReg();
+ return getSrcMCReg(*SpillCopy) == getDstMCReg(*ReloadCopy) &&
+ getDstMCReg(*SpillCopy) == getSrcMCReg(*ReloadCopy);
};
auto IsChainedCopy = [&, this](const MachineInstr &Prev,
@@ -1427,7 +1398,7 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
isCopyInstr(Current, *TII, UseCopyInstr);
if (!PrevCopy || !CurrentCopy)
return false;
- return PrevCopy->Source->getReg() == CurrentCopy->Destination->getReg();
+ return getSrcMCReg(*PrevCopy) == getDstMCReg(*CurrentCopy);
};
for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
@@ -1471,13 +1442,12 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
continue;
}
- Register Src = CopyOperands->Source->getReg();
- Register Def = CopyOperands->Destination->getReg();
+ auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
// Check if we can find a pair spill-reload copy.
LLVM_DEBUG(dbgs() << "MCP: Searching paired spill for reload: ");
LLVM_DEBUG(MI.dump());
MachineInstr *MaybeSpill =
- Tracker.findLastSeenDefInCopy(MI, Src.asMCReg(), *TRI, *TII, UseCopyInstr);
+ Tracker.findLastSeenDefInCopy(MI, Src, *TRI, *TII, UseCopyInstr);
bool MaybeSpillIsChained = ChainLeader.count(MaybeSpill);
if (!MaybeSpillIsChained && MaybeSpill &&
IsSpillReloadPair(*MaybeSpill, MI)) {
@@ -1515,8 +1485,7 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
// L2 and L5 to this chain.
LLVM_DEBUG(dbgs() << "MCP: Found spill: ");
LLVM_DEBUG(MaybeSpill->dump());
- MachineInstr *MaybePrevReload =
- Tracker.findLastSeenUseInCopy(Def.asMCReg(), *TRI);
+ MachineInstr *MaybePrevReload = Tracker.findLastSeenUseInCopy(Dst, *TRI);
auto Leader = ChainLeader.find(MaybePrevReload);
MachineInstr *L = nullptr;
if (Leader == ChainLeader.end() ||
@@ -1544,7 +1513,7 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
// MaybeSpill is unable to pair with MI. That's to say adding MI makes
// the chain invalid.
// The COPY defines Src is no longer considered as a candidate of a
- // valid chain. Since we expect the Def of a spill copy isn't used by
+ // valid chain. Since we expect the Dst of a spill copy isn't used by
// any COPY instruction until a reload copy. For example:
// L1: r1 = COPY r2
// L2: r3 = COPY r1
@@ -1557,7 +1526,7 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
LLVM_DEBUG(dbgs() << "MCP: Not paired spill-reload:\n");
LLVM_DEBUG(MaybeSpill->dump());
LLVM_DEBUG(MI.dump());
- Tracker.clobberRegister(Src.asMCReg(), *TRI, *TII, UseCopyInstr);
+ Tracker.clobberRegister(Src, *TRI, *TII, UseCopyInstr);
LLVM_DEBUG(dbgs() << "MCP: Removed tracking of " << printReg(Src, TRI)
<< "\n");
}
More information about the llvm-branch-commits
mailing list