[PATCH] D39483: [CVP] Remove some {s|u}{add|sub}.with.overflow checks.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:329
+// See if we can prove that the given overflow intrinsic will not overflow.
+static bool canRemoveOverflowCheck(IntrinsicInst *II, LazyValueInfo *LVI) {
+  switch (II->getIntrinsicID()) {
----------------
I'd call this `willNotOverflow` instead, since that's what this is computing.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:382
+    if (ConstantInt *RHSConstant = dyn_cast<ConstantInt>(RHS))
+      if ((RHSConstant->getValue().isStrictlyPositive() &&
+           Holds(ICmpInst::ICMP_SGE, LHS,
----------------
I'd avoid adding this kind of logic here.  Instead I think the right factoring is to start off by doing what `ConstantRange` lets us do today (i.e. sadd and uadd) and then improving `ConstantRange` to handle the subtraction (and other) cases you're interested in.  Putting this logic in ConstantRange will make it more likely that the code is re-used (besides making it easier to test).


https://reviews.llvm.org/D39483