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

Fri Jun 17 16:44:01 PDT 2016

Wonderful!

thanks,

David

On Fri, Jun 17, 2016 at 4:16 PM, Adam Nemet <anemet at apple.com> wrote:

>
> On Jun 14, 2016, at 9:44 PM, Adam Nemet via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>
> 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.
>
>
> David,
>
> r273064 is the final commit in the series implementing this.
>
> Adam
>
>
>
> Adam
>
>
> thanks,
>
> David
>
> On Tue, Jun 14, 2016 at 9:13 AM, Xinliang David Li <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>
>> wrote:
>>
>>>
>>>
>>> On Thu, Jun 9, 2016 at 2:53 PM, Xinliang David Li via llvm-commits <
>>> 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> wrote:
>>>>
>>>>>
>>>>> 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> 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>
>>>>>> 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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>>
>>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/20160617/5555c04b/attachment.html>