[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.


================
Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:298-302
+def write_imagef : Builtin<"write_imagef",
+                            [void_t,
+                              image2d_WO_t,
+                              VectorType<int_t, 2>,
+                              VectorType<float_t, 4>]>;
----------------
AlexeySotkin wrote:
> Pierre wrote:
> > AlexeySotkin wrote:
> > > It seems like there is something wrong with access qualifiers for images. I have applied this patch and tried to compile the following code:
> > > 
> > > ```
> > > typedef int int2 __attribute__((ext_vector_type(2)));
> > > typedef float float4 __attribute__((ext_vector_type(4)));
> > > 
> > > void kernel k(write_only image2d_t image, int2 coord, float4 data) {
> > >   write_imagef(image, coord, data);
> > > }
> > > 
> > > ```
> > > I got the following output:
> > > ```
> > > clang -cc1 -triple spir /work/tmp/tmp.cl -emit-llvm -o -  -fadd-opencl-builtins
> > > /work/tmp/tmp.cl:5:16: error: passing '__write_only image2d_t' to parameter of incompatible type '__read_only image2d_t'
> > >   write_imagef(image, coord, data);
> > >                          ^~~~~
> > > 1 error generated.
> > > ```
> > What you are saying is right. This patch is incomplete and some features are missing/ broken. 
> > I have a new version of the tablegen builtin feature where the access qualifiers are actually taken into account, but I cannot extract only this from my version. This would imply uploading the whole new version. 
> > The new version will hopefully be on top of this patch, making access qualifiers work.
> Thanks, Pierre. I'd like to start early testing of image builtins with this prototype. Do you have an idea when you will have image builtins done in this (or other) patch?
> If it is not going to happen in the nearest future, would you mind if I'll propose some changes for this patch/prototype meanwhile?
We're planning to commit the basic infrastructure early next week, which should make it easier to extend this work and enable contributions from the wider community.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60763/new/

https://reviews.llvm.org/D60763