[CodeGen] Use MCRegister instead of MCPhysReg in RegisterMaskPair. NFC #123688
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update some other places to avoid implicit conversions, but I probably missed some.
Member
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-x86 Author: Craig Topper (topperc) ChangesUpdate some other places to avoid implicit conversions this introduces, but I probably missed some. Full diff: https://github.com/llvm/llvm-project/pull/123688.diff 10 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 7fe33c3913f2dd..0b803a97247422 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -129,7 +129,7 @@ class MachineBasicBlock
/// clearly as they both have an integer type.
struct RegisterMaskPair {
public:
- MCPhysReg PhysReg;
+ MCRegister PhysReg;
LaneBitmask LaneMask;
RegisterMaskPair(MCPhysReg PhysReg, LaneBitmask LaneMask)
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index bc1a65064a8c52..65476fa05a2030 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -381,7 +381,7 @@ void BranchFolder::replaceTailWithBranchTo(MachineBasicBlock::iterator OldInst,
// full registers:
assert(P.LaneMask == LaneBitmask::getAll() &&
"Can only handle full register.");
- MCPhysReg Reg = P.PhysReg;
+ MCRegister Reg = P.PhysReg;
if (!LiveRegs.available(*MRI, Reg))
continue;
DebugLoc DL;
diff --git a/llvm/lib/CodeGen/LivePhysRegs.cpp b/llvm/lib/CodeGen/LivePhysRegs.cpp
index 96380d40848257..2ba17e46be5a6e 100644
--- a/llvm/lib/CodeGen/LivePhysRegs.cpp
+++ b/llvm/lib/CodeGen/LivePhysRegs.cpp
@@ -154,7 +154,7 @@ bool LivePhysRegs::available(const MachineRegisterInfo &MRI,
/// Add live-in registers of basic block \p MBB to \p LiveRegs.
void LivePhysRegs::addBlockLiveIns(const MachineBasicBlock &MBB) {
for (const auto &LI : MBB.liveins()) {
- MCPhysReg Reg = LI.PhysReg;
+ MCRegister Reg = LI.PhysReg;
LaneBitmask Mask = LI.LaneMask;
MCSubRegIndexIterator S(Reg, TRI);
assert(Mask.any() && "Invalid livein mask");
diff --git a/llvm/lib/CodeGen/LiveVariables.cpp b/llvm/lib/CodeGen/LiveVariables.cpp
index 55428ab7832de4..00dae84b5840bd 100644
--- a/llvm/lib/CodeGen/LiveVariables.cpp
+++ b/llvm/lib/CodeGen/LiveVariables.cpp
@@ -576,7 +576,7 @@ void LiveVariables::runOnBlock(MachineBasicBlock *MBB, unsigned NumRegs) {
// Mark live-in registers as live-in.
SmallVector<Register, 4> Defs;
for (const auto &LI : MBB->liveins()) {
- assert(Register::isPhysicalRegister(LI.PhysReg) &&
+ assert(LI.PhysReg.isPhysical() &&
"Cannot have a live-in virtual register!");
HandlePhysRegDef(LI.PhysReg, nullptr, Defs);
}
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 594ff5ac4c07f1..d41b11307e7bc0 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -894,7 +894,7 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
regsLive.clear();
if (MRI->tracksLiveness()) {
for (const auto &LI : MBB->liveins()) {
- if (!Register::isPhysicalRegister(LI.PhysReg)) {
+ if (!LI.PhysReg.isPhysical()) {
report("MBB live-in list contains non-physical register", MBB);
continue;
}
@@ -3448,7 +3448,7 @@ void MachineVerifier::visitMachineFunctionAfter() {
if (MRI->tracksLiveness())
for (const auto &MBB : *MF)
for (MachineBasicBlock::RegisterMaskPair P : MBB.liveins()) {
- MCPhysReg LiveInReg = P.PhysReg;
+ MCRegister LiveInReg = P.PhysReg;
bool hasAliases = MCRegAliasIterator(LiveInReg, TRI, false).isValid();
if (hasAliases || isAllocatable(LiveInReg) || isReserved(LiveInReg))
continue;
diff --git a/llvm/lib/CodeGen/RDFLiveness.cpp b/llvm/lib/CodeGen/RDFLiveness.cpp
index 7b4b7fdb3e76ad..682d316a5bfac0 100644
--- a/llvm/lib/CodeGen/RDFLiveness.cpp
+++ b/llvm/lib/CodeGen/RDFLiveness.cpp
@@ -895,7 +895,7 @@ void Liveness::computeLiveIns() {
void Liveness::resetLiveIns() {
for (auto &B : DFG.getMF()) {
// Remove all live-ins.
- std::vector<unsigned> T;
+ std::vector<MCRegister> T;
for (const MachineBasicBlock::RegisterMaskPair &LI : B.liveins())
T.push_back(LI.PhysReg);
for (auto I : T)
@@ -917,7 +917,7 @@ void Liveness::resetKills(MachineBasicBlock *B) {
for (auto I : B->liveins()) {
MCSubRegIndexIterator S(I.PhysReg, &TRI);
if (!S.isValid()) {
- LV.set(I.PhysReg);
+ LV.set(I.PhysReg.id());
continue;
}
do {
diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 3863ca80bb44e9..5bad4df0db19f1 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -276,7 +276,7 @@ class RegAllocFastImpl {
// Assign index for each instruction to quickly determine dominance.
InstrPosIndexes PosIndexes;
- void setPhysRegState(MCPhysReg PhysReg, unsigned NewState);
+ void setPhysRegState(MCRegister PhysReg, unsigned NewState);
bool isPhysRegFree(MCPhysReg PhysReg) const;
/// Mark a physreg as used in this instruction.
@@ -449,7 +449,7 @@ bool RegAllocFastImpl::shouldAllocateRegister(const Register Reg) const {
return ShouldAllocateRegisterImpl(*TRI, *MRI, Reg);
}
-void RegAllocFastImpl::setPhysRegState(MCPhysReg PhysReg, unsigned NewState) {
+void RegAllocFastImpl::setPhysRegState(MCRegister PhysReg, unsigned NewState) {
for (MCRegUnit Unit : TRI->regunits(PhysReg))
RegUnitStates[Unit] = NewState;
}
@@ -671,7 +671,7 @@ void RegAllocFastImpl::reloadAtBegin(MachineBasicBlock &MBB) {
return;
for (MachineBasicBlock::RegisterMaskPair P : MBB.liveins()) {
- MCPhysReg Reg = P.PhysReg;
+ MCRegister Reg = P.PhysReg;
// Set state to live-in. This possibly overrides mappings to virtual
// registers but we don't care anymore at this point.
setPhysRegState(Reg, regLiveIn);
diff --git a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
index e8a4d73c671c9b..4d0d99bce258a3 100644
--- a/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CollectLOH.cpp
@@ -251,7 +251,7 @@ static bool supportLoadFromLiteral(const MachineInstr &MI) {
/// Number of GPR registers traked by mapRegToGPRIndex()
static const unsigned N_GPR_REGS = 31;
/// Map register number to index from 0-30.
-static int mapRegToGPRIndex(MCPhysReg Reg) {
+static int mapRegToGPRIndex(MCRegister Reg) {
static_assert(AArch64::X28 - AArch64::X0 + 3 == N_GPR_REGS, "Number of GPRs");
static_assert(AArch64::W30 - AArch64::W0 + 1 == N_GPR_REGS, "Number of GPRs");
if (AArch64::X0 <= Reg && Reg <= AArch64::X28)
diff --git a/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp b/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
index 3a9421fae0f607..258010d3311817 100644
--- a/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
+++ b/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
@@ -402,7 +402,7 @@ void RegDefsUses::addLiveOut(const MachineBasicBlock &MBB,
for (const MachineBasicBlock *S : MBB.successors())
if (S != &SuccBB)
for (const auto &LI : S->liveins())
- Uses.set(LI.PhysReg);
+ Uses.set(LI.PhysReg.id());
}
bool RegDefsUses::update(const MachineInstr &MI, unsigned Begin, unsigned End) {
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index 4d40c23eb5617a..f7398ac7aa1307 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -174,7 +174,7 @@ static unsigned getPOP2Opcode(const X86Subtarget &ST) {
static bool isEAXLiveIn(MachineBasicBlock &MBB) {
for (MachineBasicBlock::RegisterMaskPair RegMask : MBB.liveins()) {
- unsigned Reg = RegMask.PhysReg;
+ MCRegister Reg = RegMask.PhysReg;
if (Reg == X86::RAX || Reg == X86::EAX || Reg == X86::AX ||
Reg == X86::AH || Reg == X86::AL)
|
arsenm
reviewed
Jan 21, 2025
Contributor
arsenm
left a comment
arsenm
left a comment
There was a problem hiding this comment.
Most of these are really MCPhysReg. There is a slight difference since MCPhysReg is 16-bit and MCRegister is 32
| for (auto &B : DFG.getMF()) { | ||
| // Remove all live-ins. | ||
| std::vector<unsigned> T; | ||
| std::vector<MCRegister> T; |
Contributor
There was a problem hiding this comment.
This probably should be MCPhysReg
Collaborator
Author
My understanding was that MCPhysReg was 16-bit primarily to reduce static data size in *GenRegisterInfo.inc. I'm probably the one who made it 16-bit. It doesn't provide any size savings in The |
Contributor
We should change it, but not in this way. We should stop tracking liveins by registers and use regunits instead. As it is now it doesn't matter which type it uses |
Collaborator
Author
Ok so what should I do for this patch? I'm trying to fix the implicit conversion from MCRegister to MCPhysReg when the RegisterMaskPair constructor is used. Should I just add a .id() call instead? |
arsenm
approved these changes
Jan 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update some other places to avoid implicit conversions this introduces, but I probably missed some.