-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LiveRegUnits] Enhanced the register liveness check #66061
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,13 +112,13 @@ class LiveRegUnits { | |
| /// The regmask has the same format as the one in the RegMask machine operand. | ||
| void addRegsInMask(const uint32_t *RegMask); | ||
|
|
||
| /// Returns true if no part of physical register \p Reg is live. | ||
| bool available(MCPhysReg Reg) const { | ||
| for (MCRegUnit Unit : TRI->regunits(Reg)) { | ||
| if (Units.test(Unit)) | ||
| return false; | ||
| } | ||
| return true; | ||
| /// Returns true if no part of physical register \p Reg is live or reserved. | ||
| bool available(const MachineRegisterInfo &MRI, MCPhysReg Reg) const; | ||
|
|
||
| /// Returns true if any part of physical register \p Reg is live | ||
| bool contains(MCPhysReg Reg) const { | ||
| return llvm::any_of(TRI->regunits(Reg), | ||
| [&](MCRegUnit Unit) { return Units.test(Unit); }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this just needs to capture this |
||
| } | ||
|
|
||
| /// Updates liveness when stepping backwards over the instruction \p MI. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,11 @@ void LiveRegUnits::addRegsInMask(const uint32_t *RegMask) { | |
| } | ||
| } | ||
|
|
||
| bool LiveRegUnits::available(const MachineRegisterInfo &MRI, | ||
| MCPhysReg Reg) const { | ||
| return !MRI.isReserved(Reg) && !contains(Reg); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. swap the order of the checks |
||
| } | ||
|
|
||
| void LiveRegUnits::stepBackward(const MachineInstr &MI) { | ||
| // Remove defined registers and regmask kills from the set. | ||
| for (const MachineOperand &MOP : MI.operands()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7838,8 +7838,8 @@ AArch64InstrInfo::getOutlinableRanges(MachineBasicBlock &MBB, | |
| // where these registers are dead. We will only outline from those ranges. | ||
| LiveRegUnits LRU(getRegisterInfo()); | ||
| auto AreAllUnsafeRegsDead = [&LRU]() { | ||
| return LRU.available(AArch64::W16) && LRU.available(AArch64::W17) && | ||
| LRU.available(AArch64::NZCV); | ||
| return !LRU.contains(AArch64::W16) && !LRU.contains(AArch64::W17) && | ||
| !LRU.contains(AArch64::NZCV); | ||
| }; | ||
|
|
||
| // We need to know if LR is live across an outlining boundary later on in | ||
|
|
@@ -7909,7 +7909,7 @@ AArch64InstrInfo::getOutlinableRanges(MachineBasicBlock &MBB, | |
| CreateNewRangeStartingAt(MI.getIterator()); | ||
| continue; | ||
| } | ||
| LRAvailableEverywhere &= LRU.available(AArch64::LR); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the positive is available check is more readable than having to negate most of the uses |
||
| LRAvailableEverywhere &= !LRU.contains(AArch64::LR); | ||
| RangeBegin = MI.getIterator(); | ||
| ++RangeLen; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think available should stay as it is here (with rename)? A separate helper can combine the isReserved check
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason to keep this naming was to keep it analogous to the functions in LivePhysRegs (where "available" has the isReserved check and "contains" is also there) so that porting from LivePhysRegs to LiveRegUnits in other files can be smoother.
Would you have suggestions on a better name for these two functions (available and contains)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just keep the available implementation in the header, it's trivia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on keeping
available