[llvm] [CodeGen] Introduce a RegisterUnit class to hold virtual reg or physical reg unit. NFC (PR #123768)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 21 07:41:13 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-regalloc
Author: Craig Topper (topperc)
<details>
<summary>Changes</summary>
LiveIntervals and MachineVerifier were previously using Register to store this, but reg units are different than physical registers. One important difference is that 0 is a valid reg unit number, but it is not a valid phyiscal register.
This patch introduces a new RegisterUnit class that is distinct from Register. It can be be converted to/from a virtual Register or a MCRegUnit. I've made all conversions explicit and used assertions to check the validity. The new class is in Register.h, but I can move it.
I also fixed a place in MachineVerifier that was ignoring reg unit 0.
---
Patch is 26.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123768.diff
4 Files Affected:
- (modified) llvm/include/llvm/CodeGen/Register.h (+31)
- (modified) llvm/include/llvm/CodeGen/TargetRegisterInfo.h (+2-2)
- (modified) llvm/lib/CodeGen/LiveIntervals.cpp (+13-12)
- (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+89-85)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/Register.h b/llvm/include/llvm/CodeGen/Register.h
index fac5f00110ef78..cb0db2bae928bc 100644
--- a/llvm/include/llvm/CodeGen/Register.h
+++ b/llvm/include/llvm/CodeGen/Register.h
@@ -160,6 +160,37 @@ template <> struct DenseMapInfo<Register> {
}
};
+/// Wrapper class representing a virtual register or register unit.
+class RegisterUnit {
+ unsigned RegUnit;
+
+public:
+ constexpr explicit RegisterUnit(MCRegUnit Unit) : RegUnit(Unit) {
+ assert(!Register::isVirtualRegister(RegUnit));
+ }
+ constexpr explicit RegisterUnit(Register Reg) : RegUnit(Reg.id()) {
+ assert(Reg.isVirtual());
+ }
+
+ constexpr bool isVirtual() const {
+ return Register::isVirtualRegister(RegUnit);
+ }
+
+ constexpr MCRegUnit asMCRegUnit() const {
+ assert(!isVirtual() && "Not a register unit");
+ return RegUnit;
+ }
+
+ constexpr Register asVirtualReg() const {
+ assert(isVirtual() && "Not a virtual register");
+ return Register(RegUnit);
+ }
+
+ constexpr bool operator==(const RegisterUnit &Other) const {
+ return RegUnit == Other.RegUnit;
+ }
+};
+
} // namespace llvm
#endif // LLVM_CODEGEN_REGISTER_H
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 0bf72637de3989..63460f5a0dae36 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -466,9 +466,9 @@ class TargetRegisterInfo : public MCRegisterInfo {
}
/// Returns true if Reg contains RegUnit.
- bool hasRegUnit(MCRegister Reg, Register RegUnit) const {
+ bool hasRegUnit(MCRegister Reg, MCRegUnit RegUnit) const {
for (MCRegUnit Unit : regunits(Reg))
- if (Register(Unit) == RegUnit)
+ if (Unit == RegUnit)
return true;
return false;
}
diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index f38527a3ce6a31..530bcac2be33d4 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/llvm/lib/CodeGen/LiveIntervals.cpp
@@ -1066,10 +1066,10 @@ class LiveIntervals::HMEditor {
for (LiveInterval::SubRange &S : LI.subranges()) {
if ((S.LaneMask & LaneMask).none())
continue;
- updateRange(S, Reg, S.LaneMask);
+ updateRange(S, RegisterUnit(Reg), S.LaneMask);
}
}
- updateRange(LI, Reg, LaneBitmask::getNone());
+ updateRange(LI, RegisterUnit(Reg), LaneBitmask::getNone());
// If main range has a hole and we are moving a subrange use across
// the hole updateRange() cannot properly handle it since it only
// gets the LiveRange and not the whole LiveInterval. As a result
@@ -1096,7 +1096,7 @@ class LiveIntervals::HMEditor {
// precomputed live range.
for (MCRegUnit Unit : TRI.regunits(Reg.asMCReg()))
if (LiveRange *LR = getRegUnitLI(Unit))
- updateRange(*LR, Unit, LaneBitmask::getNone());
+ updateRange(*LR, RegisterUnit(Unit), LaneBitmask::getNone());
}
if (hasRegMask)
updateRegMaskSlots();
@@ -1105,17 +1105,17 @@ class LiveIntervals::HMEditor {
private:
/// Update a single live range, assuming an instruction has been moved from
/// OldIdx to NewIdx.
- void updateRange(LiveRange &LR, Register Reg, LaneBitmask LaneMask) {
+ void updateRange(LiveRange &LR, RegisterUnit Reg, LaneBitmask LaneMask) {
if (!Updated.insert(&LR).second)
return;
LLVM_DEBUG({
dbgs() << " ";
if (Reg.isVirtual()) {
- dbgs() << printReg(Reg);
+ dbgs() << printReg(Reg.asVirtualReg());
if (LaneMask.any())
dbgs() << " L" << PrintLaneMask(LaneMask);
} else {
- dbgs() << printRegUnit(Reg, &TRI);
+ dbgs() << printRegUnit(Reg.asMCRegUnit(), &TRI);
}
dbgs() << ":\t" << LR << '\n';
});
@@ -1302,7 +1302,7 @@ class LiveIntervals::HMEditor {
/// Update LR to reflect an instruction has been moved upwards from OldIdx
/// to NewIdx (NewIdx < OldIdx).
- void handleMoveUp(LiveRange &LR, Register Reg, LaneBitmask LaneMask) {
+ void handleMoveUp(LiveRange &LR, RegisterUnit RegUnit, LaneBitmask LaneMask) {
LiveRange::iterator E = LR.end();
// Segment going into OldIdx.
LiveRange::iterator OldIdxIn = LR.find(OldIdx.getBaseIndex());
@@ -1326,7 +1326,7 @@ class LiveIntervals::HMEditor {
SlotIndex DefBeforeOldIdx
= std::max(OldIdxIn->start.getDeadSlot(),
NewIdx.getRegSlot(OldIdxIn->end.isEarlyClobber()));
- OldIdxIn->end = findLastUseBefore(DefBeforeOldIdx, Reg, LaneMask);
+ OldIdxIn->end = findLastUseBefore(DefBeforeOldIdx, RegUnit, LaneMask);
// Did we have a Def at OldIdx? If not we are done now.
OldIdxOut = std::next(OldIdxIn);
@@ -1484,11 +1484,12 @@ class LiveIntervals::HMEditor {
}
// Return the last use of reg between NewIdx and OldIdx.
- SlotIndex findLastUseBefore(SlotIndex Before, Register Reg,
+ SlotIndex findLastUseBefore(SlotIndex Before, RegisterUnit RegUnit,
LaneBitmask LaneMask) {
- if (Reg.isVirtual()) {
+ if (RegUnit.isVirtual()) {
SlotIndex LastUse = Before;
- for (MachineOperand &MO : MRI.use_nodbg_operands(Reg)) {
+ for (MachineOperand &MO :
+ MRI.use_nodbg_operands(RegUnit.asVirtualReg())) {
if (MO.isUndef())
continue;
unsigned SubReg = MO.getSubReg();
@@ -1531,7 +1532,7 @@ class LiveIntervals::HMEditor {
// Check if MII uses Reg.
for (MIBundleOperands MO(*MII); MO.isValid(); ++MO)
if (MO->isReg() && !MO->isUndef() && MO->getReg().isPhysical() &&
- TRI.hasRegUnit(MO->getReg(), Reg))
+ TRI.hasRegUnit(MO->getReg(), RegUnit.asMCRegUnit()))
return Idx.getRegSlot();
}
// Didn't reach Before. It must be the first instruction in the block.
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index d41b11307e7bc0..f4e6b42a9bbf26 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -313,7 +313,7 @@ struct MachineVerifier {
void report(const Twine &Msg, const MachineInstr *MI);
void report_context(const LiveInterval &LI) const;
- void report_context(const LiveRange &LR, Register VRegUnit,
+ void report_context(const LiveRange &LR, RegisterUnit VRegUnit,
LaneBitmask LaneMask) const;
void report_context(const LiveRange::Segment &S) const;
void report_context(const VNInfo &VNI) const;
@@ -322,18 +322,18 @@ struct MachineVerifier {
void report_context_liverange(const LiveRange &LR) const;
void report_context_lanemask(LaneBitmask LaneMask) const;
void report_context_vreg(Register VReg) const;
- void report_context_vreg_regunit(Register VRegOrUnit) const;
+ void report_context_vreg_regunit(RegisterUnit VRegOrUnit) const;
void verifyInlineAsm(const MachineInstr *MI);
void checkLiveness(const MachineOperand *MO, unsigned MONum);
void checkLivenessAtUse(const MachineOperand *MO, unsigned MONum,
SlotIndex UseIdx, const LiveRange &LR,
- Register VRegOrUnit,
+ RegisterUnit VRegOrUnit,
LaneBitmask LaneMask = LaneBitmask::getNone());
void checkLivenessAtDef(const MachineOperand *MO, unsigned MONum,
SlotIndex DefIdx, const LiveRange &LR,
- Register VRegOrUnit, bool SubRangeCheck = false,
+ RegisterUnit VRegOrUnit, bool SubRangeCheck = false,
LaneBitmask LaneMask = LaneBitmask::getNone());
void markReachable(const MachineBasicBlock *MBB);
@@ -344,12 +344,12 @@ struct MachineVerifier {
void verifyLiveVariables();
void verifyLiveIntervals();
void verifyLiveInterval(const LiveInterval &);
- void verifyLiveRangeValue(const LiveRange &, const VNInfo *, Register,
+ void verifyLiveRangeValue(const LiveRange &, const VNInfo *, RegisterUnit,
LaneBitmask);
void verifyLiveRangeSegment(const LiveRange &,
- const LiveRange::const_iterator I, Register,
+ const LiveRange::const_iterator I, RegisterUnit,
LaneBitmask);
- void verifyLiveRange(const LiveRange &, Register,
+ void verifyLiveRange(const LiveRange &, RegisterUnit,
LaneBitmask LaneMask = LaneBitmask::getNone());
void verifyStackFrame();
@@ -636,7 +636,7 @@ void MachineVerifier::report_context(const LiveInterval &LI) const {
OS << "- interval: " << LI << '\n';
}
-void MachineVerifier::report_context(const LiveRange &LR, Register VRegUnit,
+void MachineVerifier::report_context(const LiveRange &LR, RegisterUnit VRegUnit,
LaneBitmask LaneMask) const {
report_context_liverange(LR);
report_context_vreg_regunit(VRegUnit);
@@ -664,11 +664,13 @@ void MachineVerifier::report_context_vreg(Register VReg) const {
OS << "- v. register: " << printReg(VReg, TRI) << '\n';
}
-void MachineVerifier::report_context_vreg_regunit(Register VRegOrUnit) const {
+void MachineVerifier::report_context_vreg_regunit(
+ RegisterUnit VRegOrUnit) const {
if (VRegOrUnit.isVirtual()) {
- report_context_vreg(VRegOrUnit);
+ report_context_vreg(VRegOrUnit.asVirtualReg());
} else {
- OS << "- regunit: " << printRegUnit(VRegOrUnit, TRI) << '\n';
+ OS << "- regunit: " << printRegUnit(VRegOrUnit.asMCRegUnit(), TRI)
+ << '\n';
}
}
@@ -2828,7 +2830,7 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
void MachineVerifier::checkLivenessAtUse(const MachineOperand *MO,
unsigned MONum, SlotIndex UseIdx,
const LiveRange &LR,
- Register VRegOrUnit,
+ RegisterUnit VRegOrUnit,
LaneBitmask LaneMask) {
const MachineInstr *MI = MO->getParent();
@@ -2863,7 +2865,7 @@ void MachineVerifier::checkLivenessAtUse(const MachineOperand *MO,
void MachineVerifier::checkLivenessAtDef(const MachineOperand *MO,
unsigned MONum, SlotIndex DefIdx,
const LiveRange &LR,
- Register VRegOrUnit,
+ RegisterUnit VRegOrUnit,
bool SubRangeCheck,
LaneBitmask LaneMask) {
if (!LR.verify()) {
@@ -2973,13 +2975,13 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
if (MRI->isReservedRegUnit(Unit))
continue;
if (const LiveRange *LR = LiveInts->getCachedRegUnit(Unit))
- checkLivenessAtUse(MO, MONum, UseIdx, *LR, Unit);
+ checkLivenessAtUse(MO, MONum, UseIdx, *LR, RegisterUnit(Unit));
}
}
if (Reg.isVirtual()) {
// This is a virtual register interval.
- checkLivenessAtUse(MO, MONum, UseIdx, *LI, Reg);
+ checkLivenessAtUse(MO, MONum, UseIdx, *LI, RegisterUnit(Reg));
if (LI->hasSubRanges() && !MO->isDef()) {
LaneBitmask MOMask = SubRegIdx != 0
@@ -2989,7 +2991,8 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
for (const LiveInterval::SubRange &SR : LI->subranges()) {
if ((MOMask & SR.LaneMask).none())
continue;
- checkLivenessAtUse(MO, MONum, UseIdx, SR, Reg, SR.LaneMask);
+ checkLivenessAtUse(MO, MONum, UseIdx, SR, RegisterUnit(Reg),
+ SR.LaneMask);
LiveQueryResult LRQ = SR.Query(UseIdx);
if (LRQ.valueIn() || (MI->isPHI() && LRQ.valueOut()))
LiveInMask |= SR.LaneMask;
@@ -3081,7 +3084,7 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
DefIdx = DefIdx.getRegSlot(MO->isEarlyClobber());
if (Reg.isVirtual()) {
- checkLivenessAtDef(MO, MONum, DefIdx, *LI, Reg);
+ checkLivenessAtDef(MO, MONum, DefIdx, *LI, RegisterUnit(Reg));
if (LI->hasSubRanges()) {
LaneBitmask MOMask = SubRegIdx != 0
@@ -3090,7 +3093,8 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
for (const LiveInterval::SubRange &SR : LI->subranges()) {
if ((SR.LaneMask & MOMask).none())
continue;
- checkLivenessAtDef(MO, MONum, DefIdx, SR, Reg, true, SR.LaneMask);
+ checkLivenessAtDef(MO, MONum, DefIdx, SR, RegisterUnit(Reg), true,
+ SR.LaneMask);
}
}
}
@@ -3532,11 +3536,12 @@ void MachineVerifier::verifyLiveIntervals() {
// Verify all the cached regunit intervals.
for (unsigned i = 0, e = TRI->getNumRegUnits(); i != e; ++i)
if (const LiveRange *LR = LiveInts->getCachedRegUnit(i))
- verifyLiveRange(*LR, i);
+ verifyLiveRange(*LR, RegisterUnit(i));
}
void MachineVerifier::verifyLiveRangeValue(const LiveRange &LR,
- const VNInfo *VNI, Register Reg,
+ const VNInfo *VNI,
+ RegisterUnit RegUnit,
LaneBitmask LaneMask) {
if (VNI->isUnused())
return;
@@ -3545,14 +3550,14 @@ void MachineVerifier::verifyLiveRangeValue(const LiveRange &LR,
if (!DefVNI) {
report("Value not live at VNInfo def and not marked unused", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
return;
}
if (DefVNI != VNI) {
report("Live segment at def has different VNInfo", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
return;
}
@@ -3560,7 +3565,7 @@ void MachineVerifier::verifyLiveRangeValue(const LiveRange &LR,
const MachineBasicBlock *MBB = LiveInts->getMBBFromIndex(VNI->def);
if (!MBB) {
report("Invalid VNInfo definition index", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
return;
}
@@ -3568,7 +3573,7 @@ void MachineVerifier::verifyLiveRangeValue(const LiveRange &LR,
if (VNI->isPHIDef()) {
if (VNI->def != LiveInts->getMBBStartIdx(MBB)) {
report("PHIDef VNInfo is not defined at MBB start", MBB);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
}
return;
@@ -3578,57 +3583,56 @@ void MachineVerifier::verifyLiveRangeValue(const LiveRange &LR,
const MachineInstr *MI = LiveInts->getInstructionFromIndex(VNI->def);
if (!MI) {
report("No instruction at VNInfo def index", MBB);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
return;
}
- if (Reg != 0) {
- bool hasDef = false;
- bool isEarlyClobber = false;
- for (ConstMIBundleOperands MOI(*MI); MOI.isValid(); ++MOI) {
- if (!MOI->isReg() || !MOI->isDef())
+ bool hasDef = false;
+ bool isEarlyClobber = false;
+ for (ConstMIBundleOperands MOI(*MI); MOI.isValid(); ++MOI) {
+ if (!MOI->isReg() || !MOI->isDef())
+ continue;
+ if (RegUnit.isVirtual()) {
+ if (MOI->getReg() != RegUnit.asVirtualReg())
continue;
- if (Reg.isVirtual()) {
- if (MOI->getReg() != Reg)
- continue;
- } else {
- if (!MOI->getReg().isPhysical() || !TRI->hasRegUnit(MOI->getReg(), Reg))
- continue;
- }
- if (LaneMask.any() &&
- (TRI->getSubRegIndexLaneMask(MOI->getSubReg()) & LaneMask).none())
+ } else {
+ if (!MOI->getReg().isPhysical() ||
+ !TRI->hasRegUnit(MOI->getReg(), RegUnit.asMCRegUnit()))
continue;
- hasDef = true;
- if (MOI->isEarlyClobber())
- isEarlyClobber = true;
}
+ if (LaneMask.any() &&
+ (TRI->getSubRegIndexLaneMask(MOI->getSubReg()) & LaneMask).none())
+ continue;
+ hasDef = true;
+ if (MOI->isEarlyClobber())
+ isEarlyClobber = true;
+ }
- if (!hasDef) {
- report("Defining instruction does not modify register", MI);
- report_context(LR, Reg, LaneMask);
- report_context(*VNI);
- }
+ if (!hasDef) {
+ report("Defining instruction does not modify register", MI);
+ report_context(LR, RegUnit, LaneMask);
+ report_context(*VNI);
+ }
- // Early clobber defs begin at USE slots, but other defs must begin at
- // DEF slots.
- if (isEarlyClobber) {
- if (!VNI->def.isEarlyClobber()) {
- report("Early clobber def must be at an early-clobber slot", MBB);
- report_context(LR, Reg, LaneMask);
- report_context(*VNI);
- }
- } else if (!VNI->def.isRegister()) {
- report("Non-PHI, non-early clobber def must be at a register slot", MBB);
- report_context(LR, Reg, LaneMask);
+ // Early clobber defs begin at USE slots, but other defs must begin at
+ // DEF slots.
+ if (isEarlyClobber) {
+ if (!VNI->def.isEarlyClobber()) {
+ report("Early clobber def must be at an early-clobber slot", MBB);
+ report_context(LR, RegUnit, LaneMask);
report_context(*VNI);
}
+ } else if (!VNI->def.isRegister()) {
+ report("Non-PHI, non-early clobber def must be at a register slot", MBB);
+ report_context(LR, RegUnit, LaneMask);
+ report_context(*VNI);
}
}
void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
const LiveRange::const_iterator I,
- Register Reg,
+ RegisterUnit RegUnit,
LaneBitmask LaneMask) {
const LiveRange::Segment &S = *I;
const VNInfo *VNI = S.valno;
@@ -3636,28 +3640,28 @@ void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
if (VNI->id >= LR.getNumValNums() || VNI != LR.getValNumInfo(VNI->id)) {
report("Foreign valno in live segment", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(S);
report_context(*VNI);
}
if (VNI->isUnused()) {
report("Live segment valno is marked unused", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(S);
}
const MachineBasicBlock *MBB = LiveInts->getMBBFromIndex(S.start);
if (!MBB) {
report("Bad start of live segment, no basic block", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(S);
return;
}
SlotIndex MBBStartIdx = LiveInts->getMBBStartIdx(MBB);
if (S.start != MBBStartIdx && S.start != VNI->def) {
report("Live segment must begin at MBB entry or valno def", MBB);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(S);
}
@@ -3665,7 +3669,7 @@ void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
LiveInts->getMBBFromIndex(S.end.getPrevSlot());
if (!EndMBB) {
report("Bad end of live segment, no basic block", MF);
- report_context(LR, Reg, LaneMask);
+ report_context(LR, RegUnit, LaneMask);
report_context(S);
return;
}
@@ -3673,7 +3677,7 @@ void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
// Checks for non-live-out segments.
if (S.end != LiveInts->getMBBEndIdx(EndMBB)) {
// RegUnit intervals are allowed dead phis.
- if (!Reg.isVirtual() && VNI->isPHIDef() && S.start == VNI->def &&
+ if (!RegUnit.isVirtual() && VNI->isPHIDef() && S.start == VNI->def &&
S.end == VNI->def.getDeadSlot())
return;
@@ -3682,7 +3686,7 @@ void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
LiveInts->getInstructionFromIndex(S.end.getPrevSlot());
if (!MI) {
report("Live segment doesn't end at a valid instruction", EndMBB);
- report_...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/123768
More information about the llvm-commits
mailing list