[PATCH] D89372: [OpenCL] Remove unused extensions

Wed Oct 14 11:18:29 PDT 2020

azabaznov 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)
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89372