[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:173
@@ +172,3 @@
+
+    // Scan BB2 for instructions appearing in BB1 with identical VN and hoist
+    // them at InsertPt.
----------------
dberlin wrote:
> 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)
> 
Note: I realize you have tried to limit the branch structure to cases where you can guarantee you will only value number a given BB once.
I'm saying "if anyone ever, in the future, wanted to extend this at all, they'd have to rewrite your entire algorithm right now" :)



http://reviews.llvm.org/D18798