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: [email protected] [mailto:[email protected]]
On Behalf Of Richard Smith
Sent: Saturday, December 08, 2012 00:19
To: David Tweed
Cc: [email protected]; [email protected]
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 <[email protected]> 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:[email protected]]
> Sent: 30 November 2012 10:23
> To: David Tweed; David Tweed; 'Eli Friedman'
> Cc: 'Pekka Jääskeläinen'; '[email protected]'
> 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:[email protected]]
> Sent: 27 November 2012 16:11
> To: David Tweed; 'Eli Friedman'
> Cc: 'Pekka Jääskeläinen'; '[email protected]'
> Subject: RE: [PATCH v2] Set some OpenCl specification mandated
> types/alignments/etc
>
> Revised patch actually attached.
>
> -----Original Message-----
> From: David Tweed [mailto:[email protected]]
> Sent: 27 November 2012 16:09
> To: 'Eli Friedman'
> Cc: Pekka Jääskeläinen; [email protected]
> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
[email protected]
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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
