[PATCH] D20560: [new PM] port LoopAccessAnalysis to new pass manager (part-1)

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 9 13:01:00 PDT 2016


David, FYI, this didn’t make it to Phab due to replying inline.

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.

On Jun 9, 2016, at 6:16 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> 
> 
> On Thu, Jun 9, 2016 at 2:37 AM, Adam Nemet <anemet at apple.com <mailto:anemet at apple.com>> wrote:
> anemet added inline comments.
> 
> ================
> Comment at: llvm/trunk/include/llvm/Analysis/LoopAccessAnalysis.h:704-706
> @@ +703,5 @@
> +private:
> +  /// \brief LoopAccessInfo is created on demand. This map caches
> +  /// the computed results.
> +  DenseMap<Loop *, std::unique_ptr<LoopAccessInfo>> LoopAccessInfoMap;
> +
> ----------------
> chandlerc wrote:
> > 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()`.
> >
> > 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.
> >
> > 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.
> >
> > Does that make sense?
> Just for the record, this had to be a function pass because the loop vectorizer is a function pass with the legacy PM.
> 
> 
> This problem is solvable by providing a function level wrapper for loop access info.
> 
> 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?
> 
> David
> 
> It would be great to turn this into a loop pass with the new PM.
> 
> 
> Repository:
>   rL LLVM
> 
> http://reviews.llvm.org/D20560 <http://reviews.llvm.org/D20560>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160609/98bce006/attachment.html>


More information about the llvm-commits mailing list