[PATCH] D32990: [NewGVN] Take in account incoming edges computing congruent PhiExpression(s)

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

I'll look at the phi node story tomorrow morning early. Between cyclic args and this, they're quickly becoming a bane :)



================
Comment at: include/llvm/Transforms/Scalar/GVNExpression.h:503
+    for (unsigned I = 0; I != getNumOperands(); ++I) {
+      if (IncomingBBs[I] != OE.IncomingBBs[I])
+        return false;
----------------
dberlin wrote:
> dberlin wrote:
> > This would actually be wrong.
> > They are congruent if the same incoming bbs have the same arguments ,and for a given block, the set of incomingbbs is always the same.
> > The only issue here is the order of the incoming bbs.
> IE the following phis are congruent
> 
> phi i16 [0, %foo], [1, %bar]
> phi i16 [1, %bar], [0, %foo]
> 
Just one thing, for my understanding. When you say "wrong" you mean that's giving suboptimal answers, right? I think the approach of the original patch is actually too conservative in the sense that we miss some congruences but it should never give wrong results. Am I correct?


https://reviews.llvm.org/D32990