[PATCH] D36172: [InstCombine] Improve profitability check for folding PHI args

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Well, I'd be fine with NewGVN doing this transform.  But at least right now, it doesn't appear to be doing so -- running the new test case (@phi_multi in phi.ll) through "opt -O2 --enable-newgvn" does not move the zext/or out of the loop.   Is this supposed to happen?

As to compile time, I didn't notice any increase in real-world tests.  While it is true that the new check can be quadratic in the worst case, there are several early exits that are actually taken most of time, so *usually* the check has the same complexity as the hasOneUse test it removed.   Only in the case where a value is used in more than one PHI node do we even start the more expensive check ...


https://reviews.llvm.org/D36172