[PATCH] D89372: [OpenCL] Remove unused extensions

Wed Oct 14 12:22:14 PDT 2020

Anastasia added inline comments.

================
Comment at: clang/include/clang/Basic/OpenCLExtensions.def:74
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U)
-OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
----------------
azabaznov wrote:
> mantognini wrote:
> > azabaznov wrote:
> > > cl_khr_srgb_image_writes - Extension allowing writes to sRGB images from a kernel. This extension enables write_imagef built-in function as it described [[ https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_Ext.html#cl_khr_srgb_image_writes | here]]. So I think we shouldn't remove it. Do I miss something?
> > On second reading, this one is slightly ambiguous. On the language side, the extension doesn't add an overload; it only specifies that existing overload can be used with a different kind of image. On the API side, it does extend the set of features. But at the same time if the extended API is not supported, it's not possible to create an image in the first place and therefore impossible to call write_imagef. So I question the usefulness of such macro on the device side. Does this argument convince you it should be removed?
> > it's not possible to create an image in the first place and therefore impossible
> Not quite that right. Referring to [[ https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html#conversion-rules-for-srgba-and-sbgra-images | this ]]:
> 
> read_imagef built-in functions for OpenCL C 2.0 devices do implicit conversions to RGB for sRGB images. However, implicit conversion from sRGB to RGB is done on write_imagef only if corresponding extension is supported. Otherwise, explicit conversions inside a kernel may be required.
> 
> I'm agree that this one is kind of confusing and requires some extra investigation. But I think now we should remove only those extensions which are only API related for sure.
Ok, thanks for providing extra information. I agree this deserves extra clarifications.

I am not sure whether the frontend will be able to perform such conversions since it doesn't have information regarding the channels. The image types are completely opaque. That means that potentially the BIFs can handle the conversion internally. However, I am unclear whether the macro can be useful i.e. whether there is anything that can be done differently at the source level based on its availability in OpenCL kernel code.

It should be ok to leave this out for now from the clean up unless someone else can help quickly with some more insights.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89372