[PATCH] D59386: [ValueTracking] ConstantRange based overflow detection for unsigned add/sub

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Hmm, this looks reasonable to me, @spatel ?
Are there any concerns of performance impact of using `ConstantRange()` here ?
Though i do think it does pull it's weight.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4212
 
 OverflowResult llvm::computeOverflowForUnsignedSub(const Value *LHS,
                                                    const Value *RHS,
----------------
When committing, it might be a good idea to commit the `computeOverflowForUnsignedSub()` change as a follow-up, as usual..


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5708
 
 ConstantRange llvm::computeConstantRange(const Value *V, bool UseInstrInfo) {
+  Type *Ty = V->getType();
----------------
nikic wrote:
> lebedev.ri wrote:
> > This is used in `simplifyICmpWithConstant()`. thus i think this can be split off too,
> > although i'm not sure if that will show any test changes as-is?
> For this code to apply in `simplifyICmpWithConstant()` it would have to be an icmp with both operands constant, which will already be folded earlier. (This code is split off from simplifyICmpWithConstant, which is why the constant handling was missing: It's not needed for that usage.)
Oh right, thanks for explanation.


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

https://reviews.llvm.org/D59386