[PATCH] D144718: [ScalarEvolution] Enhance Predicate implication from condition

[Tiehu Zhang via Phabricator via llvm-commits]

TiehuZhang added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:11768
+  // FoundLHS > FoundRHS =>
+  // LHS = (FoundLHS + Offset) > RHS = (FoundRHS + Offset)
+  if ((FoundPred == Pred) && (FoundLHS->getType() == LHS->getType()) &&
----------------
mkazantsev wrote:
> This is incorrect in case of overflow. See `isKnownPredicateViaNoOverflow`.
> This is incorrect in case of overflow. See `isKnownPredicateViaNoOverflow`.




================
Comment at: llvm/test/Transforms/HardwareLoops/ARM/simple-do.ll:151
+; CHECK: [[ROUND:%[^ ]+]] = add i32 %n, -1
+; CHECK: [[HALVE:%[^ ]+]] = lshr i32 [[ROUND]], 1
 ; CHECK: [[COUNT:%[^ ]+]] = add nuw i32 [[HALVE]], 1
----------------
mkazantsev wrote:
> This is miscompile, see https://alive2.llvm.org/ce/z/ESPqU_
Thanks for your review! `https://alive2.llvm.org/` is really a useful tool.  Sometimes, it is helpful for the performance to simplify `-1,+,(-1 * %max_row_length)}<nw><%for.cond1.preheader> + ({1,+,%max_row_length}<nw><%for.cond1.preheader> smax {%max_row_length,+,%max_row_length}<nw><%for.cond1.preheader>`  to `-1 + %max_row_length` .  For most cases, `smax` is redundant, and may hurt performance. But if overflow is considered, this check is indispensable. I'll try to restrict the condition of the patch to ensure the correctness. For the scenario, do you have any suggestions?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144718