[PATCH] D56242: Elevate instructions across if-else blocks for better constant propagation

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 6 11:00:25 PST 2019


lebedev.ri added a comment.

In D56242#1347585 <https://reviews.llvm.org/D56242#1347585>, @ayonam wrote:

> In D56242#1347552 <https://reviews.llvm.org/D56242#1347552>, @lebedev.ri wrote:
>
> > In D56242#1347548 <https://reviews.llvm.org/D56242#1347548>, @ayonam wrote:
> >
> > > Yes, it actually is so.  But since there was already a pass by that name, I thought I'd name mine differently so as not to confuse one with the other.  Any suggestions to rename the new pass is most welcome.
> >
> >
> > I would personally think one wouldn't want to end up with dozens of similar passes doing essentially the same thing, but just slightly differently.
> >  Have you looked at the existing pass(es)? How are they different from this code? Perhaps it would be best to extend the existing passes?
>
>
> Yes, I too am of the same opinion.  However, the other two passes that hoist code are ConstantHoisting and GVNHoisting.  While the former hoists constants the latter hoists expressions from branches to the common dominator.  What the Elevate pass does is to hoist expressions from a join block to its predecessors if the expressions can be constant evaluated.  (In effect, it does the reverse of what the Sink pass does - to sink expressions into the join block from its predecessors).


All that is a very good point, and i think it even reinforces my previous point.
I think it would be best to be consistent, and call this `InstructionHoisting`.

> I feel that this is a different kind of analysis than the ones that are already being done for the other two hoisting passes and hence it is better if we have a separate pass for this.  We might later want to coalesce all the code hoisting passes into one single pass (like we have in SimplifyCFGPass), but I would not like to venture into it for now.

Yes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56242/new/

https://reviews.llvm.org/D56242





More information about the llvm-commits mailing list