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

[Sebastian Pop via llvm-commits]

sebpop added a comment.

The optimistic approach Danny asked for is implemented in http://reviews.llvm.org/D19338


================
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.
----------------
hiraditya wrote:
> dberlin wrote:
> > 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" :)
> > 
> Thanks for the feedback. Yes, in the current implementation we value number each instruction only once. When an expression is hoisted still the value numbers are not invalidated because one of the expressions in the child branch gets hoisted.
> 
> So, an extension of this algorithm could be to start hoisting equivalent (scalar) expressions across non-sibling branches. In that case, I think, we would still value number only once, if we hoist one of the expressions and delete the rest. For loads, or other expressions with side-effects, hoisting would require more complicated analysis across each edge dominated by the common dominator of the equivalent expressions, so maybe we could skip that to save compile time, I'm not very sure about this though. Could you give an example of a case when multiple value numbering of same expression will be required, that will be helpful.
> "[...] rewrite your entire algorithm right now" :)

Danny knows how to persuade people on doing what he wants ;-)


http://reviews.llvm.org/D18798