[PATCH] D21271: Fix `InstCombine` to not widen metadata on store-to-load forwarding

[Eli Friedman via llvm-commits]

On Mon, Jun 13, 2016 at 10:04 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Mon, Jun 13, 2016 at 9:57 AM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
>
>> On Mon, Jun 13, 2016 at 7:35 AM, Daniel Berlin <dberlin at dberlin.org>
>> wrote:
>>
>>>
>>>
>>> On Sun, Jun 12, 2016 at 10:12 PM, Eli Friedman <eli.friedman at gmail.com>
>>> wrote:
>>>
>>>> On Sun, Jun 12, 2016 at 9:31 PM, Yichao Yu <yyc1992 at gmail.com> wrote:
>>>>
>>>>> > Alias analysis results should not be confused with value equivalence
>>>>> (though
>>>>> > it is common).  MustAlias or NoAlias implies nothing about the actual
>>>>> > pointer value, only the abstract memory location it represents.
>>>>>
>>>>> I can understand they are not equivalent but shouldn't not being the
>>>>> same pointer value be a necessary condition for NoAlias? In another
>>>>> word, are the following valid?
>>>>>
>>>>> ```
>>>>> define i64 @f1(i64 *%p0, i64 *%p1) {
>>>>>   store i64 0, i64 *%p0, !tbaa !0
>>>>>   %v2 = load i64, i64 *%p1, !tbaa !1
>>>>>   ret %v2
>>>>> }
>>>>>
>>>>> define i64 @f2(i64 *%p1) {
>>>>>   store i64 0, i64 *%p1, !tbaa !0
>>>>>   %v2 = load i64, i64 *%p1, !tbaa !1
>>>>>   ret %v2
>>>>> }
>>>>>
>>>>>
>>>> It depends on what you mean by "valid"... @f1 passed two identical
>>>> pointers is basically equivalent to "ret i64 undef".
>>>>
>>>
>>> So, i can't see anything in the langref that says this.
>>>
>>> For f1, the valid things i see you can do are (and have an argument you
>>> can)
>>>
>>> 1. move the load before the store, return value of load (IE reorder them)
>>> 2. eliminate both the load and store and return 0 (IE forward the store)
>>>
>>> I don't see a valid path to ret undef. I'm eminently curious what i've
>>> missed :)
>>>
>>>
>> LLVM can turn "store zero" into "store <arbitrary-value>; store zero",
>> then reorder the load between the two stores.
>>
>> I have trouble seeing why this would be legal, unless arbitrary value is
> also zero :)
>
> The original of the load semantics were either "it loads the value that
> existed prior to function start", or "it loads the value of the store
> (zero)".
> What you are suggesting is something that changes the semantics to "it
> loads an arbitrary value".
>

Are you saying that the "insert store of arbitrary value" step isn't
legal?  I mean, it looks silly in a tiny example like this, but I'm not
sure why it wouldn't be allowed.  (It's basically equivalent to
transforming "*p = *p * 3 + 1;" to "*p *= 3; *p += 1;".)

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160613/378d8ab3/attachment.html>