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

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with two nits inline

Other than that  I'll urge you to try to refactor the tricky predicate logic in `isImpliedCondMatchingOperands` into `CmpInst` soon.


================
Comment at: lib/Analysis/ValueTracking.cpp:3829
@@ +3828,3 @@
+  // comparison (which is signless).
+  if (!PredicatesComparable(APred, BPred))
+    return false;
----------------
Nit: I'd just inline the body here.  `PredicatesComparable` isn't terribly descriptive or general, and the function is a two liner anyway.

================
Comment at: lib/Analysis/ValueTracking.cpp:3842
@@ +3841,3 @@
+  if (CmpInst::getInversePredicate(UnsignedAPred) == UnsignedBPred) {
+    switch (UnsignedAPred) {
+    default:
----------------
Please verify this, but I don't think you need to enumerate the predicates here, and you don't even need to guard this under `PredicatesComparable`.  You can just straightforwardly do;

```
if (CmpInst::getInversePredicate(UnsignedAPred) == UnsignedBPred) {
  ImpliedTrue = false;
  return true;
}
```



http://reviews.llvm.org/D18905