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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D59386#1432364 <https://reviews.llvm.org/D59386#1432364>, @lebedev.ri wrote:

> 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.


At this point, any change to heavily used ValueTracking calls may be cause for compile-time concern. 
I'd hope that ConstantRange can be optimized to just 2 ints in the usual case, so it should be fast (as we assume for KnownBits too I think). 
To be safe, we could do a test-suite or stage2 compile and see if that shows regressions, but the optimizations in the tests do look nice. :)



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5717
+  unsigned BitWidth = Ty->getScalarSizeInBits();
+  if (const ConstantDataVector *CDV = dyn_cast<ConstantDataVector>(V)) {
+    // For constant vectors we compute the range between the minimal and
----------------
By limiting to ConstantDataVector, are we excluding weird (non-power-of-2 bitwidths or number of elements) types? Is that intentional?


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

https://reviews.llvm.org/D59386