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

[Ayonam Ray via Phabricator via llvm-commits]

ayonam added a comment.

In D56242#1347552 <https://reviews.llvm.org/D56242#1347552>, @lebedev.ri wrote:

> In D56242#1347548 <https://reviews.llvm.org/D56242#1347548>, @ayonam wrote:
>
> > Sorry, I somehow got the impression that you were also looking after the regression test framework and hence thought that you should review the test case part of it.
>
>
> It is usually best to add the people who last worked on the newly changed code
>  (`git diff --name-only --cached | xargs -n 1 git blame --porcelain | grep  "^author " | sort | uniq -c | sort -nr | head -10`),
>  (maybe change `--cached` to `upstream/master..HEAD`) and the code owners. (i'm neither here)


Thanks for this tip.  I will follow the protocol henceforth.

>> 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).

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.


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

https://reviews.llvm.org/D56242