[OpenCL] Image types diagnostics.

Clang Frontend
#1

Hi,

I’m looking for advice on implementing diagnostics required by OpenCL v1.2 s6.12.14:

“Image memory objects that are being read by a kernel should be declared with the __read_only qualifier. write_image calls to image memory objects declared with the __read_only qualifier will generate a compilation error.”

My idea was to add image access qualifiers to the mangling and because built-in functions like write_image are mangled compiler will throw an error on passing an image with wrong access qualifier as it will not be able to find matching write_image built-in function.

Unfortunately it turned out that top level qualifiers are discarded by mangler.
Is any other better way to implement such diagnostics?

The main problem with it is that operations on OpenCL built-in types like images are defined as built-in function calls, which are typically defined by OpenCL run-time and not by Clang. Clang has no pre-defined knowledge about them.

I filed a ticket on clang #24937 to track similar problem, but it didn’t get any attention so far.

Thanks,
Alexey

#2

I think we had several discussions on that previously, but nothing has been concluded unfortunately.

I would really like to see it fixed properly in Clang.

My idea was to add image access qualifiers to the mangling and because built-in functions like write_image are mangled compiler will throw an error on passing an image with wrong access qualifier as it will not be able to find matching write_image built-in function.

Would this mean the error would happen during linking? I think it would be nicer to diagnose it earlier in the frontend while checking for available function declarations.

As far as I remember there was a proposal to implement image access as a qualifier field similar to address spaces. Let’s say bits 30-31 of Qualifiers::Mask would store access qualifiers.

I was talking to spec editors at some point and I had a feeling that the intention is that access qualifiers are actually part of the type (in contrast to other qualifiers). That’s why the solution above with the qualifiers is not entirely correct and would require special casing images in many places because their access qualifier always has to be taken into account. Therefore I had the idea of treating images together with access qualifiers as an integral builtin type, for example:

read_only image1d_t → OCLImage1dROTy

write_only image1d_t → OCLImage1dWOTy

read_write image1d_t → OCLImage1dRWTy

This would allow us to get such diagnostics handled by Clang automatically (as the types will no longer match):

read(read_only image1d_t img);

kernel foo(write_only image1d_t img){

read(img); // error here

}

Also I think we need to differentiate image access qualifiers in the generated IR code, because read_only, write_only and read_write images might be treated differently on different architectures. Currently there is no way for the backend to know what the image access type originally was.

I think I might have a patch for the solution with different builtin types, that I could put up for review after rebasing. I just would like to get a feeling whether that would be a good way to go.

Cheers,

Anastasia

#3

Hi Anastasia,

I think it would be useful to preserve access qualifiers in LLVM IR, so I would like to try your patch.
In addition to that the same approach might be applicable for pipes. Could your patch be extended for pipe data types too?

Thanks,
Alexey

#4

Hi Aleksey,

I think pipes should work too although it might follow slightly different path (as it’s a different class of a type).

I will create the review next days and we can discuss it further and also see whether to add pipe handling directly or in a separate commit.

Cheers,

Anastasia