<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 9, 2016 at 2:37 AM, Adam Nemet <span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">anemet added inline comments.<br>
<span class=""><br>
================<br>
Comment at: llvm/trunk/include/llvm/Analysis/LoopAccessAnalysis.h:704-706<br>
@@ +703,5 @@<br>
+private:<br>
+  /// \brief LoopAccessInfo is created on demand. This map caches<br>
+  /// the computed results.<br>
+  DenseMap<Loop *, std::unique_ptr<LoopAccessInfo>> LoopAccessInfoMap;<br>
+<br>
----------------<br>
</span><span class="">chandlerc wrote:<br>
> This doesn't seem like the right design. It leads to the `getInfo().getInfo()` awkward pattern that IMO is only marginally less awkward spelled as `getResult().getInfo()`.<br>
><br>
> Instead, I think this pretty clearly wants to be a Loop analysis in the new pass manager. I think that the caching and map should be the new pass manager's caching and map, rather than rolling our own in a function analysis manager.<br>
><br>
> As a consequence, I don't think you need this result type at all. I think the LoopAccessInfo is already the correct result type. I think you'll need the DenseMap and logic around it in the legacy pass manager's function analysis, and you'll want a different query path to use the LoopAnalysisManager directly in the new pass manager.<br>
><br>
> Does that make sense?<br>
</span>Just for the record, this had to be a function pass because the loop vectorizer is a function pass with the legacy PM.<br></blockquote><div><br></div><div><br></div><div>This problem is solvable by providing a function level wrapper for loop access info.</div><div><br></div><div>The bigger problem is that the current getter interface not only takes a  LoopInfo, but also a map of strides which is computed outside.  IIUC, current analysis manager in new PM can not handle that.  Chandler, any thought? Should we make LoopAccess Analysis a function analysis for now and handle it later when Loop analysis is more mature?</div><div><br></div><div>David</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
It would be great to turn this into a loop pass with the new PM.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D20560" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20560</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>