[cfe-commits] [PATCH] OpenCL event type

[Dmitri Gribenko]

On Mon, Dec 24, 2012 at 9:06 PM, Benyei, Guy <guy.benyei at intel.com> wrote:
> All these diagnostics are quotes from the OpenCL 1.2 spec, section 6.9. I can change them to make them clearer, of course, but isn't it the best solution to quote the spec?

If these terms are used consistently in OpenCL spec, there might be
value in using them, too.  But most other diagnostics have 'member'
and 'file scope' in them, so our messages will be inconsistent.

But these two comments still apply:

> Please make the diagnostic more specific by choosing a single qualifier with %select{}.
>
> Please consistently refer to the type as 'event_t' (like in the first diagnostic).

Dmitri

>
> Thanks
>      Guy
>
>
> -----Original Message-----
> From: Dmitri Gribenko [mailto:gribozavr at gmail.com]
> Sent: Monday, December 24, 2012 16:24
> To: Benyei, Guy
> Cc: Douglas Gregor; cfe-commits at cs.uiuc.edu
> Subject: Re: [cfe-commits] [PATCH] OpenCL event type
>
> On Mon, Dec 24, 2012 at 4:07 PM, Benyei, Guy <guy.benyei at intel.com> wrote:
>> Hi,
>> Thanks for the review, attached the updated patch.
>> Implemented the event_t specific restrictions from the OpenCL 1.2 spec, section 6.9. Changed the AST bit code for event_t - the value 44 was actually reserved for samplers, but that's not an issue too.
>> Defining events as RValue was clearly wrong, so I've removed that part, and also checked for a proper 0 value in TryOCLNULLEventInitialization.
>> I've removed the restriction about initializing event_t variables. The OpenCL spec doesn't specify this, as most of the event_t behavior is defined briefly in the OpenCL built-ins part only.
>
> +def err_event_t_kernel_arg : Error<
> +  "arguments to kernel functions in a program cannot be declared to be of "
> +  "type event_t">;
> +def err_event_t_global_var : Error<
> +  "the event type cannot be used to declare a program scope variable">;
>
> It is called 'file scope' (unless it is called differently in OpenCL spec).
>
> +def err_event_t_struct_field : Error<
> +  "the event type cannot be used to declare a structure or union
> +field">;
>
> It should say "a data member".  (It is OK to be specific like "a struct data member", but that should be done with %select{})
>
> +def err_event_t_addr_space_qual : Error<
> +  "the event type cannot be used with the __local, __constant and __global "
> +  "address space qualifiers">;
>
> Please make the diagnostic more specific by choosing a single qualifier with %select{}.
>
> Please consistently refer to the type as 'event_t' (like in the first diagnostic).



-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/