[cfe-commits] [PATCH v3] Set some OpenCl specification mandated types/alignments/etc
David Tweed
david.tweed at arm.com
Tue Dec 11 07:22:13 PST 2012
Thanks for the comments. I'll consult about whether we still think this is
the right way to do things; it certainly sounds like the address space is
sufficiently variable that it shouldn't go into setForcedLangOptions even if
the more fixed things are. The "long long" size was indeed a typo. As
regards address space qualifier numbering, the current SPIR draft on
www.khronos.org/registry/cl still specifies this mapping in sec 2.2 on page
14.
Regards,
Dave
-----Original Message-----
From: Benyei, Guy [mailto:guy.benyei at intel.com]
Sent: 09 December 2012 07:25
To: Richard Smith; David Tweed
Cc: cfe-commits at cs.uiuc.edu; tonic at nondot.org
Subject: RE: [cfe-commits] [PATCH v3] Set some OpenCl specification mandated
types/alignments/etc
Hello,
+ LongLongWidth = 64; LongLongAlign = 64;
The "long long" type is currently reserved for future use in OpenCL. I don't
think forcing it to be 64 bit is right.
+ static const LangAS::Map OpenCLAddrSpaceMap = {
+ 0, // opencl_private
+ 1, // opencl_global
+ 2, // opencl_constant
+ 3 // opencl_local
+ };
+
+ AddrSpaceMap = &OpenCLAddrSpaceMap;
The idea behind adding the AddrSpaceMap was to support different ordering of
the address spaces. AFAIK, the different vendors using Clang as frontend
compiler to OpenCL currently use different ordering. The current SPIR spec
also defines a different order (3-local, 2-constant), and the CUDA address
spaces are missing. Anyhow, I don't think any numbering should be forced
here.
+int v4[(sizeof(long long) == 8) -1];
+int v5[(__alignof(long long) == 8) -1];
Again, "long long" is reserved.
Thanks
Guy Benyei
-----Original Message-----
From: cfe-commits-bounces at cs.uiuc.edu
[mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Richard Smith
Sent: Saturday, December 08, 2012 00:19
To: David Tweed
Cc: cfe-commits at cs.uiuc.edu; tonic at nondot.org
Subject: Re: [cfe-commits] [PATCH v3] Set some OpenCl specification mandated
types/alignments/etc
+ //OpenCL fully specifies the size, alignment and representation
of floating types
Missing space between // and comment. (Also missing full stop.)
+ //endian-specific spaces can be added by individual backends as
+ desired
Missing capitalization, space full stop.
+ static const LangAS::Map OpenCLAddrSpaceMap = {
+ 0, // opencl_private
+ 1, // opencl_global
+ 2, // opencl_constant
+ 3 // opencl_local
+ };
This does not match AddressSpaces.h. You don't seem to have any tests for
these values.
+++ test/Misc/languageOptsOpenCL.cl
This test would fit better in test/SemaOpenCL/. Please follow the existing
convention for naming test files: lowercase with hyphens to separate words.
It would also be nice to have several RUN: lines with different triples, to
test that we get the same sizes no matter what target we choose.
On Fri, Dec 7, 2012 at 7:15 AM, David Tweed <david.tweed at arm.com> wrote:
> (Randomly adding various people who've been unwise enough to have
> contribute to OpenCL/SPIR thread using their email address.)
>
> ping^2 on the general approach, particularly anyone who's involved in
> OpenCL implementation. (Current status is Eli Friedman would like
> comments on general approach from others, there's been one positive
> response from Pekka, but we're trying to ensure silence==acceptance
> rather than ==didn't see
> mail.)
>
> Regards,
> Dave
>
> -----Original Message-----
> From: David Tweed [mailto:david.tweed at arm.com]
> Sent: 30 November 2012 10:23
> To: David Tweed; David Tweed; 'Eli Friedman'
> Cc: 'Pekka Jääskeläinen'; 'cfe-commits at cs.uiuc.edu'
> Subject: RE: [PATCH v2] Set some OpenCl specification mandated
> types/alignments/etc
>
> ping for comments/objections/exasperation on this general approach?
>
> Regards,
> Dave
>
> -----Original Message-----
> From: David Tweed [mailto:david.tweed at arm.com]
> Sent: 27 November 2012 16:11
> To: David Tweed; 'Eli Friedman'
> Cc: 'Pekka Jääskeläinen'; 'cfe-commits at cs.uiuc.edu'
> Subject: RE: [PATCH v2] Set some OpenCl specification mandated
> types/alignments/etc
>
> Revised patch actually attached.
>
> -----Original Message-----
> From: David Tweed [mailto:david.tweed at arm.com]
> Sent: 27 November 2012 16:09
> To: 'Eli Friedman'
> Cc: Pekka Jääskeläinen; cfe-commits at cs.uiuc.edu
> Subject: RE: [PATCH v2] Set some OpenCl specification mandated
> types/alignments/etc
>
> Hi Eli, thanks for the feedback. In general I've tested everything I
> know how to test using only the output of a compilation.
>
> | Why are you messing with LargeArrayMinWidth and LargeArrayAlign?
> | Please explain in the patch and add tests.
>
> That looks like something I forgot was an implementation thing rather
> than standard. Removed.
>
> | Why are you messing with the floating point formats (particularly
> | without setting the size and alignment alongside the format)?
> | Please explain in the patch and add tests.
>
> I didn't really think about the floating point formats possibly
> changing size; fixed. Explained in patch that all these things are
> language specified.
>
> | Is a fixed OpenCL address map really appropriate for every target?
>
> This patch sets up the mandated address spaces for OpenCL; presumably
> targets that want to add more can extend it themselves.
>
> | Also, please wait at least a couple more days; all the Apple people
> | were off last week, and I want to give them time to comment.
>
> No problem waiting a while: the problem with patches areas (like this
> one) in extremely dull areas is knowing if there's no response just
> because it's so uninteresting rather than there hasn't been time for
> commenters, but I forgot to factor in American Thanksgiving.
>
> Regards,
> Dave
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
---------------------------------------------------------------------
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.
More information about the cfe-commits
mailing list