[PATCH] D109821: [ScalarEvolution] Infer loop max trip count from array accesses

[Liren.Peng via Phabricator via llvm-commits]

Peakulorain added a comment.

In D109821#3001961 <https://reviews.llvm.org/D109821#3001961>, @jdoerfert wrote:

> drive by: this should use `llvm::getObjectSize` rather than looking for allocainst. That will allow some globals and malloc right away.

Thank you for your great idea, I changed to get memory size by using llvm::getObjectSize. Originally ,  we just want to do this for Stack Array, for safety。



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7192
+  // We can't infer from Array in Irregular Loop.
+  // FIXME: It's hard to infer if loop has multi-exiting blocks.
+  // FIXME: It's hard to infer loop bound from array operated in Nested Loop.
----------------
nikic wrote:
> Why are multiple exits a problem? This is calculating an upper bound, so even if we exit before that should still be a valid max trip count. (If multiple exits were a problem, you would also have to guard against abnormal exits.)
Thank you very much for your comments. If loop has multi-exiting blocks, such as loop header and latch are both exiting block, max memory accesses time in loop could NOT represent loop max trip count. If we can get PostDominatorTree here, MemAccessBB post dom loop header can make sure that valid trip count. So we can improve this function after PostDom info could be got.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7264
+  return (InferTripCount == TypeMax) ? 0 : InferTripCount;
+}
+
----------------
jdoerfert wrote:
> Style: Some more levels could be early exists to lower indention by 1-3 in the main part.
Thank you very much, I fixed this style problem.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12474
+
+  if (SE->getConstantMaxTripCountFromArray(L)) {
+    OS << "Loop ";
----------------
nikic wrote:
> Shouldn't we be integrating this in the main SCEV trip count calculation, rather than having it as a separate facility?
Thanks,  we move this into getSmallConstantMaxTripCount.


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

https://reviews.llvm.org/D109821