[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 06:37:16 PST 2019


lebedev.ri added a comment.

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

> In D56242#1344627 <https://reviews.llvm.org/D56242#1344627>, @lebedev.ri wrote:
>
> > Hm, why i'm the reviewer here?
>
>
> 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)

>> That being said, is "elevation" == "hoisting" here?
>>  That might be a better, more common name..
> 
> 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?


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

https://reviews.llvm.org/D56242





More information about the llvm-commits mailing list