[llvm] r265347 - CodeGen: Remove dead code in TailDuplicate
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 4 14:11:40 PDT 2016
Author: bogner
Date: Mon Apr 4 16:11:40 2016
New Revision: 265347
URL: http://llvm.org/viewvc/llvm-project?rev=265347&view=rev
Log:
CodeGen: Remove dead code in TailDuplicate
I noticed that this isn't covered by our existing tests and spent some
time trying to come up with an example it actually hits. I tried hand
rolling something based on the explanation in the comment, but couldn't
get anything that didn't abort tail duplication earlier for one reason
or another.
Then, I tried cranking tail-dup-size cranked up so this would fire
more and ran a bootstrap of clang and the nightly test suite - those
don't hit this either.
This reverts r132816 and replaces it with an assert.
Modified:
llvm/trunk/lib/CodeGen/TailDuplication.cpp
Modified: llvm/trunk/lib/CodeGen/TailDuplication.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TailDuplication.cpp?rev=265347&r1=265346&r2=265347&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TailDuplication.cpp (original)
+++ llvm/trunk/lib/CodeGen/TailDuplication.cpp Mon Apr 4 16:11:40 2016
@@ -92,8 +92,7 @@ namespace {
MachineBasicBlock *PredBB,
DenseMap<unsigned, unsigned> &LocalVRMap,
SmallVectorImpl<std::pair<unsigned,unsigned> > &Copies,
- const DenseSet<unsigned> &UsedByPhi,
- bool Remove);
+ const DenseSet<unsigned> &UsedByPhi);
void DuplicateInstruction(MachineInstr *MI,
MachineBasicBlock *TailBB,
MachineBasicBlock *PredBB,
@@ -395,7 +394,7 @@ void TailDuplicatePass::ProcessPHI(
MachineInstr *MI, MachineBasicBlock *TailBB, MachineBasicBlock *PredBB,
DenseMap<unsigned, unsigned> &LocalVRMap,
SmallVectorImpl<std::pair<unsigned, unsigned> > &Copies,
- const DenseSet<unsigned> &RegsUsedByPhi, bool Remove) {
+ const DenseSet<unsigned> &RegsUsedByPhi) {
unsigned DefReg = MI->getOperand(0).getReg();
unsigned SrcOpIdx = getPHISrcRegOpIdx(MI, PredBB);
assert(SrcOpIdx && "Unable to find matching PHI source?");
@@ -410,9 +409,6 @@ void TailDuplicatePass::ProcessPHI(
if (isDefLiveOut(DefReg, TailBB, MRI) || RegsUsedByPhi.count(DefReg))
AddSSAUpdateEntry(DefReg, NewDef, PredBB);
- if (!Remove)
- return;
-
// Remove PredBB from the PHI node.
MI->RemoveOperand(SrcOpIdx+1);
MI->RemoveOperand(SrcOpIdx);
@@ -840,7 +836,7 @@ TailDuplicatePass::TailDuplicate(Machine
if (MI->isPHI()) {
// Replace the uses of the def of the PHI with the register coming
// from PredBB.
- ProcessPHI(MI, TailBB, PredBB, LocalVRMap, CopyInfos, UsedByPhi, true);
+ ProcessPHI(MI, TailBB, PredBB, LocalVRMap, CopyInfos, UsedByPhi);
} else {
// Replace def of virtual registers with new registers, and update
// uses with PHI source register or the new registers.
@@ -894,7 +890,7 @@ TailDuplicatePass::TailDuplicate(Machine
// Replace the uses of the def of the PHI with the register coming
// from PredBB.
MachineInstr *MI = &*I++;
- ProcessPHI(MI, TailBB, PrevBB, LocalVRMap, CopyInfos, UsedByPhi, true);
+ ProcessPHI(MI, TailBB, PrevBB, LocalVRMap, CopyInfos, UsedByPhi);
if (MI->getParent())
MI->eraseFromParent();
}
@@ -926,56 +922,16 @@ TailDuplicatePass::TailDuplicate(Machine
Changed = true;
}
- // If this is after register allocation, there are no phis to fix.
- if (!PreRegAlloc)
- return Changed;
-
- // If we made no changes so far, we are safe.
- if (!Changed)
- return Changed;
-
-
- // Handle the nasty case in that we duplicated a block that is part of a loop
- // into some but not all of its predecessors. For example:
- // 1 -> 2 <-> 3 |
- // \ |
- // \---> rest |
- // if we duplicate 2 into 1 but not into 3, we end up with
- // 12 -> 3 <-> 2 -> rest |
- // \ / |
- // \----->-----/ |
- // If there was a "var = phi(1, 3)" in 2, it has to be ultimately replaced
- // with a phi in 3 (which now dominates 2).
- // What we do here is introduce a copy in 3 of the register defined by the
- // phi, just like when we are duplicating 2 into 3, but we don't copy any
- // real instructions or remove the 3 -> 2 edge from the phi in 2.
- for (SmallSetVector<MachineBasicBlock *, 8>::iterator PI = Preds.begin(),
- PE = Preds.end(); PI != PE; ++PI) {
- MachineBasicBlock *PredBB = *PI;
- if (std::find(TDBBs.begin(), TDBBs.end(), PredBB) != TDBBs.end())
- continue;
-
- // EH edges
- if (PredBB->succ_size() != 1)
- continue;
-
- DenseMap<unsigned, unsigned> LocalVRMap;
- SmallVector<std::pair<unsigned,unsigned>, 4> CopyInfos;
- MachineBasicBlock::iterator I = TailBB->begin();
- // Process PHI instructions first.
- while (I != TailBB->end() && I->isPHI()) {
- // Replace the uses of the def of the PHI with the register coming
- // from PredBB.
- MachineInstr *MI = &*I++;
- ProcessPHI(MI, TailBB, PredBB, LocalVRMap, CopyInfos, UsedByPhi, false);
- }
- MachineBasicBlock::iterator Loc = PredBB->getFirstTerminator();
- for (unsigned i = 0, e = CopyInfos.size(); i != e; ++i) {
- Copies.push_back(BuildMI(*PredBB, Loc, DebugLoc(),
- TII->get(TargetOpcode::COPY),
- CopyInfos[i].first).addReg(CopyInfos[i].second));
- }
- }
+ // TODO: We shouldn't need this assert. It's here to be defensive about
+ // removing the logic from r132816, which handled an unreachable case.
+ assert((!PreRegAlloc || !Changed ||
+ std::all_of(Preds.begin(), Preds.end(),
+ [&TDBBs](MachineBasicBlock *PredBB) {
+ return (std::find(TDBBs.begin(), TDBBs.end(), PredBB) !=
+ TDBBs.end()) ||
+ PredBB->succ_size() != 1;
+ })) &&
+ "loop block duplicated into some but not all predecessors");
return Changed;
}
More information about the llvm-commits
mailing list