[cfe-commits] [PATCH v3] Set some OpenCl specification mandated types/alignments/etc

[Tanya Lattner]

In general, I think this is an interesting approach. We have done what Eli suggested and made our own targets (subclasses of existing targets). The primary reason for this is that the data layout DescriptionString might also need to be changed as well, otherwise you might not get what you expect during code generation.

So while I think its an interesting approach, I am not 100% convinced its the right thing. The Width/Alignment I could be convinced on, but I do not think the AddrSpace map should be set to that default. There is already a "fake" address space map and I believe that any OpenCL target should set their own addr space in their respective Target .

Also, LongLong is supposed to be 128 (reserved).

-Tanya


On 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<opencl3.diff>