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

Tue Jun 14 12:44:12 PDT 2016

> On Jun 14, 2016, at 7:37 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> This work now depends on preparation work to hide strides from the LoopAccessInfo creator interface.
> 
> Adam, what is your plan on this one?

I haven’t started it yet, was busy with a PR but should get to it in the next couple of days.

Adam

> 
> thanks,
> 
> David
> 
> On Tue, Jun 14, 2016 at 9:13 AM, Xinliang David Li <davidxl at google.com <mailto:davidxl at google.com>> wrote:
> Neat. I will give it a try.
> 
> David
> 
> On Tue, Jun 14, 2016 at 2:09 AM, Sean Silva <chisophugis at gmail.com <mailto:chisophugis at gmail.com>> wrote:
> 
> 
> On Thu, Jun 9, 2016 at 2:53 PM, Xinliang David Li via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Ok.
> 
> 
> There might be more problems. Suppose we make LAI managed by the LoopAnalysisManager in PM. The analysis has the following interface to build LAI on the the fly and cached by LAM:
> 
> class LoopAccessInfoAnalysis {
> public:
>    typedef LoopAccessInfo ResultType;
>    ResultType run(LoopInfo *LI) {  return LoopAccessInfo ( ...); }
> };
> 
> Here is the question for Chandler. As mentioned by Adam, the LV pass is a function pass. With new PM, its run interface is passed with AnalysisManager<Function> reference.   With a function analysis AM, is it possible to get access to the analyis results managed by LAM?
> 
> LoopAccessInfo & LAI = AM.getResult<LoopAccessInfoAnalysis>() ?
> 
> If not, is there a way to get reference to the LAM instance associated with the Loop from function AM?
> 
> If I understand what you are asking correctly (and my understanding of new PM is correct), what you need is the "LoopAnalysisManagerFunctionProxy". E.g. in this code from LoopPassManager.h
> 
>   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
>     // Setup the loop analysis manager from its proxy.
>     LoopAnalysisManager &LAM =
>         AM.getResult<LoopAnalysisManagerFunctionProxy>(F).getManager();
>  
> 
> -- Sean Silva
> 
> 
> thanks,
> 
> David
> 
> 
> 
> On Thu, Jun 9, 2016 at 2:01 PM, Adam Nemet <anemet at apple.com <mailto:anemet at apple.com>> wrote:
> 
>> On Jun 9, 2016, at 10:28 PM, Xinliang David Li <davidxl at google.com <mailto: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>
>> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160614/4ff31903/attachment.html>