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

Bin Cheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 26 23:42:58 PDT 2021


bin.cheng added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolution.h:794
+  /// Returns 0 if the trip count is unknown.
+  unsigned getConstantMaxTripCountFromArray(const Loop *L);
+
----------------
mkazantsev wrote:
> I think it's a bad API for this. The methods above has an excuse of using word "small", assuming that they don't care about "big" trip counts (for whatever reason that is).
> 
> Your method sounds like it just computes an upper bound of trip count from array, whatever this value is. What if this value does not fit in unsigned? Any reasons we can't return an `Optional<APInt>`?
Right, it's possible the computed trip count can't be expressed by unsigned type.  Either we should return Optional<APInt> or rename it as getSmallConstantMaxTripCountFromArray and only returns non-zero if trip count can be expressed by unsigned type.
I can see two advantages with the latter choice:
1\ be consistent with existing interfaces.
2\ the interface is mostly designed to be used to avoid(or enable) various loop transformations in compilation time unknown small trip count. (like vectorization, loop idiom, more accurate BPI, etc..)

Do you have any preference on this?

Thanks


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7315
+
+      // Now we can infer a max excute time by MemLength/StepLength.
+      const SCEV *MemSize =
----------------
mkazantsev wrote:
> Is max execute time a same thing as trip count, or it's something else? If they are the same, please use "trip count" as something that is more understandable.
IIUC, it depends on the location of array reference to exiting block.  If it doesn't dominate exiting block, then this "max execute time" could be "tripcount - 1".  So the meaning for "max execute time" is the max times that the array reference can be executed in terms of SCEV derived from loop iv, without triggering UB.


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

https://reviews.llvm.org/D109821



More information about the llvm-commits mailing list