[PATCH] D32252: [GVN] Add phi-translate for scalarpre as a temporary solution

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 22:41:40 PDT 2017


wmi added a comment.

Daniel, thanks for the comments.

> There is, strangely, a doPhiTranslation function that Value has, you can use that :)

Yes, I can use it to simplify phiTranslateImpl a little.

> Newgvn gets this example case without PRE with my latest patch. It understand the equivalence between op of phis and phi of ops

Good job for newgvn :-).

> I suspect you will have issues with backedges with this approach. I'm pretty sure it will fail in some cases to fixpoint (but maybe gvn is not strong enough to make that happen)
>  At the very least, you don't want this to transform:

There is code in performScalarPRE checking whether the predecessor is the same as current bb and giving up if it is true. I tried small testcase and it seemed fine. I will pay attention to it when exercise it with more tests.

> This also needs compile time and performance measurements.

Sure. I will do it.


Repository:
  rL LLVM

https://reviews.llvm.org/D32252





More information about the llvm-commits mailing list