[PATCH] D156880: [AggressiveAntiDepBreaker] Refix renaming a subregister of a live register
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 2 05:13:00 PDT 2023
foad created this revision.
foad added reviewers: kparzysz, MatzeB, qcolombet, fhahn, arsenm.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
foad requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.
This patch reworks the fix from D20627 <https://reviews.llvm.org/D20627> "Do not rename registers that do
not start an independent live range". That fix depended on the scheduler
dependency graph having redundant edges. Those edges are removed by
D156552 <https://reviews.llvm.org/D156552> "[MachineScheduler] Track physical register dependencies
per-regunit" with the result that on several Hexagon lit tests, the
post-RA scheduler would schedule the code in a way that fails machine
verification.
Consider this code where D11 <https://reviews.llvm.org/D11> is a pair R23:R22:
SU(0): %R2 <https://reviews.llvm.org/source/clang-tools-extra/><def> = A2_add %R23, %R17 <https://reviews.llvm.org/source/LLVM.org/><kill>
(anti dependency on R23 here)
SU(8): %R23<def> = S2_asr_i_r %R22, 31
(data dependency on R23->D11 here)
SU(10): %D0<def> = A2_tfrp %D11 <https://reviews.llvm.org/D11><kill>
The original fix would detect this situation by examining the dependency
from SU(8) to SU(10) and seeing that D11 <https://reviews.llvm.org/D11> is not a subreg of R23.
A slightly more complicated example:
SU(0): %R2 <https://reviews.llvm.org/source/clang-tools-extra/><def> = A2_add %R23, %R17 <https://reviews.llvm.org/source/LLVM.org/><kill>
(anti dependency on R23 here)
SU(8): %R23<def> = S2_asr_i_r %R22, 31
(data dependency on R23 here)
SU(9): %R23<def> = S2_asr_i_r %R23, 31
(data dependency on R23->D11 here)
SU(10): %D0<def> = A2_tfrp %D11 <https://reviews.llvm.org/D11><kill>
The original fix also worked on this example, but only because
ScheduleDAGInstrs adds an extra data dependency edge directly from SU(8)
to SU(10). This edge is redundant, since you could infer it transitively
from the edges SU(8)->SU(9) and SU(9)->SU(10), and since none of the
data that SU(8) writes to R23 is read by SU(10).
After D156552 <https://reviews.llvm.org/D156552> the redundant edge SU(8)->SU(10) will not be present, so
when we examine the successors of SU(8) we will not find any that read
from a superreg of R23.
This patch removes the original fix from D20627 <https://reviews.llvm.org/D20627>, which examined edges in
the dependency graph. Instead it extends a check that was already being
done in FindSuitableFreeRegisters: instead of checking that *some*
register is a superreg of all registers in the rename group, we now
check that the specific register that carries the anti-dependency that
we want to break is a superreg of all registers in the rename group.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D156880
Files:
llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
Index: llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
===================================================================
--- llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
+++ llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
@@ -176,8 +176,9 @@
std::set<unsigned> &PassthruRegs);
void ScanInstruction(MachineInstr &MI, unsigned Count);
BitVector GetRenameRegisters(unsigned Reg);
- bool FindSuitableFreeRegisters(unsigned AntiDepGroupIndex,
- RenameOrderType& RenameOrder,
+ bool FindSuitableFreeRegisters(unsigned SuperReg,
+ unsigned AntiDepGroupIndex,
+ RenameOrderType &RenameOrder,
std::map<unsigned, unsigned> &RenameMap);
};
Index: llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
===================================================================
--- llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
+++ llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
@@ -533,9 +533,8 @@
}
bool AggressiveAntiDepBreaker::FindSuitableFreeRegisters(
- unsigned AntiDepGroupIndex,
- RenameOrderType& RenameOrder,
- std::map<unsigned, unsigned> &RenameMap) {
+ unsigned SuperReg, unsigned AntiDepGroupIndex, RenameOrderType &RenameOrder,
+ std::map<unsigned, unsigned> &RenameMap) {
std::vector<unsigned> &KillIndices = State->GetKillIndices();
std::vector<unsigned> &DefIndices = State->GetDefIndices();
std::multimap<unsigned, AggressiveAntiDepState::RegisterReference>&
@@ -550,17 +549,12 @@
if (Regs.empty())
return false;
- // Find the "superest" register in the group. At the same time,
- // collect the BitVector of registers that can be used to rename
+ // Collect the BitVector of registers that can be used to rename
// each register.
LLVM_DEBUG(dbgs() << "\tRename Candidates for Group g" << AntiDepGroupIndex
<< ":\n");
std::map<unsigned, BitVector> RenameRegisterMap;
- unsigned SuperReg = 0;
for (unsigned Reg : Regs) {
- if ((SuperReg == 0) || TRI->isSuperRegister(SuperReg, Reg))
- SuperReg = Reg;
-
// If Reg has any references, then collect possible rename regs
if (RegRefs.count(Reg) > 0) {
LLVM_DEBUG(dbgs() << "\t\t" << printReg(Reg, TRI) << ":");
@@ -892,30 +886,8 @@
}
}
- if (AntiDepReg == 0) continue;
-
- // If the definition of the anti-dependency register does not start
- // a new live range, bail out. This can happen if the anti-dep
- // register is a sub-register of another register whose live range
- // spans over PathSU. In such case, PathSU defines only a part of
- // the larger register.
- RegAliases.reset();
- for (MCRegAliasIterator AI(AntiDepReg, TRI, true); AI.isValid(); ++AI)
- RegAliases.set(*AI);
- for (SDep S : PathSU->Succs) {
- SDep::Kind K = S.getKind();
- if (K != SDep::Data)
- continue;
- unsigned R = S.getReg();
- if (!RegAliases[R])
- continue;
- if (TRI->isSubRegisterEq(AntiDepReg, R))
- continue;
- AntiDepReg = 0;
- break;
- }
-
- if (AntiDepReg == 0) continue;
+ if (AntiDepReg == 0)
+ continue;
}
assert(AntiDepReg != 0);
@@ -931,7 +903,8 @@
// Look for a suitable register to use to break the anti-dependence.
std::map<unsigned, unsigned> RenameMap;
- if (FindSuitableFreeRegisters(GroupIndex, RenameOrder, RenameMap)) {
+ if (FindSuitableFreeRegisters(AntiDepReg, GroupIndex, RenameOrder,
+ RenameMap)) {
LLVM_DEBUG(dbgs() << "\tBreaking anti-dependence edge on "
<< printReg(AntiDepReg, TRI) << ":");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156880.546416.patch
Type: text/x-patch
Size: 4017 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230802/775f1c20/attachment.bin>
More information about the llvm-commits
mailing list