[RegAllocFast] Document the allocation algorithm. NFC - #219835
Conversation
RegUnitStates is indexed by register unit, not by physical register, and holds a tagged union whose default case is a virtual register number. Record why that cannot collide with the RegUnitState enumerators, and that regLiveIn is a reloadAtBegin() scratch marker the backward walk never observes. allocateInstruction() described itself as a two-step algorithm while the body runs eight phases in a different order; list the phases and the constraints that fix their order. LiveOut and Reloaded are the two reasons a def must spill. Drop a redundant local in reloadAtBegin(). Aided by Opus 5
There was a problem hiding this comment.
Most of the changes make sense and are helpful, but I'm a bit concerned about scope here, as the changes seem a bit scattered (especially removing the P.PhysReg local variable).
I do agree that it's redundant but doesn't seem like it follows this PR's overall documentation theme.
|
|
||
| // Returns true if MO is tied and the operand it's tied to is not Undef (not | ||
| // Undef is not the same thing as Def). | ||
| // Returns true if MO is tied to an operand that is not undef. |
There was a problem hiding this comment.
Do you think it's worth keeping the notUndef != Def line for clarity?
There was a problem hiding this comment.
The original comment parses badly.
How about // Returns true if MO is tied to an operand that is not undef. A tie to an undef operand carries no value, so such a def needs no live-through handling.
There was a problem hiding this comment.
Better comment.
// Returns true if this def (MO) ties to a use that actually carries a value
// (not undef).
The |
…physreg-def-regallocfast.mir
MaskRay
changed the title
Disagree, I don't want to see any code changes in a documentation commit |
OK, I can extract it with the |
Extracted to #221120 |
3 participants
Add a file-level overview: block-local allocation, a stack slot for every
value crossing a block boundary, no liveness analysis or coalescing, and a
backward walk where uses acquire registers and defs release them.
Correct two stale claims: RegUnitStates is indexed by register unit, not
by physical register, and allocateInstruction() runs eight phases rather
than the two its comment described.
Drop a redundant local in reloadAtBegin(), and assert the def operand
that isTiedToNotUndef() documents.
Aided by Opus 5