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

[Balaram Makam via Phabricator via llvm-commits]

bmakam added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:953
+  BB->removeFromParent();
+  if (BB->getParent() == DestBB->getParent() && BB != DestBB)
+    DT->deleteEdge(BB, DestBB);
----------------
kuhar wrote:
> 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 :).
Its possible I might be missing something here, but if I call BB->removeFromParent() and then DT->deleteEdge(BB, DestBB) I hit the above assert for 533 lit test cases with ninja check.


https://reviews.llvm.org/D37343