[OpenCL] Fix support for cl_khr_mipmap_image_writes

[AlexeySotkin]

Summary

Text of the extension is available here:https://github.com/KhronosGroup/OpenCL-Docs/blob/master/ext/cl_khr_mipmap_image.asciidoc

Patch by Ilya Mashkov

[asavonic]

What about get_image_num_mip_levels functions defined in the extension specification?

Edit: I mean, should the get_image_num_mip_levels(write_only img) function be only available if cl_khr_mipmap_image_writes extension is supported, or cl_khr_mipmap_image is enough?

clang/lib/Headers/opencl-c.h

14780

Shouldn't "color" be renamed to "depth" here as well?

[Anastasia]

clang/lib/Headers/opencl-c.h

14686

Do we actually need pragma for this extension? I.e. does it need to activate any special mode in the compiler?

[AlexeySotkin]

In D71460#1783390, @asavonic wrote:

What about get_image_num_mip_levels functions defined in the extension specification?

Edit: I mean, should the get_image_num_mip_levels(write_only img) function be only available if cl_khr_mipmap_image_writes extension is supported, or cl_khr_mipmap_image is enough?

I think cl_khr_mipmap_image is enough, because "the cl_khr_mipmap_image_writes extension adds built-in functions that can be used to write a mip-mapped image", while get_image_num_mip_levels(write_only img) only retrieve a property of the image.

[AlexeySotkin]

clang/lib/Headers/opencl-c.h

14686

I'm not aware of any special mode required for this extension. These begin/end pragma lines were added to disable the extension by default as it is required by the OpenCL extension spec. Is there any other mechanism which should be used for this purpose?
Probably, we should do the same for cl_khr_mipmap_image(and maybe others?), because with the current compiler, built-ins from this extension can be compiled successfully even if #pragma OPENCL EXTENSION cl_khr_mipmap_image : disable is specified. See https://godbolt.org/z/fNEWuG

[AlexeySotkin]

Rename color to depth

[Anastasia]

clang/lib/Headers/opencl-c.h

14686

What I am saying is that pragma is only typically used to activate some special mode in the compiler. If we don't need to activate anything perhaps we don't need to add pragma at all? It simplifies compiler and application code too. Would regular include mechanisms be enough to guard the availability of those functions?

Maybe this thread will help to understand the topic better: https://github.com/KhronosGroup/OpenCL-Docs/issues/82

[AlexeySotkin]

clang/lib/Headers/opencl-c.h

14686

I think you are right. I have removed the pragma.

[Anastasia]

LGTM! Thanks!

clang/lib/Headers/opencl-c.h

14686

I think we probably should get rid of most of extensions in clang/include/clang/Basic/OpenCLExtensions.def and just try to implement the extensions macro using target specific header.

But it doesn't belong to your change and it's probably too big of the change anyway. Just a thought. :)

[Anastasia]

Adding @svenvh mainly to check if any fix is needed for the TableGen BIFs too?

[svenvh]

In D71460#1844959, @Anastasia wrote:

Adding @svenvh mainly to check if any fix is needed for the TableGen BIFs too?

Yes I believe we should also update clang/lib/Sema/OpenCLBuiltins.td.

[svenvh]

I have updated the TableGen OpenCL builtin definitions accordingly in 91b3083aecd ("[OpenCL] Fix tblgen support for cl_khr_mipmap_image_writes", 2020-02-05).

[AlexeySotkin]

Is there any chance to get this commit cherry-picked to 10.x branch?

clang/include/clang/Basic/OpenCLExtensions.def

 OPENCLEXT_INTERNAL(cl_khr_gl_depth_images, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_gl_msaa_sharing, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_spir, 120, ~0U)
 
 // OpenCL 2.0.
 OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
+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)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)
 
 // Clang Extensions.
 OPENCLEXT_INTERNAL(cl_clang_storage_class_specifiers, 100, ~0U)
 
 // AMD OpenCL extensions

clang/lib/Headers/opencl-c.h

 
 #ifdef cl_khr_depth_images
 void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, float color);
 void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, float color);
 #endif //cl_khr_depth_images
 
 // OpenCL Extension v2.0 s9.18 - Mipmaps
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, float4 color);
 void __ovld write_imagei(write_only image1d_t image, int coord, int lod, int4 color);
 void __ovld write_imageui(write_only image1d_t image, int coord, int lod, uint4 color);
 
 void __ovld write_imagef(write_only image1d_array_t image_array, int2 coord, int lod, float4 color);
 void __ovld write_imagei(write_only image1d_array_t image_array, int2 coord, int lod, int4 color);
 void __ovld write_imageui(write_only image1d_array_t image_array, int2 coord, int lod, uint4 color);
 
 void __ovld write_imagef(write_only image2d_t image, int2 coord, int lod, float4 color);
 void __ovld write_imagei(write_only image2d_t image, int2 coord, int lod, int4 color);
 void __ovld write_imageui(write_only image2d_t image, int2 coord, int lod, uint4 color);
 
 void __ovld write_imagef(write_only image2d_array_t image_array, int4 coord, int lod, float4 color);
 void __ovld write_imagei(write_only image2d_array_t image_array, int4 coord, int lod, int4 color);
 void __ovld write_imageui(write_only image2d_array_t image_array, int4 coord, int lod, uint4 color);
 
-void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int lod, float color);
-void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, int lod, float color);
+void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int lod, float depth);
+void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, int lod, float depth);
 
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imagef(write_only image3d_t image, int4 coord, int lod, float4 color);
 void __ovld write_imagei(write_only image3d_t image, int4 coord, int lod, int4 color);
 void __ovld write_imageui(write_only image3d_t image, int4 coord, int lod, uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : end
+#endif //defined(cl_khr_mipmap_image_writes)
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 // Image write functions for half4 type
 #ifdef cl_khr_fp16
 void __ovld write_imageh(write_only image1d_t image, int coord, half4 color);
 void __ovld write_imageh(write_only image2d_t image, int2 coord, half4 color);
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imageh(write_only image3d_t image, int4 coord, half4 color);

 #endif
 
 #ifdef cl_khr_depth_images
 void __ovld write_imagef(read_write image2d_depth_t image, int2 coord, float color);
 void __ovld write_imagef(read_write image2d_array_depth_t image, int4 coord, float color);
 #endif //cl_khr_depth_images
 
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#ifdef cl_khr_mipmap_image_writes
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(read_write image1d_t image, int coord, int lod, float4 color);
 void __ovld write_imagei(read_write image1d_t image, int coord, int lod, int4 color);
 void __ovld write_imageui(read_write image1d_t image, int coord, int lod, uint4 color);
 
 void __ovld write_imagef(read_write image1d_array_t image_array, int2 coord, int lod, float4 color);
 void __ovld write_imagei(read_write image1d_array_t image_array, int2 coord, int lod, int4 color);
 void __ovld write_imageui(read_write image1d_array_t image_array, int2 coord, int lod, uint4 color);
 
 void __ovld write_imagef(read_write image2d_t image, int2 coord, int lod, float4 color);
 void __ovld write_imagei(read_write image2d_t image, int2 coord, int lod, int4 color);
 void __ovld write_imageui(read_write image2d_t image, int2 coord, int lod, uint4 color);
 
 void __ovld write_imagef(read_write image2d_array_t image_array, int4 coord, int lod, float4 color);
 void __ovld write_imagei(read_write image2d_array_t image_array, int4 coord, int lod, int4 color);
 void __ovld write_imageui(read_write image2d_array_t image_array, int4 coord, int lod, uint4 color);
 
 void __ovld write_imagef(read_write image2d_depth_t image, int2 coord, int lod, float color);
 void __ovld write_imagef(read_write image2d_array_depth_t image, int4 coord, int lod, float color);
 
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imagef(read_write image3d_t image, int4 coord, int lod, float4 color);
 void __ovld write_imagei(read_write image3d_t image, int4 coord, int lod, int4 color);
 void __ovld write_imageui(read_write image3d_t image, int4 coord, int lod, uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : end
+#endif //cl_khr_mipmap_image_writes
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 // Image write functions for half4 type
 #ifdef cl_khr_fp16
 void __ovld write_imageh(read_write image1d_t image, int coord, half4 color);
 void __ovld write_imageh(read_write image2d_t image, int2 coord, half4 color);
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imageh(read_write image3d_t image, int4 coord, half4 color);

clang/test/SemaOpenCL/extension-version.cl

 #ifdef cl_khr_mipmap_image
 #error "Incorrect cl_khr_mipmap_image define"
 #endif
 // expected-warning@+2{{unsupported OpenCL extension 'cl_khr_mipmap_image' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_khr_mipmap_image : enable
 
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+#ifndef cl_khr_mipmap_image_writes
+#error "Missing cl_khr_mipmap_image_writes define"
+#endif
+#else
+#ifdef cl_khr_mipmap_image_writes
+#error "Incorrect cl_khr_mipmap_image_writes define"
+#endif
+// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_mipmap_image_writes' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : enable
+
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #ifndef cl_khr_srgb_image_writes
 #error "Missing cl_khr_srgb_image_writes define"
 #endif
 #else
 // expected-warning@+2{{unsupported OpenCL extension 'cl_khr_srgb_image_writes' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_khr_srgb_image_writes : enable