[PATCH] D98422: [Alias] Add a ah-hoc pattern with two PHI for isKnownNonEqual

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2548
+      // to the PHI's block.
+      if (Q.DT->dominates(PN1->getParent(), Pred)) {
+        // loop:
----------------
jaykang10 wrote:
> nikic wrote:
> > I don't understand why this code is needed. For your example you are comparing `%cmp.0` and `%mul = mul nsw i32 %cmp.0, 2`, which should already be recognized as non-equal. What is this backedge check for?
> There is a slightly different case as below.
> 
> ```
>         // loop_header:
>         //   %cmp.0 = phi i32 [ 3, %entry ], [ %inc, %loop_body ]
>         //   %pos.0 = phi i32 [ 1, %entry ], [ %cmp.0, %loop_body ]
>         //   ...
>         // loop_body:
>         //   ...
>         //   %inc = add i32 %cmp.0, 1
> ```
> If the `%cmp.0` is ahead of `%pos.0`, it is not easy to say that `%cmp.0` is not same with `%pos.0`. Following PHI instruction's definition which @dmgreen mentioned, the incoming value comes from incoming block. In this case, if there is backedge from incoming block, I think we can say `%pos.0` has previous iteration's `%inc` value and `%com.0` is not same with `%pos.0`. That was the idea for backedge. I am sorry for making you confused. If you feel something wrong from it, please let me know... I could be wrong...
I don't think this reasoning is generally valid, at least not without additional checks. Consider this counter-example:

```
%p1 = phi(1, %p1.next)
%p2 = phi(3, %p1)
%p1.next = or %p1, 2
```

On the first iteration `%p1 = 1, %p2 = 3`, on the second `%p1 = 3, %p2 = 1`, on the third `%p1 = 3, %p2 = 3`, so the phis are not known non-equal.


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

https://reviews.llvm.org/D98422