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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

LG



================
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)
----------------
nikic wrote:
> lebedev.ri wrote:
> > nikic wrote:
> > > lebedev.ri wrote:
> > > > This looks like it should be a new differential, after the this one.
> > > This change is needed because the LHSKnown variable no longer exists, so it can't come after. There's no reason it can't come before though, so done in rL357969.
> > Sure. But i was talking about the entire block of code i highlighted with a comment.
> > I.e. i think the rest of the `computeOverflowForSignedAdd()` change should be in a new diff too.
> > Or if you leave just the `computeConstantRangeIncludingKnownBits()` part, there are no test changes?
> Ooooh, I didn't realize that you can highlight a range of code and assumed your comment is about the change directly above.
> 
> But yes, just the computeConstantRangeIncludingKnownBits() change doesn't result in test changes (it it not strictly speaking NFC, but it's likely not possible to actually trigger a difference without using the function in a signed context).
Okay, thank you.


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

https://reviews.llvm.org/D60420