[PATCH] D98422: [ValueTracking] Handle two PHIs in isKnownNonEqual()

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2548
+  if (Depth >= MaxAnalysisRecursionDepth - 1)
+    return false;
+
----------------
nikic wrote:
> Ah sorry, this isn't what I meant. What the other code does is pass `MaxAnalysisRecursionDepth - 1` to the recursive call (i.e. the recursive `isKnownNonEqual` call in this case). I just tried this, but unfortunately this would not cover your case (it needs to recurse two more levels).
um... only one pair of phi operands can use full recursion...

```
  bool UsedFullRecursion = false;
  for (const BasicBlock *IncomBB : PN1->blocks()) {
    if (!VisitedBBs.insert(IncomBB).second)
      continue; // Don't reprocess blocks that we have dealt with already.
    const Value *IV1 = PN1->getIncomingValueForBlock(IncomBB);
    const Value *IV2 = PN2->getIncomingValueForBlock(IncomBB);
    const ConstantInt *C1 = dyn_cast<ConstantInt>(IV1);
    const ConstantInt *C2 = dyn_cast<ConstantInt>(IV2);
    if (C1 && C2) {
      if (C1->getValue().eq(C2->getValue()))
        return false;
    } else {
      // Only one pair of phi operands is allowed for full recursion.
      if (UsedFullRecursion)
        return false;

      Query RecQ = Q; 
      RecQ.CxtI = IncomBB->getTerminator();
      if (!isKnownNonEqual(IV1, IV2, Depth + 1, RecQ))
        return false;
      UsedFullRecursion = true;
    }    
  }
```
How you you think about above one? Is it acceptable?


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

https://reviews.llvm.org/D98422