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

Tue Jun 14 10:37:04 PDT 2016

This work now depends on preparation work to hide strides from the
LoopAccessInfo creator interface.

Adam, what is your plan on this one?

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
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160614/9878ca86/attachment.html>