[PATCH] D35391: [Dominators] Teach LoopDeletion to use the new incremental API

[Tobias Grosser via Phabricator via llvm-commits]

grosser requested changes to this revision.
grosser added a comment.
This revision now requires changes to proceed.

Nice work! This mostly looks good to me, but I have some minor question. I mark this as request changes to be notified by Phabricator in case you reply. It seems some of the email notifications don't really work at the moment.



================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:285
+  // defer it until here. We tell the DT that we delete the edge from the
+  // preaheader to the header and the incremental algorithm will automatically
+  // disconnect the rest of the loop.
----------------
preheader


================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:290
+  DT.deleteEdge(Preheader, L->getHeader());
+  DT.insertEdge(Preheader, ExitBlock);
+
----------------
I think it would be great to document this behavior in deleteEdge() and reference this behavior there! Also, do we happen to have a test case that documents this bulk removal of nodes in DT. I think this is an interesting, but not totally expected use of the deleteEdge API and it would be great if this has good test coverage.

Thanks again for working on this,


https://reviews.llvm.org/D35391