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

Tue Jun 14 09:13:42 PDT 2016

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/84f9eec3/attachment.html>