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>
Demikhovsky, Elena via llvm-dev
2016-Mar-07 09:42 UTC
[llvm-dev] Fwd: [PATCH] D17497: Support arbitrary address space for intrinsics
I see your point. I’ll try to implement forward reference in intrinsics pattern. Otherwise verifier does not check correlation between pointer and data types. - Elena From: Philip Reames [mailto:listmail at philipreames.com] Sent: Friday, March 04, 2016 21:35 To: llvm-dev <llvm-dev at lists.llvm.org>; Demikhovsky, Elena <elena.demikhovsky at intel.com>; Artur Pilipenko <apilipenko at azulsystems.com> Subject: Re: [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 with Artur'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><mailto:elena.demikhovsky at intel.com> Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org<mailto:reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org> To: elena.demikhovsky at intel.com<mailto:elena.demikhovsky at intel.com>, apilipenko at azulsystems.com<mailto:apilipenko at azulsystems.com>, listmail at philipreames.com<mailto:listmail at philipreames.com>, ayal.zaks at intel.com<mailto:ayal.zaks at intel.com>, Matthew.Arsenault at amd.com<mailto:Matthew.Arsenault at amd.com>, pjcoup at gmail.com<mailto:pjcoup at gmail.com> CC: llvm-commits at lists.llvm.org<mailto: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 _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto: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<mailto: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. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160307/f88152d5/attachment-0001.html>
Artur Pilipenko via llvm-dev
2016-Mar-09 17:01 UTC
[llvm-dev] [PATCH] D17497: Support arbitrary address space for intrinsics
It’s not verified by intrinsic overloading mechanism, but there is an explicit check about masked load and store argument types. Artur On 07 Mar 2016, at 12:42, Demikhovsky, Elena <elena.demikhovsky at intel.com<mailto:elena.demikhovsky at intel.com>> wrote: I see your point. I’ll try to implement forward reference in intrinsics pattern. Otherwise verifier does not check correlation between pointer and data types. - Elena From: Philip Reames [mailto:listmail at philipreames.com] Sent: Friday, March 04, 2016 21:35 To: llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>>; Demikhovsky, Elena <elena.demikhovsky at intel.com<mailto:elena.demikhovsky at intel.com>>; Artur Pilipenko <apilipenko at azulsystems.com<mailto:apilipenko at azulsystems.com>> Subject: Re: [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 with Artur'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><mailto:elena.demikhovsky at intel.com> Reply-To: reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org<mailto:reviews+D17497+public+90f3d1b9468ba8ca at reviews.llvm.org> To: elena.demikhovsky at intel.com<mailto:elena.demikhovsky at intel.com>, apilipenko at azulsystems.com<mailto:apilipenko at azulsystems.com>, listmail at philipreames.com<mailto:listmail at philipreames.com>, ayal.zaks at intel.com<mailto:ayal.zaks at intel.com>, Matthew.Arsenault at amd.com<mailto:Matthew.Arsenault at amd.com>, pjcoup at gmail.com<mailto:pjcoup at gmail.com> CC: llvm-commits at lists.llvm.org<mailto: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 _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto: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<mailto: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. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160309/9fb1618e/attachment.html>