[cfe-dev] [OpenCL patch] kernel attributes implementation

[Tanya Lattner]

Guy & Anton,

I'll just review this merged patch. Here is my feedback:

- Remove the type specifier diagnostic. I don't think its needed (can you just use the existing one?) and its unrelated to the metadata.

- I'd like to suggest changing how the metadata is organized. So it would be like this:
NamedMetaData Node - opencl.kernels
	MDNode Pair of Function*, MDNode which is a list of all the metadata
		MDNode list that is a key/value pair. So for each attribute you have key (ie. "vec_type_hint"), and then the value(s).

Its just a slight variation on how you have things (where you have a list with the first arg being the key). This makes it pretty obvious what the layout is in my opinion.

Does that seem reasonable?

- The DummyType is a creative solution. I think we'll need Doug to sign off on that change though.

- Add some comments to the function EmitOpenCLKernelMetadata. Maybe one sentence to say what the metadata is and reference to spec.

- Nit pick: Add periods to your comments in AttributeList.h, 


Thanks,
Tanya
		

On Jan 4, 2012, at 8:46 AM, Anton Lokhmotov wrote:

> Hi Guy,
> 
> We've done the merge (including your most recent test using regular
> expressions).  Please review if anything is missing.
> 
> I don't quite understand this change (in lib/Sema/SemaType.cpp):
> 
> +      } else if (S.getLangOptions().OpenCL) {
> +        S.Diag(DeclLoc, diag::err_missing_type_specifier_opencl)
> +          << DS.getSourceRange();
> +        declarator.setInvalidType(true);
> 
> Could you please explain how it relates to the kernel attributes?
> 
> Cheers,
> Anton.<kernel_attributes.patch>