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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:350
+    if (RHSConstant->isZero())
+      return true;
+  switch (II->getIntrinsicID()) {
----------------
This case should be getting constant folded.  If it isn't, that's an InstCombine bug.  Once you remove this, you can just directly have the second switch.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:355
+  case Intrinsic::uadd_with_overflow:
+    if (ConstantInt *LHSConstant = dyn_cast<ConstantInt>(LHS))
+      if (LHSConstant->isZero())
----------------
We should be canonicalizing commutative ops so that if there is a constant operand it is the RHS (if not, that's again an instcombine bugs).


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:376
+           Holds(ICmpInst::ICMP_SGE, RHS, ConstantInt::get(RHSType, 0))) ||
+          (LHSConstant->isMaxValue(true) &&
+           Holds(ICmpInst::ICMP_SLE, RHS, ConstantInt::get(RHSType, 0))))
----------------
Can you avoid doing this stuff by hand by using the `LazyValueInfo::getConstantRange` and the `ConstantRange::makeGuaranteedNoWrapRegion` APIs?


https://reviews.llvm.org/D39483