[PATCH] D39483: [CVP] Remove some {s|u}add.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:333
+    return false;
+  case Intrinsic::uadd_with_overflow:
+  case Intrinsic::sadd_with_overflow:
----------------
I don't think you need this either -- you can just directly go to the switch below.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:337
+  }
+  auto NeverOverflows = [&] (Value *LHS, Value *RHS, unsigned NoWrapKind) {
+    ConstantRange RRange = LVI->getConstantRange(RHS, II->getParent(), II);
----------------
How about calling this `NoWrapOnAddition` ?


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:351
+  APInt UMax = APInt::getMaxValue(RHSType->getBitWidth());
+  APInt SMax = APInt::getSignedMaxValue(RHSType->getBitWidth());
+  APInt SMin = APInt::getSignedMinValue(RHSType->getBitWidth());
----------------
You don't need these anymore.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:364
+
+static void processOverflowIntrinsic(IntrinsicInst *II) {
+  Value *NewOp = nullptr;
----------------
`process` is too vague -- how about `eraseOverflowIntrinsic`?


================
Comment at: test/Transforms/CorrelatedValuePropagation/overflows.ll:183
+define i32 @unsigned_add_r1(i32 %x) {
+; CHECK: define i32 @unsigned_add_r1
+; CHECK-NOT: @llvm.uadd.with.overflow.i32
----------------
Typical idiom is to use `CHECK-LABEL` for the function name, as:

```
; CHECK-LABEL: @unsigned_add_r1(
; CHECK-NOT: ...
```



https://reviews.llvm.org/D39483