[llvm] 97324f6 - [AggressiveAntiDepBreaker] Refix renaming a subregister of a live register
Jay Foad via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 7 07:41:52 PDT 2023
Author: Jay Foad
Date: 2023-08-07T15:41:40+01:00
New Revision: 97324f6274184e607fa6d6cffb1aebee317d4644
URL: https://github.com/llvm/llvm-project/commit/97324f6274184e607fa6d6cffb1aebee317d4644
DIFF: https://github.com/llvm/llvm-project/commit/97324f6274184e607fa6d6cffb1aebee317d4644.diff
LOG: [AggressiveAntiDepBreaker] Refix renaming a subregister of a live register
This patch reworks the fix from 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 "[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 is a pair R23:R22:
SU(0): %R2<def> = A2_add %R23, %R17<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<kill>
The original fix would detect this situation by examining the dependency
from SU(8) to SU(10) and seeing that D11 is not a subreg of R23.
A slightly more complicated example:
SU(0): %R2<def> = A2_add %R23, %R17<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<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 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, 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.
Differential Revision: https://reviews.llvm.org/D156880
Added:
Modified:
llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
index d06c4327bda6ac..c5367221cae7a7 100644
--- a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
+++ b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
@@ -533,9 +533,8 @@ BitVector AggressiveAntiDepBreaker::GetRenameRegisters(unsigned Reg) {
}
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 @@ bool AggressiveAntiDepBreaker::FindSuitableFreeRegisters(
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 @@ unsigned AggressiveAntiDepBreaker::BreakAntiDependencies(
}
}
- 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 @@ unsigned AggressiveAntiDepBreaker::BreakAntiDependencies(
// 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) << ":");
diff --git a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.h b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
index cece217e645cdc..06c4c6957ba043 100644
--- a/llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
+++ b/llvm/lib/CodeGen/AggressiveAntiDepBreaker.h
@@ -176,8 +176,9 @@ class LLVM_LIBRARY_VISIBILITY AggressiveAntiDepState {
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);
};
More information about the llvm-commits
mailing list