[PATCH] D154069: [IRCE] Parse range checks in the form of "LHS - RHS vs Limit"

[Aleksandr Popov via Phabricator via llvm-commits]

aleksandr.popov marked 3 inline comments as done.
aleksandr.popov added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:376
+  // TODO: Can we remove this restriction?
+  if (!SE.willNotOverflow(Instruction::BinaryOps::Sub, ICmpInst::isSigned(Pred),
+                          IV, Offset, cast<Instruction>(VariantLHS))) {
----------------
skatkov wrote:
> More context here in comment. Why with this restriction failed we cannot process.
I removed the check that initial subtraction will not overflow and added explanation why we don't need that check


================
Comment at: llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:395
+
+  // We are going to reassociate expression but the computations can overflow.
+  // To avoid it, let's use extended type for all operands and then add runtime
----------------
skatkov wrote:
> What exactly overflow you can expect?
> Does it makes sense to check, may be we can prove that overflow is not possible and we can avoid scaling?
Good point, done


================
Comment at: llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1688
+    // TODO: Support runtime overflow check for End
+    return std::nullopt;
+  }
----------------
skatkov wrote:
> Can we add some LLVM_DEBUG output here and add a test which tests whether it is printed and check the values of getBegin(), getEnd() and IndVar?
> 
> It would be nice to have tests for this functionality?
Done, thanks!


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

https://reviews.llvm.org/D154069