[PATCH] D44515: [IRCE] Change min value safety check

[Max Kazantsev via Phabricator via llvm-commits]

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


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:690
+static bool SumCannotBeMaxInLoop(const SCEV *BoundSCEV,
+                                 const SCEV *StepMinusOne, Loop *L,
+                                 ScalarEvolution &SE, bool Signed) {
----------------
You are making this function for SCEVs regardless of their semantics, please rename `BoundSCEV` and `StepMinusOne` into something more general.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:697
+  const SCEV *Limit = SE.getMinusSCEV(Max, StepMinusOne);
+  auto Predicate = Signed ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
+  return SE.isLoopBackedgeGuardedByCond(L, Predicate, BoundSCEV, Limit);
----------------
This is incorrect. You need to prove that `BoundSCEV < Limit`, so you should use SLT/ULT predicates, not SGT/ULT.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:698
+  auto Predicate = Signed ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
+  return SE.isLoopBackedgeGuardedByCond(L, Predicate, BoundSCEV, Limit);
 }
----------------
It is also potentially incorrect. You need to check this condition not on loop backedge but on loop entry. Otherwise it can be in theory wrong on 1st iteration. It is checked with `isLoopEntryGuardedByCond`.


https://reviews.llvm.org/D44515