Skip to content

[RDF] Rename RegisterId field in RegisterRef Reg->Id. NFC #168154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 15, 2025
Merged

[RDF] Rename RegisterId field in RegisterRef Reg->Id. NFC #168154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions llvm/include/llvm/CodeGen/RDFGraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ struct TargetOperandInfo {

// Packed register reference. Only used for storage.
struct PackedRegisterRef {
RegisterId Reg;
RegisterId Id;
uint32_t MaskId;
};

Expand Down Expand Up @@ -779,13 +779,13 @@ struct DataFlowGraph {
void releaseBlock(NodeId B, DefStackMap &DefM);

PackedRegisterRef pack(RegisterRef RR) {
return {RR.Reg, LMI.getIndexForLaneMask(RR.Mask)};
return {RR.Id, LMI.getIndexForLaneMask(RR.Mask)};
}
PackedRegisterRef pack(RegisterRef RR) const {
return {RR.Reg, LMI.getIndexForLaneMask(RR.Mask)};
return {RR.Id, LMI.getIndexForLaneMask(RR.Mask)};
}
RegisterRef unpack(PackedRegisterRef PR) const {
return RegisterRef(PR.Reg, LMI.getLaneMaskForIndex(PR.MaskId));
return RegisterRef(PR.Id, LMI.getLaneMaskForIndex(PR.MaskId));
}

RegisterRef makeRegRef(unsigned Reg, unsigned Sub) const;
Expand Down
34 changes: 17 additions & 17 deletions llvm/include/llvm/CodeGen/RDFRegisters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,45 +91,45 @@ struct RegisterRef {
static constexpr RegisterId UnitFlag = 1u << 31;

public:
RegisterId Reg = 0;
RegisterId Id = 0;
LaneBitmask Mask = LaneBitmask::getNone(); // Only for registers.

constexpr RegisterRef() = default;
constexpr explicit RegisterRef(RegisterId R,
LaneBitmask M = LaneBitmask::getAll())
: Reg(R), Mask(isRegId(R) && R != 0 ? M : LaneBitmask::getNone()) {}
: Id(R), Mask(isRegId(R) && R != 0 ? M : LaneBitmask::getNone()) {}

// Classify null register as a "register".
constexpr bool isReg() const { return Reg == 0 || isRegId(Reg); }
constexpr bool isUnit() const { return isUnitId(Reg); }
constexpr bool isMask() const { return isMaskId(Reg); }
constexpr bool isReg() const { return Id == 0 || isRegId(Id); }
constexpr bool isUnit() const { return isUnitId(Id); }
constexpr bool isMask() const { return isMaskId(Id); }

constexpr MCRegister asMCReg() const {
assert(isReg());
return Reg;
return Id;
}

constexpr MCRegUnit asMCRegUnit() const {
assert(isUnit());
return Reg & ~UnitFlag;
return Id & ~UnitFlag;
}

constexpr unsigned asMaskIdx() const {
assert(isMask());
return Reg & ~MaskFlag;
return Id & ~MaskFlag;
}

constexpr operator bool() const {
return !isReg() || (Reg != 0 && Mask.any());
explicit constexpr operator bool() const {
return !isReg() || (Id != 0 && Mask.any());
}

size_t hash() const {
return std::hash<RegisterId>{}(Reg) ^
return std::hash<RegisterId>{}(Id) ^
std::hash<LaneBitmask::Type>{}(Mask.getAsInteger());
}

static constexpr bool isRegId(RegisterId Id) {
return !(Id & UnitFlag) && !(Id & MaskFlag);
return Id != 0 && !(Id & UnitFlag) && !(Id & MaskFlag);
Copy link
Contributor

@s-barannikov s-barannikov Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary?
It is confusing that isReg() and isRegId() return different values for invalid register.

Copy link
Collaborator Author

@topperc topperc Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not be necessary. It was an unintentional mistake in my previous patch. When isRegId used Register::isPhysicalRegister it returned false for 0.

}
static constexpr bool isUnitId(RegisterId Id) { return Id & UnitFlag; }
static constexpr bool isMaskId(RegisterId Id) { return Id & MaskFlag; }
Expand All @@ -151,21 +151,21 @@ struct PhysicalRegisterInfo {
return RegisterRef::toMaskId(RegMasks.find(RM));
}

const uint32_t *getRegMaskBits(RegisterId R) const {
return RegMasks.get(RegisterRef(R).asMaskIdx());
const uint32_t *getRegMaskBits(RegisterRef RR) const {
return RegMasks.get(RR.asMaskIdx());
}

bool alias(RegisterRef RA, RegisterRef RB) const;

// Returns the set of aliased physical registers.
std::set<RegisterId> getAliasSet(RegisterId Reg) const;
std::set<RegisterId> getAliasSet(RegisterRef RR) const;

RegisterRef getRefForUnit(uint32_t U) const {
return RegisterRef(UnitInfos[U].Reg, UnitInfos[U].Mask);
}

const BitVector &getMaskUnits(RegisterId MaskId) const {
return MaskInfos[RegisterRef(MaskId).asMaskIdx()].Units;
const BitVector &getMaskUnits(RegisterRef RR) const {
return MaskInfos[RR.asMaskIdx()].Units;
}

std::set<RegisterId> getUnits(RegisterRef RR) const;
Expand Down
24 changes: 12 additions & 12 deletions llvm/lib/CodeGen/RDFGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1061,13 +1061,13 @@ void DataFlowGraph::pushClobbers(Instr IA, DefStackMap &DefM) {

// Push the definition on the stack for the register and all aliases.
// The def stack traversal in linkNodeUp will check the exact aliasing.
DefM[RR.Reg].push(DA);
Defined.insert(RR.Reg);
for (RegisterId A : getPRI().getAliasSet(RR.Reg)) {
DefM[RR.Id].push(DA);
Defined.insert(RR.Id);
for (RegisterId A : getPRI().getAliasSet(RR)) {
if (RegisterRef::isRegId(A) && !isTracked(RegisterRef(A)))
continue;
// Check that we don't push the same def twice.
assert(A != RR.Reg);
assert(A != RR.Id);
if (!Defined.count(A))
DefM[A].push(DA);
}
Expand Down Expand Up @@ -1109,7 +1109,7 @@ void DataFlowGraph::pushDefs(Instr IA, DefStackMap &DefM) {
#ifndef NDEBUG
// Assert if the register is defined in two or more unrelated defs.
// This could happen if there are two or more def operands defining it.
if (!Defined.insert(RR.Reg).second) {
if (!Defined.insert(RR.Id).second) {
MachineInstr *MI = Stmt(IA).Addr->getCode();
dbgs() << "Multiple definitions of register: " << Print(RR, *this)
<< " in\n " << *MI << "in " << printMBBReference(*MI->getParent())
Expand All @@ -1119,12 +1119,12 @@ void DataFlowGraph::pushDefs(Instr IA, DefStackMap &DefM) {
#endif
// Push the definition on the stack for the register and all aliases.
// The def stack traversal in linkNodeUp will check the exact aliasing.
DefM[RR.Reg].push(DA);
for (RegisterId A : getPRI().getAliasSet(RR.Reg)) {
DefM[RR.Id].push(DA);
for (RegisterId A : getPRI().getAliasSet(RR)) {
if (RegisterRef::isRegId(A) && !isTracked(RegisterRef(A)))
continue;
// Check that we don't push the same def twice.
assert(A != RR.Reg);
assert(A != RR.Id);
DefM[A].push(DA);
}
// Mark all the related defs as visited.
Expand Down Expand Up @@ -1465,11 +1465,11 @@ void DataFlowGraph::buildPhis(BlockRefsMap &PhiM, Block BA,

for (RegisterRef RR : Defs.refs()) {
if (!DefM.empty()) {
auto F = DefM.find(RR.Reg);
auto F = DefM.find(RR.Id);
// Do not create a phi for unallocatable registers, or for registers
// that are never livein to BA.
// If a phi exists for RR, do not create another.
if (!MRI.isAllocatable(RR.Reg) || PhiDefs.hasCoverOf(RR) ||
if (!MRI.isAllocatable(RR.asMCReg()) || PhiDefs.hasCoverOf(RR) ||
F == DefM.end() || F->second.empty())
continue;
// Do not create a phi, if all reaching defs are clobbering
Expand Down Expand Up @@ -1601,7 +1601,7 @@ void DataFlowGraph::linkStmtRefs(DefStackMap &DefM, Stmt SA, Predicate P) {
Defs.insert(RR);
#endif

auto F = DefM.find(RR.Reg);
auto F = DefM.find(RR.Id);
if (F == DefM.end())
continue;
DefStack &DS = F->second;
Expand Down Expand Up @@ -1691,7 +1691,7 @@ void DataFlowGraph::linkBlockRefs(DefStackMap &DefM, BlockRefsMap &PhiClobberM,
for (auto U : IA.Addr->members_if(IsUseForBA, *this)) {
PhiUse PUA = U;
RegisterRef RR = PUA.Addr->getRegRef(*this);
linkRefUp<UseNode *>(IA, PUA, DefM[RR.Reg]);
linkRefUp<UseNode *>(IA, PUA, DefM[RR.Id]);
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions llvm/lib/CodeGen/RDFLiveness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ void Liveness::computePhiInfo() {
uint16_t F = A.Addr->getFlags();
if ((F & (NodeAttrs::Undef | NodeAttrs::PhiRef)) == 0) {
RegisterRef R = A.Addr->getRegRef(DFG);
RealUses[R.Reg].insert({A.Id, R.Mask});
RealUses[R.Id].insert({A.Id, R.Mask});
}
UN = A.Addr->getSibling();
}
Expand Down Expand Up @@ -706,8 +706,8 @@ void Liveness::computePhiInfo() {
LaneBitmask M = R.Mask & V.second;
if (M.none())
continue;
if (RegisterRef SS = ClearIn(RegisterRef(R.Reg, M), MidDefs, SM)) {
NodeRefSet &RS = RealUseMap[P.first][SS.Reg];
if (RegisterRef SS = ClearIn(RegisterRef(R.Id, M), MidDefs, SM)) {
NodeRefSet &RS = RealUseMap[P.first][SS.Id];
Changed |= RS.insert({V.first, SS.Mask}).second;
}
}
Expand Down Expand Up @@ -839,7 +839,7 @@ void Liveness::computeLiveIns() {
RegisterAggr TA(PRI);
TA.insert(D.Addr->getRegRef(DFG)).intersect(S);
LaneBitmask TM = TA.makeRegRef().Mask;
LOX[S.Reg].insert({D.Id, TM});
LOX[S.Id].insert({D.Id, TM});
}
}
}
Expand Down Expand Up @@ -899,7 +899,7 @@ void Liveness::resetLiveIns() {
// Add the newly computed live-ins.
const RegisterAggr &LiveIns = LiveMap[&B];
for (RegisterRef R : LiveIns.refs())
B.addLiveIn({MCPhysReg(R.Reg), R.Mask});
B.addLiveIn({R.asMCReg(), R.Mask});
}
}

Expand Down Expand Up @@ -1046,7 +1046,7 @@ void Liveness::traverse(MachineBasicBlock *B, RefMap &LiveIn) {

for (const std::pair<const RegisterId, NodeRefSet> &LE : LiveInCopy) {
RegisterRef LRef(LE.first);
NodeRefSet &NewDefs = LiveIn[LRef.Reg]; // To be filled.
NodeRefSet &NewDefs = LiveIn[LRef.Id]; // To be filled.
const NodeRefSet &OldDefs = LE.second;
for (NodeRef OR : OldDefs) {
// R is a def node that was live-on-exit
Expand Down Expand Up @@ -1129,7 +1129,7 @@ void Liveness::traverse(MachineBasicBlock *B, RefMap &LiveIn) {
RegisterRef RR = UA.Addr->getRegRef(DFG);
for (NodeAddr<DefNode *> D : getAllReachingDefs(UA))
if (getBlockWithRef(D.Id) != B)
LiveIn[RR.Reg].insert({D.Id, RR.Mask});
LiveIn[RR.Id].insert({D.Id, RR.Mask});
}
}

Expand Down
38 changes: 19 additions & 19 deletions llvm/lib/CodeGen/RDFRegisters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ bool PhysicalRegisterInfo::alias(RegisterRef RA, RegisterRef RB) const {
return !disjoint(getUnits(RA), getUnits(RB));
}

std::set<RegisterId> PhysicalRegisterInfo::getAliasSet(RegisterId Reg) const {
std::set<RegisterId> PhysicalRegisterInfo::getAliasSet(RegisterRef RR) const {
// Do not include Reg in the alias set.
std::set<RegisterId> AS;
assert(!RegisterRef::isUnitId(Reg) && "No units allowed");
if (RegisterRef::isMaskId(Reg)) {
assert(!RR.isUnit() && "No units allowed");
if (RR.isMask()) {
// XXX SLOW
const uint32_t *MB = getRegMaskBits(Reg);
const uint32_t *MB = getRegMaskBits(RR);
for (unsigned i = 1, e = TRI.getNumRegs(); i != e; ++i) {
if (MB[i / 32] & (1u << (i % 32)))
continue;
Expand All @@ -116,8 +116,8 @@ std::set<RegisterId> PhysicalRegisterInfo::getAliasSet(RegisterId Reg) const {
return AS;
}

assert(RegisterRef::isRegId(Reg));
for (MCRegAliasIterator AI(Reg, &TRI, false); AI.isValid(); ++AI)
assert(RR.isReg());
for (MCRegAliasIterator AI(RR.asMCReg(), &TRI, false); AI.isValid(); ++AI)
AS.insert(*AI);

return AS;
Expand All @@ -139,7 +139,7 @@ std::set<RegisterId> PhysicalRegisterInfo::getUnits(RegisterRef RR) const {

assert(RR.isMask());
unsigned NumRegs = TRI.getNumRegs();
const uint32_t *MB = getRegMaskBits(RR.Reg);
const uint32_t *MB = getRegMaskBits(RR);
for (unsigned I = 0, E = (NumRegs + 31) / 32; I != E; ++I) {
uint32_t C = ~MB[I]; // Clobbered regs
if (I == 0) // Reg 0 should be ignored
Expand All @@ -160,7 +160,7 @@ std::set<RegisterId> PhysicalRegisterInfo::getUnits(RegisterRef RR) const {
}

RegisterRef PhysicalRegisterInfo::mapTo(RegisterRef RR, RegisterId R) const {
if (RR.Reg == R)
if (RR.Id == R)
return RR;
if (unsigned Idx = TRI.getSubRegIndex(RegisterRef(R).asMCReg(), RR.asMCReg()))
return RegisterRef(R, TRI.composeSubRegIndexLaneMask(Idx, RR.Mask));
Expand All @@ -177,11 +177,11 @@ RegisterRef PhysicalRegisterInfo::mapTo(RegisterRef RR, RegisterId R) const {

bool PhysicalRegisterInfo::equal_to(RegisterRef A, RegisterRef B) const {
if (!A.isReg() || !B.isReg()) {
// For non-regs, or comparing reg and non-reg, use only the Reg member.
return A.Reg == B.Reg;
// For non-regs, or comparing reg and non-reg, use only the Id member.
return A.Id == B.Id;
}

if (A.Reg == B.Reg)
if (A.Id == B.Id)
return A.Mask == B.Mask;

// Compare reg units lexicographically.
Expand Down Expand Up @@ -213,14 +213,14 @@ bool PhysicalRegisterInfo::equal_to(RegisterRef A, RegisterRef B) const {

bool PhysicalRegisterInfo::less(RegisterRef A, RegisterRef B) const {
if (!A.isReg() || !B.isReg()) {
// For non-regs, or comparing reg and non-reg, use only the Reg member.
return A.Reg < B.Reg;
// For non-regs, or comparing reg and non-reg, use only the Id member.
return A.Id < B.Id;
}

if (A.Reg == B.Reg)
if (A.Id == B.Id)
return A.Mask < B.Mask;
if (A.Mask == B.Mask)
return A.Reg < B.Reg;
return A.Id < B.Id;

// Compare reg units lexicographically.
llvm::MCRegUnitMaskIterator AI(A.asMCReg(), &getTRI());
Expand Down Expand Up @@ -275,7 +275,7 @@ void PhysicalRegisterInfo::print(raw_ostream &OS, const RegisterAggr &A) const {

bool RegisterAggr::hasAliasOf(RegisterRef RR) const {
if (RR.isMask())
return Units.anyCommon(PRI.getMaskUnits(RR.Reg));
return Units.anyCommon(PRI.getMaskUnits(RR));

for (MCRegUnitMaskIterator U(RR.asMCReg(), &PRI.getTRI()); U.isValid(); ++U) {
auto [Unit, LaneMask] = *U;
Expand All @@ -288,7 +288,7 @@ bool RegisterAggr::hasAliasOf(RegisterRef RR) const {

bool RegisterAggr::hasCoverOf(RegisterRef RR) const {
if (RR.isMask()) {
BitVector T(PRI.getMaskUnits(RR.Reg));
BitVector T(PRI.getMaskUnits(RR));
return T.reset(Units).none();
}

Expand All @@ -303,7 +303,7 @@ bool RegisterAggr::hasCoverOf(RegisterRef RR) const {

RegisterAggr &RegisterAggr::insert(RegisterRef RR) {
if (RR.isMask()) {
Units |= PRI.getMaskUnits(RR.Reg);
Units |= PRI.getMaskUnits(RR);
return *this;
}

Expand Down Expand Up @@ -392,7 +392,7 @@ RegisterAggr::ref_iterator::ref_iterator(const RegisterAggr &RG, bool End)
: Owner(&RG) {
for (int U = RG.Units.find_first(); U >= 0; U = RG.Units.find_next(U)) {
RegisterRef R = RG.PRI.getRefForUnit(U);
Masks[R.Reg] |= R.Mask;
Masks[R.Id] |= R.Mask;
}
Pos = End ? Masks.end() : Masks.begin();
Index = End ? Masks.size() : 0;
Expand Down
Loading
Loading