[OpenCL] Check that global samplers are const

[svenvh]

Summary

Patch by Simon Perretta.

[yaxunl]

include/clang/Basic/DiagnosticSemaKinds.td

8301

global sampler requires a const or constant address space qualifier

lib/Sema/SemaDecl.cpp

6085

should be else if ?

if the previous error already happens, it is unnecessary to do the next check

[svenvh]

Improve diagnostic text as suggested.

[svenvh]

lib/Sema/SemaDecl.cpp

6085

Sure, I can do that, I am just not sure what the desirable behaviour is here? They are different errors, so with the current patch it emits both diagnostics (see the first test change). If we just want one diagnostic (the first one in that test case), I can change it to else if; please let me know if you prefer that.

[yaxunl]

lib/Sema/SemaDecl.cpp

6085

On second thoughts, let's keep it as it is.

Can you add tests for

const sampler_t

const constant sampler_t

const global sampler_t

Thanks.

[svenvh]

Added more tests as suggested.

[yaxunl]

LGTM. Thanks!

include/clang/Basic/DiagnosticSemaKinds.td

 def err_sampler_initializer_not_integer : Error<
   "sampler_t initialization requires 32-bit integer, not %0">;
 def warn_sampler_initializer_invalid_bits : Warning<
   "sampler initializer has invalid %0 bits">, InGroup<SpirCompat>, DefaultIgnore;
 def err_sampler_argument_required : Error<
   "sampler_t variable required - got %0">;
 def err_wrong_sampler_addressspace: Error<
   "sampler type cannot be used with the __local and __global address space qualifiers">;
+def err_opencl_sampler_qualifier : Error<
+  "global sampler requires a const qualifier">;
 def err_opencl_cast_non_zero_to_event_t : Error<
   "cannot cast non-zero value '%0' to 'event_t'">;
 def err_opencl_global_invalid_addr_space : Error<
   "%select{program scope|static local|extern}0 variable must reside in %1 address space">;
 def err_missing_actual_pipe_type : Error<
   "missing actual type specifier for pipe">;
 def err_reference_pipe_type : Error <
   "pipes packet types cannot be of reference type">;

lib/Sema/SemaDecl.cpp

if (!getOpenCLOptions().isEnabled("cl_khr_fp16")) {
       // OpenCL v1.2 s6.1.1.1: reject declaring variables of the half and
       // half array type (unless the cl_khr_fp16 extension is enabled).
       if (Context.getBaseElementType(R)->isHalfType()) {
         Diag(D.getIdentifierLoc(), diag::err_opencl_half_declaration) << R;
         D.setInvalidType();
       }
     }
 
+    if (R->isSamplerT()) {
       // OpenCL v1.2 s6.9.b p4:
       // The sampler type cannot be used with the __local and __global address
       // space qualifiers.
-    if (R->isSamplerT() && (R.getAddressSpace() == LangAS::opencl_local ||
-      R.getAddressSpace() == LangAS::opencl_global)) {
+      if (R.getAddressSpace() == LangAS::opencl_local ||
+          R.getAddressSpace() == LangAS::opencl_global) {
         Diag(D.getIdentifierLoc(), diag::err_wrong_sampler_addressspace);
       }
 
+      // OpenCL v1.2 s6.12.14.1:
+      // A global sampler must be declared with either the constant address
+      // space qualifier or with the const qualifier.
+      if (DC->isTranslationUnit() &&
+          !(R.getAddressSpace() == LangAS::opencl_constant ||
+          R.isConstQualified())) {
+        Diag(D.getIdentifierLoc(), diag::err_opencl_sampler_qualifier);
+        D.setInvalidType();
+      }
+    }
+
     // OpenCL v1.2 s6.9.r:
     // The event type cannot be used with the __local, __constant and __global
     // address space qualifiers.
     if (R->isEventT()) {
       if (R.getAddressSpace()) {
         Diag(D.getLocStart(), diag::err_event_t_addr_space_qual);
         D.setInvalidType();
       }

test/SemaOpenCL/sampler_t.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE       2
 #define CLK_NORMALIZED_COORDS_TRUE      1
 #define CLK_FILTER_NEAREST              0x10
 #define CLK_FILTER_LINEAR               0x20
 
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address space must be initialized}}
-global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}}
+global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}} expected-error {{global sampler requires a const qualifier}}
 
 constant sampler_t glb_smp4 = 0;
 #ifdef CHECK_SAMPLER_VALUE
 // expected-warning@-2{{sampler initializer has invalid Filter Mode bits}}
 #endif
 
 constant sampler_t glb_smp5 = 0x1f;
 #ifdef CHECK_SAMPLER_VALUE

 void foo(sampler_t); // expected-note{{passing argument to parameter here}}
 
 constant struct sampler_s {
   sampler_t smp; // expected-error{{the 'sampler_t' type cannot be used to declare a structure or union field}}
 } sampler_str = {0};
 
 sampler_t bad(void); //expected-error{{declaring function return value of type 'sampler_t' is not allowed}}
 
+sampler_t global_nonconst_smp = 0; // expected-error {{global sampler requires a const qualifier}}
+
 void kernel ker(sampler_t argsmp) {
   local sampler_t smp; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}}
   const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
   const sampler_t const_smp2;
   const sampler_t const_smp3 = const_smp;
   const sampler_t const_smp4 = f();
   const sampler_t const_smp5 = 1.0f; // expected-error{{initializing 'const sampler_t' with an expression of incompatible type 'float'}}
   const sampler_t const_smp6 = 0x100000000LL; // expected-error{{sampler_t initialization requires 32-bit integer, not 'long long'}}