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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: include/llvm/Transforms/Scalar/GVNExpression.h:503
+    for (unsigned I = 0; I != getNumOperands(); ++I) {
+      if (IncomingBBs[I] != OE.IncomingBBs[I])
+        return false;
----------------
davide wrote:
> 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?
Yes, it will claim phi nodes are not congruent that are.
It should never claim that phi nodes are congruent that aren't.



https://reviews.llvm.org/D32990