[PATCH] D63459: Loop Cache Analysis

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto marked 10 inline comments as done.
etiotto added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopCacheAnalysis.h:76
+  /// Return true if the current object and the indexed reference \p Other
+  /// have distance smaller than 'TemporalReuseThreashold' in the dimension
+  /// associated with the given loop \p L.
----------------
greened wrote:
> Referencing a `cl:opt` defined in the `.cpp`?  It's a little confusing.
I'll pass it as an argument.


================
Comment at: llvm/include/llvm/Analysis/LoopCacheAnalysis.h:107
+  /// for the trip count or nullptr if it cannot be computed.
+  const SCEV *computeTripCount(const Loop &L) const;
+
----------------
greened wrote:
> Why is this part of `IndexedReference`?
I changed into a static function in the .cpp file instead.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:44
+    "temporal-reuse-threshold", cl::init(2), cl::Hidden,
+    cl::desc("Use this to specify the temporal reuse distance threshold"));
+
----------------
greened wrote:
> What units is this in?  What does "2" mean?
I clarified the description.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:267
+
+  auto isOneDimensionalArray = [&](const SCEV *AccessFn, const Loop *L) {
+    const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(AccessFn);
----------------
fhahn wrote:
> It looks like this function does not capture much and might be better as a separate member function?
OK I'll make it a private member function.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459