[PATCH] D63459: Loop Cache Analysis

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto marked 2 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:
> > > 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.
> So how will clients with different needs use this analysis?  Some might want the definition in the paper but others might want to restrict it to exact memory locations only.  A `cl::opt` doesn't allow configuration by clients.
I added a optional parameter to allow users to specify the temporal reuse threshold. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459