[PATCH] D63459: Loop Cache Analysis

[David Greene via Phabricator via llvm-commits]

greened 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.
----------------
Referencing a `cl:opt` defined in the `.cpp`?  It's a little confusing.


================
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;
+
----------------
Why is this part of `IndexedReference`?


================
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"));
+
----------------
What units is this in?  What does "2" mean?


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:189
+      LLVM_DEBUG(dbgs().indent(2) << "No temporal reuse: distance unknown\n");
+      return false;
+    }
----------------
This seems a bit dangerous to me.  Depending on the client, we might want to assume reuse if we don't know the distance.  Could this function return a tribool of (yes/no/unknown)?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459