diff options
author | Justin Bogner <mail@justinbogner.com> | 2016-04-04 21:11:40 +0000 |
---|---|---|
committer | Justin Bogner <mail@justinbogner.com> | 2016-04-04 21:11:40 +0000 |
commit | 9ab8131a57ed4cecfe00225a56f57d5173b80b2e (patch) | |
tree | 2d054ca46409a4874992c31be4048841b2f6c463 /llvm/lib/CodeGen/TailDuplication.cpp | |
parent | 7ddec63d8fe687fba5c01eee84f450678521181c (diff) | |
download | bcm5719-llvm-9ab8131a57ed4cecfe00225a56f57d5173b80b2e.tar.gz bcm5719-llvm-9ab8131a57ed4cecfe00225a56f57d5173b80b2e.zip |
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.
llvm-svn: 265347
Diffstat (limited to 'llvm/lib/CodeGen/TailDuplication.cpp')
-rw-r--r-- | llvm/lib/CodeGen/TailDuplication.cpp | 72 |
1 files changed, 14 insertions, 58 deletions
diff --git a/llvm/lib/CodeGen/TailDuplication.cpp b/llvm/lib/CodeGen/TailDuplication.cpp index 2a8431c2bab..7f135be9f0f 100644 --- a/llvm/lib/CodeGen/TailDuplication.cpp +++ b/llvm/lib/CodeGen/TailDuplication.cpp @@ -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(MachineBasicBlock *TailBB, 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(MachineBasicBlock *TailBB, // 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(MachineBasicBlock *TailBB, 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; } |