[PATCH] D23214: [MDA] Treat invariant.start as non-dependence

[Daniel Berlin via llvm-commits]

Hi Anna,
Sounds good.

If you touch invariant.end (and not just invariant.start) :

You should add a testcase to make sure that invariant.start's don't get
value forwarded to other invariant starts of the same result :)

IE

%foo = llvm.invariant.start(%ptr)
...
llvm.invariant.end(%foo)
<scalar code in same block>
%bar = llvm.invariant.start(%ptr)
llvm.invariant.end(%bar)

While readonly, if you can prove the memory state is the same, it may
become smart enough to decide it can forward the values now, and replace
%bar with %foo.

If you leave invariant.end read/write, this should not happen, of course.

But might be worth adding a testcase anyway so it doesn't break in the
future :)


On Fri, Aug 5, 2016 at 3:58 PM, Anna Thomas <anna at azul.com> wrote:

> Thanks Daniel, Sanjoy.
>
> I’ll be making the changes in the alias analysis getModRefInfo instead of
> within the MDA. getModRefInfo is used by MDA, so the implications on the
> various passes (GVN, Memcpyopt and DSE) should be the same,
> it’s just that the code change will be within the same code region where
> other special cases exist :)
>
> Anna
>
>
> On Aug 5, 2016, at 3:36 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
> > That said, in theory:
>> >
>> > %a = add 5, 3
>> > bar = invariant.start(foo, 5)
>> > load foo
>> > invariant.end(bar)
>> >
>> > to
>> > bar = invariant.start(foo, 5)
>> > %a = add 5, 3
>> > load foo
>> > invariant.end(bar)
>> >
>> > is probably also illegal, and has the same issues :)
>>
>> I don't understand why that ^ is a problem.
>>
>>
> You have extended the scope :)
>
> Even if the extension is not meaningful :)
>
>
>> >     With the memdep changes we will allow reordering loads across the
>> >     invarinat_start call:
>> >
>> >        int val = *ptr;
>> >        invariant_start(ptr)
>> >
>> >     <==>
>> >
>> >        int val = *ptr;
>> >        invariant_start(ptr)
>> >
>> >
>> > (you meant for the first one to have the load after invariant start, i
>> > presume)
>>
>> I meant given what we're teaching MDA, the two are equivalent (hence
>> the "bidirectional arrow" :)).
>
>
> I meant that the examples are identical
>  :)
>
>
>>
>>
>> > I'm willing to give it a shot if you want, i'm just in the "we'll
>> > probably break something unexpected" camp :)
>>
>> I now think it is better to add one more special case to AliasAnalysis
>> around invariant_start s, like we do for assumes.  It's a hack, but at
>> least a localized hack.
>
>
> I already added the same hack to memoryssa, so i can't complain :)
>
>
>>
>>
>> -- Sanjoy
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160805/73d42993/attachment.html>