[PATCH] D12414: [NVPTX] add an NVPTX-specific alias analysis

[Chandler Carruth via llvm-commits]

The funny thing is, we actually *did* have most of this discussion. ;] It
was Hal's second comment on my big AA infrastructure patch which you can
find here: http://reviews.llvm.org/D12080

The summary is that this is obviously an important use case long-term. The
infrastructure was designed to make it possible to handle this, the only
actual lack of flexibility is in the legacy pass manager wrapper, and only
there because at the time there wasn't anything in-tree that needed the
flexibility. The new pass manager has this essentially baked-in.

The reason I didn't just bake this into the legacy pass manager is...
well... the legacy pass manager makes this pretty ridiculously annoying to
implement. There is no point at which you can really "parameterize" the
analysis passes in the legacy pass manager. So the least bad idea I've come
up with is... incredibly ugly:

- Create an ImmutablePass which is a TargetAAWrapperPass or some such
horror, which can be constructed from a callable that can be passed an
AAResults object and will register with it any target-specific AAResult
object. And it exposes this callable (or this functionality) from its API.

- Teach the AAResultsWrapperPass to use this horror if it is available, but
ignore it otherwise.

- Add the TargetAAWrapperPass outside of the PassManagerBuilder similarly
to how we add the TargetTransformInfoWrapperPass.

Anyone else have less horrible ideas about how to thread these things
together?

However, all this said, I'm not sure whether what we really want is to make
target-specific AAs easy, or to come up with an interface for an address
space aware AA to query the target for its interpretation of address spaces.

-Chandler



On Tue, Mar 15, 2016 at 7:23 PM Hal Finkel <hfinkel at anl.gov> wrote:

>
> ------------------------------
>
> *From: *"Daniel Berlin" <dberlin at dberlin.org>
> *To: *reviews+D12414+public+cf5673a649e54709 at reviews.llvm.org
> *Cc: *"Jingyue Wu" <jingyue at google.com>, wengxt at gmail.com, "Chandler
> Carruth" <chandlerc at gmail.com>, "Justin Holewinski" <
> jholewinski at nvidia.com>, "Eli Bendersky" <eliben at google.com>, "Hal
> Finkel" <hfinkel at anl.gov>, "Bjarke Roune" <broune at google.com>, "Mark
> Heffernan" <meheff at google.com>, "llvm-commits" <
> llvm-commits at lists.llvm.org>
> *Sent: *Tuesday, March 15, 2016 1:03:35 PM
> *Subject: *Re: [PATCH] D12414: [NVPTX] add an NVPTX-specific alias
> analysis
>
>
>
> +1.
> If we need this, we should make it work ;)
>
> I'm not sure anyone sat down and said "it would be really bad to let
> targets have their own AA", and make such a decision
> (if they did, they didn't discuss it on llvm-dev)
>
>
> I'm pretty sure I specifically requested that the infrastructure design
> not preclude this from working. We certainly don't want targets working
> around general AA deficiencies with target-specific passes, but for dealing
> with target-specific intrinsics and address spaces (etc.) I think it makes
> a lot of sense.
>
>  -Hal
>
>
>
> On Tue, Mar 15, 2016 at 10:57 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>> hfinkel added a comment.
>>
>> In http://reviews.llvm.org/D12414#375657, @jingyue wrote:
>>
>> > It doesn't work with the new alias analysis infrastructure.
>> >
>> > I didn't manage to find a way to run it with `opt`. The new
>> AliasAnalysis needs to know and populate all potential alias analyses in a
>> target-independent phase. However, NVPTXAliasAnalysis is target-dependent
>> and not even visible to AliasAnalysis (e.g. compiling LLVM with NVPTX
>> disabled).
>>
>>
>> Do you have an alternative plan?
>>
>> It seems like we could add a feature in the new AA to query a
>> target-provided analysis.
>>
>>
>> http://reviews.llvm.org/D12414
>>
>>
>>
>>
>
>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160315/6dd9adb4/attachment.html>