[PATCH] D142387: [SCCP] Use range info to prove AddInst has NUW flag.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:172
+      if (!Op->getType()->isIntegerTy())
+        return ConstantRange::getFull(1);
+      unsigned Bitwidth = Op->getType()->getScalarSizeInBits();
----------------
nikic wrote:
> Why is this needed?
It's not needed in the latest version, originally this was to exclude vector constants. Removed, thanks!


================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:182
+        return ConstantRange::getFull(Bitwidth);
+      return R.getConstantRange();
+    };
----------------
nikic wrote:
> There is an SCCPInstVisitor::getConstantRange() helper -- probably that should be moved to SCCPSolver and reused here?
>  probably that should be moved to SCCPSolver and reused here?
I just made it a standalone static function


================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:188
+    auto NUWRange = ConstantRange::makeGuaranteedNoWrapRegion(
+        Instruction::Add, RangeA, OverflowingBinaryOperator::NoUnsignedWrap);
+    if (NUWRange.contains(RangeB)) {
----------------
nikic wrote:
> Should be RangeB. Doesn't make a difference for add, but would be wrong when extending to sub.
Flipped Ranges, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142387