On Mon, Dec 24, 2012 at 9:06 PM, Benyei, Guy <[email protected]> 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:[email protected]]
> Sent: Monday, December 24, 2012 16:24
> To: Benyei, Guy
> Cc: Douglas Gregor; [email protected]
> Subject: Re: [cfe-commits] [PATCH] OpenCL event type
>
> On Mon, Dec 24, 2012 at 4:07 PM, Benyei, Guy <[email protected]> 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 <[email protected]>*/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
