[PATCH] D63459: Loop Cache Analysis

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto marked 5 inline comments as done.
etiotto 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"));
+
----------------
greened wrote:
> 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.
That's correct,  a[i] and a[i+1] are consider to have temporal reuse when the threshold is 2. A threshold of 1 would cause only references to the same memory location have temporal reuse. The analysis attempts to implement the algorithm in the paper mentioned in the summary, and 
I agree that this is a bit confusing. I will add a comment to attempt to clarify better.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:189
+      LLVM_DEBUG(dbgs().indent(2) << "No temporal reuse: distance unknown\n");
+      return false;
+    }
----------------
greened wrote:
> 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)?
If the dependence distance is unknown at compile time the references are conservatively considered to have no spacial reuse, and consequently the analysis will overestimate the number of cache lines used by the loop (when it is in the innermost position in the nest). 

For now I can return an Optional<bool> (didn't find a tribool data type readily available in LLVM). This will make it easier to place references with 'unknow' distance in the same reference group if we find a motivating test case that needs it.    


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459