[PATCH] D37343: [CGP] Merge empty case blocks if no extra moves are added.

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:953
+  BB->removeFromParent();
+  if (BB->getParent() == DestBB->getParent() && BB != DestBB)
+    DT->deleteEdge(BB, DestBB);
----------------
bmakam wrote:
> bmakam wrote:
> > efriedma wrote:
> > > "BB->getParent() == DestBB->getParent()" still here.  What is that trying to check for?
> > err, I was trying to avoid hitting this assert from DT->deleteEdge:
> >  void llvm::DominatorTreeBase<llvm::BasicBlock, false>::deleteEdge(NodeT *, NodeT *) [NodeT = llvm::BasicBlock, IsPostDom = false]: Assertion `From->getParent() == Parent' failed.
> > 
> > But looks like this code is unreachable, so I don't need to delete the edge BB, DestBB after BB->eraseFromParent()?
> I think the right way to delete the edge is
> 1. BB->getTerminator()->eraseFromParent()
> 2. DT->deleteEdge(BB, DestBB)
> 3. BB->eraseFromParent()
> 
> This way I don't hit the assert and I can delete the edge in DT.
Are you sure that calling `BB->removeFromParent()` actually sets the BB's Parent pointer? The sequence of calls I mention is used in a couple of places quite some time now.

Your method looks correct as well, but I wouldn't call it the most intuitive one :).


https://reviews.llvm.org/D37343