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

[Sean Silva via llvm-commits]

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/466ed161/attachment.html>