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

[Sanjoy Das via llvm-commits]

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

================
Comment at: include/llvm/Analysis/ValueTracking.h:433
@@ +432,3 @@
+  /// Return true if RHS is known to be implied by LHS.  The implication may be
+  /// either true or false depending on the value of ImpliedCond.
+  /// A & B must be i1 (boolean) values or a vector of such values. Note that
----------------
I'd use something a little more descriptive -- maybe `ImpliedTrue`?

The Right(TM) fix here is to change `isImpliedCondition` to return `Optional<bool>`, but I don't know how much work that will be.

================
Comment at: lib/Analysis/ValueTracking.cpp:3796
@@ +3795,3 @@
+    case CmpInst::ICMP_SLT:
+    case CmpInst::ICMP_SLE: {
+      ImpliedCond = false;
----------------
I'm not sure this is correct for "or equal" predicates -- `A >= B` does not imply `!(B >= A)` since if `A == B` then both of these are true.

================
Comment at: lib/Analysis/ValueTracking.cpp:3799
@@ +3798,3 @@
+      return true;
+    }
+    }
----------------
Minor: no braces need here.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2733
@@ -2730,2 +2732,3 @@
     auto *OldCond = BI->getCondition();
-    BI->setCondition(ConstantInt::getTrue(BB->getContext()));
+    static ConstantInt *CI = ImpliedCond
+                                 ? ConstantInt::getTrue(BB->getContext())
----------------
Huh?  Why `static` ?


Repository:
  rL LLVM

http://reviews.llvm.org/D18905