[PATCH] D18905: [ValueTracking] Improve isImpliedCondition for conditions with matching operands.

[Geoff Berry via llvm-commits]

gberry added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3803
@@ +3802,3 @@
+    return false;
+
+  // If the operands are swapped and the predicates match we can infer the
----------------
Perhaps this function could be simplified by canonicalizing to the IsMatchingOps case at the beginning:

```
if (IsSwappedOps) {
  std::swap(BLHS, BRHS);
  BPred = ICmpInst::getSwappedPredicate(BPred);
}
```

Then you don't need to consider the IsSwappedOps cases below.

================
Comment at: lib/Analysis/ValueTracking.cpp:3920
@@ +3919,3 @@
+  /// comparison (which is signless).
+  if (ICmpInst::isSigned(APred) != ICmpInst::isSigned(BPred) &&
+      !ICmpInst::isEquality(APred) && !ICmpInst::isEquality(BPred))
----------------
This check seems like it should be pushed down into isImpliedCondMatchingOperands since it doesn't necessarily apply to isImpliedCondOperands?  I feel like maybe I'm missing something here.

================
Comment at: lib/Analysis/ValueTracking.cpp:3928
@@ +3927,3 @@
+
+  if (APred == BPred) {
+    ImpliedTrue = true;
----------------
I don't follow why this check needs to be added when isImpliedCondOperands has not been changed.


http://reviews.llvm.org/D18905