[PATCH] D64869: [SCEV] get more accurate range for AddExpr with NW flag

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5571
+    for (unsigned i = 1, e = Add->getNumOperands(); i != e; ++i) {
+      if (Add->hasNoSignedWrap() &&
+          SignHint == ScalarEvolution::HINT_RANGE_SIGNED)
----------------
I'd very strongly prefer if you structured this as so:
NoWrapKind = compute(Add);
for each operand:
  sum += sum.addWithWrapping(Op, NoWrapKind)

This requires that NoWrapKind above be any of: none, signed, unsigned, and both.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:829
+  assert(
+      (NoWrapKind == OBO::NoSignedWrap || NoWrapKind == OBO::NoUnsignedWrap) &&
+      "NoWrapKind invalid!");
----------------
Why not allow both?


================
Comment at: llvm/lib/IR/ConstantRange.cpp:834
+                                 const ConstantRange &Other) {
+    if (NonFullRange.isFullSet())
+      return getFull();
----------------
I believe this is impossible from the dominating check, please convert to an assert.


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

https://reviews.llvm.org/D64869