[RDF] RegisterRef/RegisterId improvements. NFC #168030
Merged
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
RegisterId can represent a physical register, a MCRegUnit, or an index into a side structure that stores register masks. These 3 types were encoded by using the physical reg, stack slot, and virtual register encoding partitions from the Register class. This encoding scheme alias wasn't well contained so Register::index2StackSlot and Register::stackSlotIndex appeared in multiple places. This patch gives RegisterRef its own encoding defines and separates it from Register. I've removed the generic idx() method in favor of getAsMCReg(), getAsMCRegUnit(), and getMaskIdx() for some degree of type safety. Some places used the RegisterId field of RegisterRef directly as a register. Those have been updated to use getAsMCReg. Some special cases for RegisterId 0 have been removed as it can be treated like a MCRegister by existing code. I think I want to rename the Reg field of RegisterRef to Id, but I'll do that in another patch. Additionally, callers of the RegisterRef constructor need to be audited for implicit conversions from Register/MCRegister to unsigned.
topperc
changed the title
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
topperc
commented
Nov 14, 2025
| std::set<RegisterId> PhysicalRegisterInfo::getUnits(RegisterRef RR) const { | ||
| std::set<RegisterId> Units; | ||
|
|
||
| if (RR.Reg == 0) |
Collaborator
Author
There was a problem hiding this comment.
RegisterId 0 is considered a register and the mask is forced to none, so we can let the RR.isReg(), RR.Mask.none() handle this.
topperc
commented
Nov 14, 2025
| if (A.isReg()) { | ||
| MCRegister Reg = A.asMCReg(); | ||
| if (Reg && Reg.id() < TRI.getNumRegs()) | ||
| OS << TRI.getName(Reg); |
Collaborator
Author
There was a problem hiding this comment.
We can probably use printReg for all cases, but it would slightly change the formatting so I didn't include it here.
| static constexpr RegisterId toUnitId(unsigned Idx) { | ||
| return Idx | Register::VirtualRegFlag; | ||
| } | ||
| static constexpr RegisterId toUnitId(unsigned Idx) { return Idx | UnitFlag; } |
Contributor
| return RegisterRef::toMaskId(RegMasks.find(RM)); | ||
| } | ||
|
|
||
| const uint32_t *getRegMaskBits(RegisterId R) const { |
Contributor
There was a problem hiding this comment.
I was thinking about passing RegisterRef to this, getMaskUnits and getAliasSet to avoid reconstructing RegisterRef
But LGTM as is
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.
RegisterId can represent a physical register, a MCRegUnit, or
an index into a side structure that stores register masks. These 3
types were encoded by using the physical reg, stack slot, and
virtual register encoding partitions from the Register class.
This encoding scheme alias wasn't well contained so
Register::index2StackSlot and Register::stackSlotIndex appeared
in multiple places.
This patch gives RegisterRef its own encoding defines and separates
it from Register.
I've removed the generic idx() method in favor of getAsMCReg(),
getAsMCRegUnit(), and getMaskIdx() for some degree of type safety.
Some places used the RegisterId field of RegisterRef directly as a
register. Those have been updated to use getAsMCReg.
Some special cases for RegisterId 0 have been removed as it can
be treated like a MCRegister by existing code.
I think I want to rename the Reg field of RegisterRef to Id, but
I'll do that in another patch.
Additionally, callers of the RegisterRef constructor need to be
audited for implicit conversions from Register/MCRegister
to unsigned.