[PATCH] D24805: [GVNSink] Initial GVNSink prototype

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/GVNSink.cpp:869
+    if (!PDT->getRootNode())
+      // This can happen if the function has no returns.
+      return {0, 0};
----------------
This is just flat out broken :)


================
Comment at: lib/Transforms/Scalar/GVNSink.cpp:871
+      return {0, 0};
+    for (auto &N : depth_first(PDT)) {
+      if (!N->getBlock())
----------------
efriedma wrote:
> efriedma wrote:
> > Could you use `post_order(F)` instead of PDT here?  If not, could you add a comment explaining what properties of the post-dominator tree you're depending on?
> Err, post_order() isn't right; I guess you want a reverse post-order traversal on the inverted graph, or something like that?
Yes, it's RPO on the inverse graph.



Repository:
  rL LLVM

https://reviews.llvm.org/D24805