[cfe-dev] OpenCL support

[Peter Collingbourne]

On Wed, Dec 22, 2010 at 03:54:33PM -0000, Anton Lokhmotov wrote:
> > Comments on the second patch to come...
> I'm attaching it again for convenience...

Here are my comments on the second patch.

> +enum OpenCLAddressSpace {
> +  OPENCL_PRIVATE = 0,
> +  OPENCL_GLOBAL = 1,
> +  OPENCL_LOCAL = 2,
> +  OPENCL_CONSTANT = 3
> +};

If we are going to standardise these address space numbers across
Clang and the LLVM backends, this enum declaration should be added
somewhere in LLVM so that the backends have access to it.  Perhaps a
new header file called something like "llvm/Support/AddressSpaces.h"?

> +enum OpenCLImageAccess {
> +  OPENCL_READ_ONLY = 1,
> +  OPENCL_WRITE_ONLY = 2,
> +  OPENCL_READ_WRITE = 3
> +};

This should live in AST somewhere - once the image access attribute
is added to the AST we will need access to this declaration from all
AST clients.

Also, please add test cases which test that:

1) the LLVM produced by the code generator is annotated with the
   correct address space numbers for each new address space added;

2) the image access attributes are recognised by the parser/semantic
   analyser.

Other than that, LGTM.

Thanks,
-- 
Peter