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

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Analysis/ValueTracking.cpp:3823
@@ +3822,3 @@
+  // equivalent to reduce the amount of logic needed.
+  APred = ICmpInst::getUnsignedPredicate(APred);
+  BPred = ICmpInst::getUnsignedPredicate(BPred);
----------------
This scares me.  Can you use a different variable name for `APred` and `BPred` here, like `UnsignedAPred`?

================
Comment at: lib/Analysis/ValueTracking.cpp:3850
@@ +3849,3 @@
+    if (BPred == CmpInst::ICMP_EQ ||  // A > B implies A == B is false.
+        BPred == CmpInst::ICMP_ULE) { // A > B implies A <= B is false.
+      ImpliedTrue = false;
----------------
Won't this return true for `A ugt B => NOT(A sle B)` (since you unconditionally coerced both to the unsigned variants) (consider A = -1, B = 0)?


http://reviews.llvm.org/D18905