Philip Reames via llvm-dev
2016-Feb-24 17:28 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
This probably needs broader discussion. We have an existing naming mechanism for polymorphic intrinsics; Elena is proposing a new one to avoid making the names for various load/store intrinsics particularly ugly. My personal take: 1) I like the cleaner naming scheme. 2) I'm not sure the additional complexity is worth it. (Not specific to the particular implementation proposed here.) 3) I have no strong preference other than that the @llvm.masked_load (and friends) intrinsics support alternate address spaces in some form in the near future. What do others think? Philip -------- Forwarded Message -------- Subject: [PATCH] D17497: Support arbitrary address space for intrinsics Date: Mon, 22 Feb 2016 08:39:38 +0000 From: Elena Demikhovsky <elena.demikhovsky at intel.com> Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org To: elena.demikhovsky at intel.com, apilipenko at azulsystems.com, listmail at philipreames.com, ayal.zaks at intel.com, Matthew.Arsenault at amd.com, pjcoup at gmail.com CC: llvm-commits at lists.llvm.org delena created this revision. delena added reviewers: apilipenko, reames, Ayal, arsenm, pjcoup. delena added a subscriber: llvm-commits. delena set the repository for this revision to rL LLVM. This is an alternative proposal for supporting address space in intrinsics. It's applicable for any intrinsic, not only masked-load-store. Related to http://reviews.llvm.org/D17270 I this proposal I add non-zero address space suffix to intrinsic name. The original name looks like @llvm.xxx.xxx The same name with address space 1 : @llvm.xxx.xxx.a_1 (I did not update documentation. I'll do this if the proposed change looks reasonable for reviewers ). Repository: rL LLVM http://reviews.llvm.org/D17497 Files: ../include/llvm/IR/IRBuilder.h ../include/llvm/IR/Intrinsics.h ../lib/IR/Function.cpp ../lib/IR/IRBuilder.cpp ../lib/IR/Verifier.cpp ../test/Transforms/LoopVectorize/X86/gather_scatter.ll -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160224/12aaed3a/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: D17497.48653.patch Type: text/x-patch Size: 13401 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160224/12aaed3a/attachment-0001.bin>
David Chisnall via llvm-dev
2016-Feb-24 17:32 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
My gut feeling is that it’s not worth it. When we move from typed to untyped pointers, we’re going to change the mangling from something like p200i8 to just p200, which is already quite a bit cleaner, and actually looks cleaner to me than the version proposed in this patch. David> On 24 Feb 2016, at 17:28, Philip Reames via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > This probably needs broader discussion. We have an existing naming mechanism for polymorphic intrinsics; Elena is proposing a new one to avoid making the names for various load/store intrinsics particularly ugly. > > My personal take: > 1) I like the cleaner naming scheme. > 2) I'm not sure the additional complexity is worth it. (Not specific to the particular implementation proposed here.) > 3) I have no strong preference other than that the @llvm.masked_load (and friends) intrinsics support alternate address spaces in some form in the near future. > > What do others think? > > Philip > > > -------- Forwarded Message -------- > Subject: [PATCH] D17497: Support arbitrary address space for intrinsics > Date: Mon, 22 Feb 2016 08:39:38 +0000 > From: Elena Demikhovsky <elena.demikhovsky at intel.com> > Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org > To: elena.demikhovsky at intel.com, apilipenko at azulsystems.com, listmail at philipreames.com, ayal.zaks at intel.com, Matthew.Arsenault at amd.com, pjcoup at gmail.com > CC: llvm-commits at lists.llvm.org > > delena created this revision. > delena added reviewers: apilipenko, reames, Ayal, arsenm, pjcoup. > delena added a subscriber: llvm-commits. > delena set the repository for this revision to rL LLVM. > > This is an alternative proposal for supporting address space in intrinsics. It's applicable for any intrinsic, not only masked-load-store. > Related to > http://reviews.llvm.org/D17270 > > > I this proposal I add non-zero address space suffix to intrinsic name. The original name looks like > @llvm.xxx.xxx > The same name with address space 1 : > @llvm.xxx.xxx.a_1 > > (I did not update documentation. I'll do this if the proposed change looks reasonable for reviewers ). > > Repository: > rL LLVM > > > http://reviews.llvm.org/D17497 > > > Files: > ../include/llvm/IR/IRBuilder.h > ../include/llvm/IR/Intrinsics.h > ../lib/IR/Function.cpp > ../lib/IR/IRBuilder.cpp > ../lib/IR/Verifier.cpp > ../test/Transforms/LoopVectorize/X86/gather_scatter.ll > > > > > > <D17497.48653.patch>_______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Matt Arsenault via llvm-dev
2016-Feb-24 18:40 UTC
[llvm-dev] [PATCH] D17497: Support arbitrary address space for intrinsics
> On Feb 24, 2016, at 09:32, David Chisnall via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > My gut feeling is that it’s not worth it. When we move from typed to untyped pointers, we’re going to change the mangling from something like p200i8 to just p200, which is already quite a bit cleaner, and actually looks cleaner to me than the version proposed in this patch. > > David+1 The prettiness of intrinsic mangling is irrelevant, especially since this will all change with typeless pointers anyway. -Matt
Demikhovsky, Elena via llvm-dev
2016-Feb-25 08:19 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
One more possible solution that I considered is allowing forward reference in types. def int_xxx_yyy : Intrinsic<[LLVMVectorElt<1>], [llvm_anyvector_ty], [IntrNoMem]>; For example, you want to add a vector operation that returns scalar: In the current implementation it will look like i32 @llvm.vec.i32.v8i32(<8 x i32>%x) If you allow forward reference, it will look like i32 @llvm.vec.v8i32(<8 x i32>%x). The same about masked intrinsics. @llvm.masked.load.p0v8i32.v8i32 - you have a long name with duplicated info just due to the limitation in the current infrastructure. - Elena -----Original Message----- From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of David Chisnall via llvm-dev Sent: Wednesday, February 24, 2016 19:32 To: Philip Reames <listmail at philipreames.com> Cc: llvm-dev <llvm-dev at lists.llvm.org> Subject: Re: [llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics My gut feeling is that it’s not worth it. When we move from typed to untyped pointers, we’re going to change the mangling from something like p200i8 to just p200, which is already quite a bit cleaner, and actually looks cleaner to me than the version proposed in this patch. David> On 24 Feb 2016, at 17:28, Philip Reames via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > This probably needs broader discussion. We have an existing naming mechanism for polymorphic intrinsics; Elena is proposing a new one to avoid making the names for various load/store intrinsics particularly ugly. > > My personal take: > 1) I like the cleaner naming scheme. > 2) I'm not sure the additional complexity is worth it. (Not specific > to the particular implementation proposed here.) > 3) I have no strong preference other than that the @llvm.masked_load (and friends) intrinsics support alternate address spaces in some form in the near future. > > What do others think? > > Philip > > > -------- Forwarded Message -------- > Subject: [PATCH] D17497: Support arbitrary address space for intrinsics > Date: Mon, 22 Feb 2016 08:39:38 +0000 > From: Elena Demikhovsky <elena.demikhovsky at intel.com> > Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org > To: elena.demikhovsky at intel.com, apilipenko at azulsystems.com, listmail at philipreames.com, ayal.zaks at intel.com, Matthew.Arsenault at amd.com, pjcoup at gmail.com > CC: llvm-commits at lists.llvm.org > > delena created this revision. > delena added reviewers: apilipenko, reames, Ayal, arsenm, pjcoup. > delena added a subscriber: llvm-commits. > delena set the repository for this revision to rL LLVM. > > This is an alternative proposal for supporting address space in intrinsics. It's applicable for any intrinsic, not only masked-load-store. > Related to > http://reviews.llvm.org/D17270 > > > I this proposal I add non-zero address space suffix to intrinsic name. > The original name looks like @llvm.xxx.xxx The same name with address > space 1 : > @llvm.xxx.xxx.a_1 > > (I did not update documentation. I'll do this if the proposed change looks reasonable for reviewers ). > > Repository: > rL LLVM > > > http://reviews.llvm.org/D17497 > > > Files: > ../include/llvm/IR/IRBuilder.h > ../include/llvm/IR/Intrinsics.h > ../lib/IR/Function.cpp > ../lib/IR/IRBuilder.cpp > ../lib/IR/Verifier.cpp > ../test/Transforms/LoopVectorize/X86/gather_scatter.ll > > > > > > <D17497.48653.patch>_______________________________________________ > 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 --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Philip Reames via llvm-dev
2016-Mar-02 19:21 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
Elena, I'd like to propose that we move forward withArtur's original patch <http://reviews.llvm.org/D17270> and separate the discussion of how we might change our intrinsic naming scheme. Artur's patch is addressing a correctness problem; that has to overrule stylistic concerns. We are seeing failures in our nightly tests due to this issue on an ongoing basis, and I'd really like to get the correctness issue resolved in the immediate future. I am more than happy to continue the discussion about better naming schemes - in particular, I like you're idea of potentially allowing forward references - but I strongly feel we need to decouple it from a bug fix for a correctness issue. Philip On 02/24/2016 09:28 AM, Philip Reames via llvm-dev wrote:> This probably needs broader discussion. We have an existing naming > mechanism for polymorphic intrinsics; Elena is proposing a new one to > avoid making the names for various load/store intrinsics particularly > ugly. > > My personal take: > 1) I like the cleaner naming scheme. > 2) I'm not sure the additional complexity is worth it. (Not specific > to the particular implementation proposed here.) > 3) I have no strong preference other than that the @llvm.masked_load > (and friends) intrinsics support alternate address spaces in some form > in the near future. > > What do others think? > > Philip > > > -------- Forwarded Message -------- > Subject: [PATCH] D17497: Support arbitrary address space for intrinsics > Date: Mon, 22 Feb 2016 08:39:38 +0000 > From: Elena Demikhovsky <elena.demikhovsky at intel.com> > Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org > To: elena.demikhovsky at intel.com, apilipenko at azulsystems.com, > listmail at philipreames.com, ayal.zaks at intel.com, > Matthew.Arsenault at amd.com, pjcoup at gmail.com > CC: llvm-commits at lists.llvm.org > > > > delena created this revision. > delena added reviewers: apilipenko, reames, Ayal, arsenm, pjcoup. > delena added a subscriber: llvm-commits. > delena set the repository for this revision to rL LLVM. > > This is an alternative proposal for supporting address space in intrinsics. It's applicable for any intrinsic, not only masked-load-store. > Related tohttp://reviews.llvm.org/D17270 > > I this proposal I add non-zero address space suffix to intrinsic name. The original name looks like > @llvm.xxx.xxx > The same name with address space 1 : > @llvm.xxx.xxx.a_1 > > (I did not update documentation. I'll do this if the proposed change looks reasonable for reviewers ). > > Repository: > rL LLVM > > http://reviews.llvm.org/D17497 > > Files: > ../include/llvm/IR/IRBuilder.h > ../include/llvm/IR/Intrinsics.h > ../lib/IR/Function.cpp > ../lib/IR/IRBuilder.cpp > ../lib/IR/Verifier.cpp > ../test/Transforms/LoopVectorize/X86/gather_scatter.ll > > > > > > > _______________________________________________ > 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/20160302/0b29ef98/attachment-0001.html>
Philip Reames via llvm-dev
2016-Mar-04 19:35 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
Per my previous email, I have just signed off on Artur's original patch. Philip On 03/02/2016 11:21 AM, Philip Reames via llvm-dev wrote:> Elena, > > I'd like to propose that we move forward withArtur's original patch > <http://reviews.llvm.org/D17270> and separate the discussion of how we > might change our intrinsic naming scheme. Artur's patch is addressing > a correctness problem; that has to overrule stylistic concerns. We > are seeing failures in our nightly tests due to this issue on an > ongoing basis, and I'd really like to get the correctness issue > resolved in the immediate future. > > I am more than happy to continue the discussion about better naming > schemes - in particular, I like you're idea of potentially allowing > forward references - but I strongly feel we need to decouple it from a > bug fix for a correctness issue. > > Philip > > On 02/24/2016 09:28 AM, Philip Reames via llvm-dev wrote: >> This probably needs broader discussion. We have an existing naming >> mechanism for polymorphic intrinsics; Elena is proposing a new one to >> avoid making the names for various load/store intrinsics particularly >> ugly. >> >> My personal take: >> 1) I like the cleaner naming scheme. >> 2) I'm not sure the additional complexity is worth it. (Not specific >> to the particular implementation proposed here.) >> 3) I have no strong preference other than that the @llvm.masked_load >> (and friends) intrinsics support alternate address spaces in some >> form in the near future. >> >> What do others think? >> >> Philip >> >> >> -------- Forwarded Message -------- >> Subject: [PATCH] D17497: Support arbitrary address space for intrinsics >> Date: Mon, 22 Feb 2016 08:39:38 +0000 >> From: Elena Demikhovsky <elena.demikhovsky at intel.com> >> Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org >> To: elena.demikhovsky at intel.com, apilipenko at azulsystems.com, >> listmail at philipreames.com, ayal.zaks at intel.com, >> Matthew.Arsenault at amd.com, pjcoup at gmail.com >> CC: llvm-commits at lists.llvm.org >> >> >> >> delena created this revision. >> delena added reviewers: apilipenko, reames, Ayal, arsenm, pjcoup. >> delena added a subscriber: llvm-commits. >> delena set the repository for this revision to rL LLVM. >> >> This is an alternative proposal for supporting address space in intrinsics. It's applicable for any intrinsic, not only masked-load-store. >> Related tohttp://reviews.llvm.org/D17270 >> >> I this proposal I add non-zero address space suffix to intrinsic name. The original name looks like >> @llvm.xxx.xxx >> The same name with address space 1 : >> @llvm.xxx.xxx.a_1 >> >> (I did not update documentation. I'll do this if the proposed change looks reasonable for reviewers ). >> >> Repository: >> rL LLVM >> >> http://reviews.llvm.org/D17497 >> >> Files: >> ../include/llvm/IR/IRBuilder.h >> ../include/llvm/IR/Intrinsics.h >> ../lib/IR/Function.cpp >> ../lib/IR/IRBuilder.cpp >> ../lib/IR/Verifier.cpp >> ../test/Transforms/LoopVectorize/X86/gather_scatter.ll >> >> >> >> >> >> >> _______________________________________________ >> 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/20160304/5ec5be2a/attachment.html>
Possibly Parallel Threads
- Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
- Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
- enabling interleaved access loop vectorization
- enabling interleaved access loop vectorization
- enabling interleaved access loop vectorization