Filipe Cabecinhas via llvm-dev
2017-Jul-20 01:07 UTC
[llvm-dev] [RFC] dereferenceable metadata
Indeed. But the problem here is that Dinar is trying to keep information
after a load/store is removed by instcombine
For example:
v4sf v = {p[0], p[1], p[2], p[3]};
v4sf v2 = shuffle(v, 0, 0, 2, 2);
Some pass comes in and removes the p[3] and p[1].
Now you have smaller code, but lost the ability to use a vector load for
all those values + shuffle. The code got scalarized because we lost the
information that p[3] is valid.
The attribute on a value/load/something might not be ideal (especially
since we'd be changing other loads (ones we didn't remove)). But I think
Dinar wanted to start a conversation about how we can keep this information
around even after we delete some loads.
Thank you,
Filipe
On Wed, 19 Jul 2017 at 12:53, Nuno Lopes via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> When a pointer is passed to load/store, it's already implicit that it
is
> dereferenceable for the size of the access (otherwise it would be undefined
> behavior).
> Isn't that information sufficient for your use case? (sorry,
didn't read
> the whole thread carefully).
>
> Nuno
>
> -----Original Message-----
> From: Dinar Temirbulatov via llvm-dev
> Sent: Tuesday, July 18, 2017 7:36 PM
> Subject: [llvm-dev] [RFC] dereferenceable metadata
>
> Hi,
> While working on PR21780, I used "dereferenceable_or_null"
metadata
> and I realized now that it is not correct for my solution to use this
> metadata type since it might point to an address that it is not
> dereferenceable but null. I think that we need another new metadata
> type, something like "dereferenceable" with that we could
annotate
> any load (not just pointer type like with dereferenceable_or_null).
> For example, we could annotate this load : "%ld0 = load double,
> double* %ptr, align 8" with dereferenceable<2> and that means
that for
> %ptr address memory with length 16-bytes is known to be
> accessible(dereferenceable). Originally in PR21780, InstCombine pass
> removed some loads as dead(in a Scalar IR form) and later that
> prevented us to vectorize the operation, but before removing such
> loads we could annotate the remaining loads in the series with
> "dereferenceable" and later restore IR in a way that is suitable
for
> vectorization(see https://reviews.llvm.org/D35139 ). Also, please note
> that’s the information that is lost when InstCombine kills the last
> load in the series and there is no way to restore this information
> later in following passes.
> Thanks, Dinar.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170720/562874cc/attachment.html>
Dinar Temirbulatov via llvm-dev
2017-Jul-20 12:58 UTC
[llvm-dev] [RFC] dereferenceable metadata
>it's already implicit that it is dereferenceable for the size of the access (otherwise it would be undefined behavior).The values we are loading after restoring those loads are not matter for us and it is just that chain of loads for vectorization that important for us. And those load would never get rematerialization if vectorization is not profitable for the shuffle operation. Once we remove the load in instcombine and there is no way to know that memory is dereferenceable from the previous load in the chain of loads. Thanks, Dinar. On Thu, Jul 20, 2017 at 4:07 AM, Filipe Cabecinhas <me at filcab.net> wrote:> Indeed. But the problem here is that Dinar is trying to keep information > after a load/store is removed by instcombine > > For example: > > v4sf v = {p[0], p[1], p[2], p[3]}; > v4sf v2 = shuffle(v, 0, 0, 2, 2); > > Some pass comes in and removes the p[3] and p[1]. > > Now you have smaller code, but lost the ability to use a vector load for all > those values + shuffle. The code got scalarized because we lost the > information that p[3] is valid. > > The attribute on a value/load/something might not be ideal (especially since > we'd be changing other loads (ones we didn't remove)). But I think Dinar > wanted to start a conversation about how we can keep this information around > even after we delete some loads. > > Thank you, > > Filipe > > On Wed, 19 Jul 2017 at 12:53, Nuno Lopes via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >> >> When a pointer is passed to load/store, it's already implicit that it is >> dereferenceable for the size of the access (otherwise it would be >> undefined >> behavior). >> Isn't that information sufficient for your use case? (sorry, didn't read >> the whole thread carefully). >> >> Nuno >> >> -----Original Message----- >> From: Dinar Temirbulatov via llvm-dev >> Sent: Tuesday, July 18, 2017 7:36 PM >> Subject: [llvm-dev] [RFC] dereferenceable metadata >> >> Hi, >> While working on PR21780, I used "dereferenceable_or_null" metadata >> and I realized now that it is not correct for my solution to use this >> metadata type since it might point to an address that it is not >> dereferenceable but null. I think that we need another new metadata >> type, something like "dereferenceable" with that we could annotate >> any load (not just pointer type like with dereferenceable_or_null). >> For example, we could annotate this load : "%ld0 = load double, >> double* %ptr, align 8" with dereferenceable<2> and that means that for >> %ptr address memory with length 16-bytes is known to be >> accessible(dereferenceable). Originally in PR21780, InstCombine pass >> removed some loads as dead(in a Scalar IR form) and later that >> prevented us to vectorize the operation, but before removing such >> loads we could annotate the remaining loads in the series with >> "dereferenceable" and later restore IR in a way that is suitable for >> vectorization(see https://reviews.llvm.org/D35139 ). Also, please note >> that’s the information that is lost when InstCombine kills the last >> load in the series and there is no way to restore this information >> later in following passes. >> Thanks, Dinar. >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On 07/20/2017 07:58 AM, Dinar Temirbulatov via llvm-dev wrote:>> it's already implicit that it is dereferenceable for the size of the access (otherwise it would be undefined behavior). > The values we are loading after restoring those loads are not matter > for us and it is just that chain of loads for vectorization that > important for us. And those load would never get rematerialization if > vectorization is not profitable for the shuffle operation. Once we > remove the load in instcombine and there is no way to know that memory > is dereferenceable from the previous load in the chain of loads.I think you're looking for some kind of "dereferenceable assumption" or "phantom load" that we could put in the IR when we remove an actual load/store to preserve dereferencability information. I think this would need to be an intrinsic, not metadata, because you're interested in putting it at some point in the control flow (e.g. above a loop). There are other use cases for this kind of thing as well, but there are costs to keeping this kind of information, and we'd need to consider this carefully. -Hal> Thanks, Dinar. > > On Thu, Jul 20, 2017 at 4:07 AM, Filipe Cabecinhas <me at filcab.net> wrote: >> Indeed. But the problem here is that Dinar is trying to keep information >> after a load/store is removed by instcombine >> >> For example: >> >> v4sf v = {p[0], p[1], p[2], p[3]}; >> v4sf v2 = shuffle(v, 0, 0, 2, 2); >> >> Some pass comes in and removes the p[3] and p[1]. >> >> Now you have smaller code, but lost the ability to use a vector load for all >> those values + shuffle. The code got scalarized because we lost the >> information that p[3] is valid. >> >> The attribute on a value/load/something might not be ideal (especially since >> we'd be changing other loads (ones we didn't remove)). But I think Dinar >> wanted to start a conversation about how we can keep this information around >> even after we delete some loads. >> >> Thank you, >> >> Filipe >> >> On Wed, 19 Jul 2017 at 12:53, Nuno Lopes via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >>> When a pointer is passed to load/store, it's already implicit that it is >>> dereferenceable for the size of the access (otherwise it would be >>> undefined >>> behavior). >>> Isn't that information sufficient for your use case? (sorry, didn't read >>> the whole thread carefully). >>> >>> Nuno >>> >>> -----Original Message----- >>> From: Dinar Temirbulatov via llvm-dev >>> Sent: Tuesday, July 18, 2017 7:36 PM >>> Subject: [llvm-dev] [RFC] dereferenceable metadata >>> >>> Hi, >>> While working on PR21780, I used "dereferenceable_or_null" metadata >>> and I realized now that it is not correct for my solution to use this >>> metadata type since it might point to an address that it is not >>> dereferenceable but null. I think that we need another new metadata >>> type, something like "dereferenceable" with that we could annotate >>> any load (not just pointer type like with dereferenceable_or_null). >>> For example, we could annotate this load : "%ld0 = load double, >>> double* %ptr, align 8" with dereferenceable<2> and that means that for >>> %ptr address memory with length 16-bytes is known to be >>> accessible(dereferenceable). Originally in PR21780, InstCombine pass >>> removed some loads as dead(in a Scalar IR form) and later that >>> prevented us to vectorize the operation, but before removing such >>> loads we could annotate the remaining loads in the series with >>> "dereferenceable" and later restore IR in a way that is suitable for >>> vectorization(see https://reviews.llvm.org/D35139 ). Also, please note >>> that’s the information that is lost when InstCombine kills the last >>> load in the series and there is no way to restore this information >>> later in following passes. >>> Thanks, Dinar. >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory