[PATCH] D60420: [ValueTracking] Use computeConstantRange() for signed add/sub overflow determination

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Please can you split up your changes?
https://llvm.org/docs/DeveloperPolicy.html#code-reviews

> Split your patch into multiple smaller patches that build on each other. The smaller your patch, the higher the probability that somebody will take a quick look at it.

I would very much prefer to quickly look through 3 small differentials here rather than one medium..



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4087-4091
+  ConstantRange CR1 = ConstantRange::fromKnownBits(Known, ForSigned);
+  ConstantRange CR2 = computeConstantRange(V, UseInstrInfo);
+  ConstantRange::PreferredRangeType RangeType =
+      ForSigned ? ConstantRange::Signed : ConstantRange::Unsigned;
+  return CR1.intersectWith(CR2, RangeType);
----------------
So the only thing happening here is that `RangeType` is now being passed, right?
Please, precommit the NFC part of the refactoring, and leave only
```
+ ConstantRange::PreferredRangeType RangeType =
+     ForSigned ? ConstantRange::Signed : ConstantRange::Unsigned;
- return CR1.intersectWith(CR2);
+ return CR1.intersectWith(CR2, RangeType);
```


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4136-4159
+  ConstantRange LHSRange = computeConstantRangeIncludingKnownBits(
+      LHS, /*ForSigned=*/true, DL, /*Depth=*/0, AC, CxtI, DT);
+  ConstantRange RHSRange = computeConstantRangeIncludingKnownBits(
+      RHS, /*ForSigned=*/true, DL, /*Depth=*/0, AC, CxtI, DT);
   OverflowResult OR =
       mapOverflowResult(LHSRange.signedAddMayOverflow(RHSRange));
   if (OR != OverflowResult::MayOverflow)
----------------
This looks like it should be a new differential, after the this one.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4195-4198
+  ConstantRange LHSRange = computeConstantRangeIncludingKnownBits(
+      LHS, /*ForSigned=*/true, DL, /*Depth=*/0, AC, CxtI, DT);
+  ConstantRange RHSRange = computeConstantRangeIncludingKnownBits(
+      RHS, /*ForSigned=*/true, DL, /*Depth=*/0, AC, CxtI, DT);
----------------
And one more new differential


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60420/new/

https://reviews.llvm.org/D60420