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

[Xinliang David Li via llvm-commits]

On Thu, Jun 9, 2016 at 1:01 PM, Adam Nemet <anemet at apple.com> wrote:

> 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.
>


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?

David




> 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> 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/42a1a280/attachment.html>