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

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 9 09:16:50 PDT 2016


On Thu, Jun 9, 2016 at 2:37 AM, Adam Nemet <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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160609/5960d98b/attachment-0001.html>


More information about the llvm-commits mailing list