[llvm-dev] [RFC] BasicAA considers address spaces?

[Daniel Berlin via llvm-dev]

On Fri, Aug 7, 2015 at 11:35 AM, Jingyue Wu via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> + the new llvm-dev
>
> On Fri, Aug 7, 2015 at 11:30 AM, Jingyue Wu <jingyue at google.com> wrote:
>>
>> Hi folks,
>>
>> Unsurprisingly, leveraging the fact that certain address spaces don't
>> alias can significantly improve alias analysis precision and enhance
>> (observably 2x performance gain) load/store optimizations such as LICM and
>> DSE.

Not shocking

>>
>> This sounds to me an overdue feature. I saw several discussion threads on
>> that direction, but none of them really happened.
>>
>> (1)
>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20111010/129615.html.
>> Justin Holewinski proposed to add an address-space alias analysis that
>> treats pointers in different address spaces not aliasing. This patch got
>> shot down because, in some targets, address spaces may alias. For example,
>> in CUDA+NVPTX, addrspace(0) aliases everyone.

ITYM other addrspace may alias addrspace(0).

Sorry to be pedantic, but i think it's important to make sure we are
all on the same page that backends can't change the aliasing behavior
of the default address space, only the aliasing behavior of the other
address spaces.

>>
>> So, can we make BasicAA to consider address spaces? Specifically, I am
>> proposing:
>> a) adding a new interface like TTI::addressSpacesAlias(unsigned,
>> unsigned), and
>> b) adding a little piece of logic in BasicAA that reports "no alias" if
>> address spaces don't alias.


>>
>> This approach addresses the issue brought up in (2) because TTI can see
>> the entire codegen. It also resolves the issue that shut down (1) because it
>> allows address spaces to alias in a target-defined way. Actually, John
>> Criswell did mention in that thread the idea of embedding alias info in
>> TargetData. Now that we have TTI, it seems a better place to hold
>> target-specific alias info than DataLayout.
>>
>> Any comments?

First, I agree this is a really useful feature to have.
If it really is target specific (and it seems to be), ISTM like TTI is
the right place to add a hook.

However, i'd still rather see it as a pass (as it was in 2011) and
something exposing the AA interface, than further push things into
basicaa.

It's not at all clear to me why basicaa should have this knowledge.

But this is a mild preference, if others think this is a bad idea, ...