[PATCH] D63459: Loop Cache Analysis

[David Greene via Phabricator via llvm-commits]

greened added inline comments.


================
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"));
+
----------------
etiotto wrote:
> greened wrote:
> > What units is this in?  What does "2" mean?
> I clarified the description.
I'm still a bit confused.  Does this mean the `a[i]` and `a[i+1]` will be considered to have temporal reuse with a value of `2`?  Perhaps "temporal reuse" itself needs a definition.  Are we talking about referencing the same exact memory location, the same cache line, the same page, ...?  It seems odd to me that "temporal reuse" would mean anything other than accessing exactly the same memory location.  Everything else I would consider to be "spacial reuse."

At the very least this deserves a longer comment block about what it means and its implications.  Some clients want the definition of "temporal reuse" to be "access the exact same memory location" and this default value seems to mean something very different.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:239
+                 << "No temporal reuse: distance is greater than 2 at depth="
+                 << Level << "\n");
+      return false;
----------------
The debug message should reference `MaxDistance` and not hard-code the value `2`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459