[PATCH] D17184: [LVI] Extend select handling to catch min/max/clamp idioms

[Sanjoy Das via llvm-commits]

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

================
Comment at: lib/Analysis/LazyValueInfo.cpp:972
@@ +971,3 @@
+    if (ConstantInt *CIBase = dyn_cast<ConstantInt>(ICI->getOperand(1))) {
+      auto addConstants = [](ConstantInt *A, ConstantInt *B) {
+        assert(A->getType() == B->getType());
----------------
Nit: should be `AddConstants`.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:979
@@ +978,3 @@
+      case ICmpInst::ICMP_EQ:
+        if (match(SI->getFalseValue(), m_Add(m_Specific(A),
+                                             m_ConstantInt(CIAdded)))) {
----------------
This would be easier to follow if you summarize the transform you're doing / pattern you're matching, like

`select (A == CIBase) _ (A + CIAdded)`

================
Comment at: lib/Analysis/LazyValueInfo.cpp:982
@@ +981,3 @@
+          auto ResNot = addConstants(CIBase, CIAdded);
+          FalseVal = intersect(FalseVal,
+                               LVILatticeVal::getNot(ResNot));
----------------
How can you infer this without looking at the `SI->getTrueValue()`?  Can't the select be `select (A == CIBase) (CIBase + CIAdded) (A + CIAdded)`?  Or is there a check somewhere that I missed?

================
Comment at: lib/IR/ConstantRange.cpp:717
@@ -716,1 +716,3 @@
 ConstantRange
+ConstantRange::smin(const ConstantRange &Other) const {
+  // X smax Y is: range(smin(X_smin, Y_smin),
----------------
These should get tests in `unittests/IR/ConstantRange.cpp`.  I'd say split these out, add tests and land them -- the `smin` and `umin` implementations lgtm in their current form (with the edit @ab suggested).


http://reviews.llvm.org/D17184