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

[Adam Nemet via llvm-commits]

> On Jun 9, 2016, at 10:28 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> 
> 
> On Thu, Jun 9, 2016 at 1:01 PM, Adam Nemet <anemet at apple.com <mailto: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?

The two functions look identical except that LV also exposes the strides as a set.

To further clarify the plan should be to have LAA collect the symbolic strides and then have LoopVersioning emit the run-time checks and rewrite the versioned loop (instead of LoopVersioningLICM).

LV would have to continue to emit the run-time checks itself because it does not yet use LoopVersioning.

Adam

> 
> David
> 
> 
>  
> On Jun 9, 2016, at 6:16 PM, Xinliang David Li <davidxl at google.com <mailto: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/69ddfe4f/attachment.html>