[OpenCL] Remove the need for subgroups extension pragma in enqueue kernel and pipe builtins

[Anastasia]

Summary

There is no extension pragma requirement in the spec for these functions and all other builtin functions are available without the pragama.

This patch simplifies the parser and makes the language semantics consistent.

[azabaznov]

I wish it could be true... But cl_khr_subgroups still requires pragma in some versions as it wasn't a core feature earlier (https://www.khronos.org/registry/OpenCL/sdk/2.2/docs/man/html/cl_khr_subgroups.html).

I know that we are going around in circles and we discussed this many times... But I see your point and this will definitely lead to overall simplification of OpenCL codebase in clang. Maybe we could add a comment into docs that we are deprecating pragmas?

[Anastasia]

In D100984#2707980, @azabaznov wrote:

I wish it could be true... But cl_khr_subgroups still requires pragma in some versions as it wasn't a core feature earlier (https://www.khronos.org/registry/OpenCL/sdk/2.2/docs/man/html/cl_khr_subgroups.html).

Btw I am not suggesting removing the pragma. We will still have to parse it for backward compatibility. I am only dropping the requirement of using it in order to call get_kernel_max_sub_group_size_for_ndrange or get_kernel_sub_group_count_for_ndrange when the extension is supported. Because I don't see why this would be needed especially that all other functions from subgroups can be called without the pragma https://godbolt.org/z/MYavchPT3.

Btw the newer extension specs don't contain the pragma e.g.
https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#_extended_subgroup_functions

I know that we are going around in circles and we discussed this many times... But I see your point and this will definitely lead to overall simplification of OpenCL codebase in clang. Maybe we could add a comment into docs that we are deprecating pragmas?

Right now we have the following in the docs: https://clang.llvm.org/docs/OpenCLSupport.html#implementation-guidelines

Note that some legacy extensions (published prior to OpenCL 3.0) still provide some non-conformant functionality for pragmas e.g. add diagnostics on the use of types or functions. This functionality is not guaranteed to remain in future releases. However, any future changes should not affect backward compatibility.

This is not quite deprecation but it makes it clear that the behavior can change as soon as backward compatibility is preserved.

Would you like us to add or ammend anything?

[azabaznov]

Btw I am not suggesting removing the pragma. We will still have to parse it for backward compatibility. I am only dropping the requirement of using it in order to call get_kernel_max_sub_group_size_for_ndrange or get_kernel_sub_group_count_for_ndrange when the extension is supported. Because I don't see why this would be needed especially that all other functions from subgroups can be called without the pragma https://godbolt.org/z/MYavchPT3.

Oh! I didn't get that! I see another one https://godbolt.org/z/3GPW31W9c, https://godbolt.org/z/dYasedxjx. This is also fixed with your patch, right?

Btw the newer extension specs don't contain the pragma e.g. https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#_extended_subgroup_functions

Indeed... No wording about pragma even in OpenCL C 2.0...

Right now we have the following in the docs: https://clang.llvm.org/docs/OpenCLSupport.html#implementation-guidelines

Note that some legacy extensions (published prior to OpenCL 3.0) still provide some non-conformant functionality for pragmas e.g. add diagnostics on the use of types or functions. This functionality is not guaranteed to remain in future releases. However, any future changes should not affect backward compatibility.

This is not quite deprecation but it makes it clear that the behavior can change as soon as backward compatibility is preserved.

Would you like us to add or ammend anything?

I think this fine for this change since the patch doesn't change anything but fixes a bug.

[Anastasia]

In D100984#2708168, @azabaznov wrote:

Btw I am not suggesting removing the pragma. We will still have to parse it for backward compatibility. I am only dropping the requirement of using it in order to call get_kernel_max_sub_group_size_for_ndrange or get_kernel_sub_group_count_for_ndrange when the extension is supported. Because I don't see why this would be needed especially that all other functions from subgroups can be called without the pragma https://godbolt.org/z/MYavchPT3.

Oh! I didn't get that! I see another one https://godbolt.org/z/3GPW31W9c, https://godbolt.org/z/dYasedxjx. This is also fixed with your patch, right?

True, I have forgotten about those. :)

PS the patch fixes those too.

[azabaznov]

LGTM!

[mantognini]

Thanks, I believe this goes in the right direction.

clang/lib/Sema/SemaChecking.cpp

if (!(*I)->isPointerType() || !(*I)->getPointeeType()->isVoidType() ||
       IllegalParams = true;
     }
   }
 
   return IllegalParams;
 }
 
 static bool checkOpenCLSubgroupExt(Sema &S, CallExpr *Call) {
-  if (!S.getOpenCLOptions().isAvailableOption("cl_khr_subgroups",
-                                              S.getLangOpts())) {
+  if (!S.getOpenCLOptions().isSupported("cl_khr_subgroups", S.getLangOpts())) {
     S.Diag(Call->getBeginLoc(), diag::err_opencl_requires_extension)
         << 1 << Call->getDirectCallee() << "cl_khr_subgroups";
     return true;
   }
   return false;
 }
 
 static bool SemaOpenCLBuiltinNDRangeAndBlock(Sema &S, CallExpr *TheCall) {

clang/test/SemaOpenCL/cl20-device-side-enqueue.cl

 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir-unknown-unknown" -verify -pedantic -fsyntax-only -DB32 -DQUALS=
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir-unknown-unknown" -verify -pedantic -fsyntax-only -DB32 -DQUALS= -cl-ext=-cl_khr_subgroups
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir-unknown-unknown" -verify -pedantic -fsyntax-only -DB32 -DQUALS="const volatile"
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir64-unknown-unknown" -verify -pedantic -fsyntax-only -Wconversion -DWCONV -DQUALS=
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir64-unknown-unknown" -verify -pedantic -fsyntax-only -Wconversion -DWCONV -DQUALS="const volatile"
 
 typedef struct {int a;} ndrange_t;
 // Diagnostic tests for different overloads of enqueue_kernel from Table 6.13.17.1 of OpenCL 2.0 Spec.
 kernel void enqueue_kernel_tests() {
   queue_t default_queue;
kernel void work_group_size_tests() {
   });
   size = get_kernel_preferred_work_group_size_multiple(block_B);   // expected-error {{blocks used in enqueue_kernel call are expected to have parameters of type 'local void*'}}
   size = get_kernel_preferred_work_group_size_multiple(block_D);   // expected-error {{blocks used in enqueue_kernel call are expected to have parameters of type 'local void*'}}
   size = get_kernel_preferred_work_group_size_multiple();          // expected-error {{too few arguments to function call, expected 1, have 0}}
   size = get_kernel_preferred_work_group_size_multiple(1);         // expected-error{{expected block argument}}
   size = get_kernel_preferred_work_group_size_multiple(block_A, 1); // expected-error{{too many arguments to function call, expected 1, have 2}}
 }
 
+#ifdef cl_khr_subgroups
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
-
 kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}}
 }
 
 kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
   buf[0] = get_kernel_sub_group_count_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_sub_group_count_for_ndrange', expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_sub_group_count_for_ndrange', expected block argument type}}
 }
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
-
+#else
 kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups support}}
 }
 
 kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups support}}
 }
+#endif // ifdef cl_khr_subgroups