[PATCH] D18798: New code hoisting pass based on GVN

[Daniel Berlin via llvm-commits]

dberlin added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:113
@@ +112,3 @@
+    for (auto *BB : *Dom)
+      hoistExpressions(BB);
+
----------------
You may just want to use df_iterator over the dom tree instead of recursion.  It's easier to follow, and won't blow out the stack :)
If you use recursion, you have to or together all these values.


================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:173
@@ +172,3 @@
+
+    // Scan BB2 for instructions appearing in BB1 with identical VN and hoist
+    // them at InsertPt.
----------------
This still value numbers bbs unnecessarily, and can try to do so multiple times depending on the branch structure.

The way i suggested only value numbers a given BB once :)

Let me suggest a simpler way than rewriting the algorithm entirely (as i suggested):

value number everything up front, store a sorted set bbtovalues, keeping a list of value numbers that exist in each bb.

Instead of walking instructions in this algorithm, intersect bbtovalues. For each VN in the intersection, see if the expressions for that VN that occur in each BB can be hoisted.

(this will only ever walk the expressions you might hoist, instead of try to value number every expression again)



http://reviews.llvm.org/D18798