[llvm] [CodeGen] Remove Register::operator++/+= NFC (PR #228196)
Jan Rehders via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 1 11:53:29 PDT 2026
https://github.com/janr-bay created https://github.com/llvm/llvm-project/pull/228196
Follow up PR for #224083 removing existing operators on Register class as proposed by @arsenm and @davemgreen
Operators are replaced with an explicit function changeVirtRegIndex with extra checks. Split into two commits in case using arithmetic relying on conversion to/from unsigned is preferred over the explicit function
>From 9b241f7714cb2abd310baf37a33df79f4939807d Mon Sep 17 00:00:00 2001
From: Jan Rehders <jrehders at baylibre.com>
Date: Fri, 25 Sep 2026 19:58:54 +0200
Subject: [PATCH 1/2] [CodeGen] Remove Register::operator++/+= NFC
These do not make a lot of sense and where only used in a few places. Replaced
by iterating over the list of registers or relying on conversion between
unsigned and Register for the few places where it was still used.
---
llvm/include/llvm/CodeGen/Register.h | 20 -----------
.../SelectionDAG/FunctionLoweringInfo.cpp | 8 +++--
.../SelectionDAG/SelectionDAGBuilder.cpp | 7 ++--
llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp | 36 ++++++++++++-------
4 files changed, 32 insertions(+), 39 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/Register.h b/llvm/include/llvm/CodeGen/Register.h
index 3682f5166dd7d..b0584658911b8 100644
--- a/llvm/include/llvm/CodeGen/Register.h
+++ b/llvm/include/llvm/CodeGen/Register.h
@@ -139,26 +139,6 @@ class Register {
constexpr bool operator!=(MCPhysReg Other) const {
return Reg != unsigned(Other);
}
-
- /// Operators to move from one register to another nearby register by adding
- /// an offset.
- Register &operator++() {
- assert(isValid());
- ++Reg;
- return *this;
- }
-
- Register operator++(int) {
- Register R(*this);
- ++(*this);
- return R;
- }
-
- Register &operator+=(unsigned RHS) {
- assert(isValid());
- Reg += RHS;
- return *this;
- }
};
// Provide DenseMapInfo for Register
diff --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
index 1a2f6dd1bf4e4..f3b97a2438c3e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -315,7 +315,7 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
for (unsigned i = 0; i != NumRegisters; ++i)
BuildMI(MBB, DL, TII->get(TargetOpcode::PHI), PHIReg + i);
- PHIReg += NumRegisters;
+ PHIReg = PHIReg + NumRegisters;
}
}
}
@@ -584,8 +584,10 @@ FunctionLoweringInfo::getValueFromVirtualReg(Register Vreg) {
Register Reg = P.second;
for (EVT VT : ValueVTs) {
unsigned NumRegisters = TLI->getNumRegisters(Fn->getContext(), VT);
- for (unsigned i = 0, e = NumRegisters; i != e; ++i)
- VirtReg2Value[Reg++] = P.first;
+ for (unsigned i = 0, e = NumRegisters; i != e; ++i) {
+ Reg = Reg + 1;
+ VirtReg2Value[Reg] = P.first;
+ }
}
}
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index f7eec8c5088d4..9c5de8359c016 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12538,7 +12538,7 @@ SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
const unsigned NumRegisters = TLI.getNumRegisters(*DAG.getContext(), VT);
for (unsigned i = 0; i != NumRegisters; ++i)
FuncInfo.PHINodesToUpdate.emplace_back(&*MBBI++, Reg + i);
- Reg += NumRegisters;
+ Reg = Reg + NumRegisters;
}
}
}
@@ -13275,7 +13275,8 @@ void SelectionDAGBuilder::visitCallBrLandingPad(const CallInst &I) {
// getRegistersForValue may produce 1 to many registers based on whether
// the OpInfo.ConstraintVT is legal on the target or not.
for (Register &Reg : OpInfo.AssignedRegs.Regs) {
- Register OriginalDef = FollowCopyChain(MRI, InitialDef++);
+ InitialDef = InitialDef + 1;
+ Register OriginalDef = FollowCopyChain(MRI, InitialDef);
if (OriginalDef.isPhysical())
FuncInfo.MBB->addLiveIn(OriginalDef);
// Update the assigned registers to use the original defs.
@@ -13292,7 +13293,7 @@ void SelectionDAGBuilder::visitCallBrLandingPad(const CallInst &I) {
SDValue Flag;
SDValue V = TLI.LowerAsmOutputForConstraint(Chain, Flag, getCurSDLoc(),
OpInfo, DAG);
- ++InitialDef;
+ InitialDef = InitialDef + 1;
ResultValues.push_back(V);
ResultVTs.push_back(OpInfo.ConstraintVT);
break;
diff --git a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
index 4d2d5aebbf421..59ef9c1a24247 100644
--- a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
@@ -172,6 +172,16 @@ namespace {
return PseudoOpc < TE.PseudoOpc;
}
};
+
+ constexpr Register CalleeSavedFPRegs[] = {
+ ARM::S16, ARM::S17, ARM::S18, ARM::S19, ARM::S20, ARM::S21,
+ ARM::S22, ARM::S23, ARM::S24, ARM::S25, ARM::S26, ARM::S27,
+ ARM::S28, ARM::S29, ARM::S30, ARM::S31};
+
+ constexpr Register CalleeSavedRegs[] = {ARM::R4, ARM::R5, ARM::R6, ARM::R7,
+ ARM::R8, ARM::R9, ARM::R10, ARM::R11};
+ constexpr ArrayRef<Register> CalleeSavedLoRegs{CalleeSavedRegs, 4};
+ constexpr ArrayRef<Register> CalleeSavedHiRegs{CalleeSavedRegs + 4, 4};
}
static const NEONLdStTableEntry NEONLdStTable[] = {
@@ -1637,7 +1647,7 @@ void ARMExpandPseudo::CMSESaveClearFPRegsV81(MachineBasicBlock &MBB,
BuildMI(MBB, MBBI, DL, TII->get(ARM::VSTMSDB_UPD), ARM::SP)
.addReg(ARM::SP)
.add(predOps(ARMCC::AL));
- for (Register Reg = ARM::S16; Reg <= ARM::S31; ++Reg)
+ for (Register Reg : CalleeSavedFPRegs)
VPUSH.addReg(Reg);
// Clear FP registers with a VSCCLRM.
@@ -1842,7 +1852,7 @@ void ARMExpandPseudo::CMSERestoreFPRegsV81(
BuildMI(MBB, MBBI, DL, TII->get(ARM::VLDMSIA_UPD), ARM::SP)
.addReg(ARM::SP)
.add(predOps(ARMCC::AL));
- for (Register Reg = ARM::S16; Reg <= ARM::S31; ++Reg)
+ for (Register Reg : CalleeSavedFPRegs)
VPOP.addReg(Reg, RegState::Define);
}
}
@@ -2107,10 +2117,11 @@ static void CMSEPushCalleeSaves(const TargetInstrInfo &TII,
Register JumpReg, const LivePhysRegs &LiveRegs,
bool Thumb1Only) {
const DebugLoc &DL = MBBI->getDebugLoc();
+
if (Thumb1Only) { // push Lo and Hi regs separately
MachineInstrBuilder PushMIB =
BuildMI(MBB, MBBI, DL, TII.get(ARM::tPUSH)).add(predOps(ARMCC::AL));
- for (Register Reg = ARM::R4; Reg < ARM::R8; ++Reg) {
+ for (Register Reg : CalleeSavedLoRegs) {
PushMIB.addReg(
Reg, getUndefRegState(Reg != JumpReg && !LiveRegs.contains(Reg)));
}
@@ -2122,21 +2133,19 @@ static void CMSEPushCalleeSaves(const TargetInstrInfo &TII,
// memory, and allow us to later pop them with a single instructions.
// FIXME: Could also use any of r0-r3 that are free (including in the
// first PUSH above).
- const Register LoRegs[] = {ARM::R7, ARM::R6, ARM::R5, ARM::R4};
- const Register HiRegs[] = {ARM::R11, ARM::R10, ARM::R9, ARM::R8};
- unsigned HiIdx = 0;
- for (Register LoReg : LoRegs) {
+ unsigned HiIdx = CalleeSavedHiRegs.size() - 1;
+ for (Register LoReg : llvm::reverse(CalleeSavedLoRegs)) {
if (JumpReg == LoReg)
continue;
BuildMI(MBB, MBBI, DL, TII.get(ARM::tMOVr), LoReg)
- .addReg(HiRegs[HiIdx],
- getUndefRegState(!LiveRegs.contains(HiRegs[HiIdx])))
+ .addReg(CalleeSavedHiRegs[HiIdx], getUndefRegState(!LiveRegs.contains(
+ CalleeSavedHiRegs[HiIdx])))
.add(predOps(ARMCC::AL));
- ++HiIdx;
+ --HiIdx;
}
MachineInstrBuilder PushMIB2 =
BuildMI(MBB, MBBI, DL, TII.get(ARM::tPUSH)).add(predOps(ARMCC::AL));
- for (Register Reg = ARM::R4; Reg < ARM::R8; ++Reg) {
+ for (Register Reg : CalleeSavedLoRegs) {
if (Reg == JumpReg)
continue;
PushMIB2.addReg(Reg, RegState::Kill);
@@ -2159,7 +2168,7 @@ static void CMSEPushCalleeSaves(const TargetInstrInfo &TII,
BuildMI(MBB, MBBI, DL, TII.get(ARM::t2STMDB_UPD), ARM::SP)
.addReg(ARM::SP)
.add(predOps(ARMCC::AL));
- for (Register Reg = ARM::R4; Reg < ARM::R12; ++Reg) {
+ for (Register Reg : CalleeSavedRegs) {
PushMIB.addReg(
Reg, getUndefRegState(Reg != JumpReg && !LiveRegs.contains(Reg)));
}
@@ -2189,7 +2198,8 @@ static void CMSEPopCalleeSaves(const TargetInstrInfo &TII,
BuildMI(MBB, MBBI, DL, TII.get(ARM::t2LDMIA_UPD), ARM::SP)
.addReg(ARM::SP)
.add(predOps(ARMCC::AL));
- for (Register Reg = ARM::R4; Reg < ARM::R12; ++Reg)
+
+ for (Register Reg : CalleeSavedRegs)
PopMIB.addReg(Reg, RegState::Define);
}
}
>From b26ead84416cac32721215483fc35155628a6676 Mon Sep 17 00:00:00 2001
From: Jan Rehders <jrehders at baylibre.com>
Date: Thu, 1 Oct 2026 13:31:18 +0200
Subject: [PATCH 2/2] [CodeGen] Register class relies on less implicit
conversions NFC
---
llvm/include/llvm/CodeGen/Register.h | 12 ++++++++++++
.../CodeGen/SelectionDAG/FunctionLoweringInfo.cpp | 4 ++--
.../lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 6 +++---
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/Register.h b/llvm/include/llvm/CodeGen/Register.h
index b0584658911b8..35bb19675583c 100644
--- a/llvm/include/llvm/CodeGen/Register.h
+++ b/llvm/include/llvm/CodeGen/Register.h
@@ -89,6 +89,18 @@ class Register {
return Reg & ~Register::VirtualRegFlag;
}
+ /// Changes the virtual register number by offset
+ void changeVirtRegIndex(unsigned offset) {
+ assert(isVirtual() && "Not a virtual register");
+ assert(Reg + offset >= Reg && "Register number overflow");
+ Reg += offset;
+ }
+
+ /// Make calling changeVirtRegIndex with anything implicitly converting to
+ /// unsigned a compiler error to prevent bugs due to signed/unsigned mismatch
+ /// and other overflow bugs
+ template <typename T> void changeVirtRegIndex(T offset) = delete;
+
/// Compute the frame index from a register value representing a stack slot.
int stackSlotIndex() const {
assert(isStack() && "Not a stack slot");
diff --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
index f3b97a2438c3e..c0feed4e4b824 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -315,7 +315,7 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
for (unsigned i = 0; i != NumRegisters; ++i)
BuildMI(MBB, DL, TII->get(TargetOpcode::PHI), PHIReg + i);
- PHIReg = PHIReg + NumRegisters;
+ PHIReg.changeVirtRegIndex(NumRegisters);
}
}
}
@@ -585,7 +585,7 @@ FunctionLoweringInfo::getValueFromVirtualReg(Register Vreg) {
for (EVT VT : ValueVTs) {
unsigned NumRegisters = TLI->getNumRegisters(Fn->getContext(), VT);
for (unsigned i = 0, e = NumRegisters; i != e; ++i) {
- Reg = Reg + 1;
+ Reg.changeVirtRegIndex(1u);
VirtReg2Value[Reg] = P.first;
}
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 9c5de8359c016..f41e6317d4ed6 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12538,7 +12538,7 @@ SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
const unsigned NumRegisters = TLI.getNumRegisters(*DAG.getContext(), VT);
for (unsigned i = 0; i != NumRegisters; ++i)
FuncInfo.PHINodesToUpdate.emplace_back(&*MBBI++, Reg + i);
- Reg = Reg + NumRegisters;
+ Reg.changeVirtRegIndex(NumRegisters);
}
}
}
@@ -13275,7 +13275,7 @@ void SelectionDAGBuilder::visitCallBrLandingPad(const CallInst &I) {
// getRegistersForValue may produce 1 to many registers based on whether
// the OpInfo.ConstraintVT is legal on the target or not.
for (Register &Reg : OpInfo.AssignedRegs.Regs) {
- InitialDef = InitialDef + 1;
+ InitialDef.changeVirtRegIndex(1u);
Register OriginalDef = FollowCopyChain(MRI, InitialDef);
if (OriginalDef.isPhysical())
FuncInfo.MBB->addLiveIn(OriginalDef);
@@ -13293,7 +13293,7 @@ void SelectionDAGBuilder::visitCallBrLandingPad(const CallInst &I) {
SDValue Flag;
SDValue V = TLI.LowerAsmOutputForConstraint(Chain, Flag, getCurSDLoc(),
OpInfo, DAG);
- InitialDef = InitialDef + 1;
+ InitialDef.changeVirtRegIndex(1u);
ResultValues.push_back(V);
ResultVTs.push_back(OpInfo.ConstraintVT);
break;
More information about the llvm-commits
mailing list