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

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 13 11:32:56 PDT 2016


On Mon, Jun 13, 2016 at 11:07 AM, Eli Friedman <eli.friedman at gmail.com>
wrote:

> 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 believe so, yes, *if* the effect is to change the semantics of the load.



> 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;".)
>

I would suggest, in this case, it is not legal because it changes the
semantics of the load in an observable way.


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


More information about the llvm-commits mailing list