<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 9, 2016 at 1:01 PM, 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"><div style="word-wrap:break-word">David, FYI, this didn’t make it to Phab due to replying inline.<div><br></div><div>Anyhow, we could move the map of stride collection to LAA.  I didn’t do that initially as a shortcut but I can look into doing that, just LMK.</div></div></blockquote><div><br></div><div><br></div><div>I have not looked in detail, but looks like client code have different ways to collect strides: see LoopVectorizor and LoopVersioningLICM.    How do you plan to handle this?</div><div><br></div><div>David</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div class="h5"><div></div><div>On Jun 9, 2016, at 6:16 PM, Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>> wrote:<div><div><blockquote type="cite"><br><div><br><br style="font-family:Helvetica;font-size:10px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:10px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Thu, Jun 9, 2016 at 2:37 AM, Adam Nemet<span> </span><span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">anemet added inline comments.<br><span><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>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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>It would be great to turn this into a loop pass with the new PM.<br><div><div><br><br>Repository:<br> <span> </span>rL LLVM<br><br><a href="http://reviews.llvm.org/D20560" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20560</a></div></div></blockquote></div></div></blockquote></div><br></div></div></div></div></blockquote></div><br></div></div>