[OpenCL] Prohibit using reserve_id_t in program scope.

[Anastasia]

Btw, Spec v2.0 s6.9.p seems to mention other types as well!

lib/Sema/SemaDecl.cpp

5923 ↗

(On Diff #79226)

• Could we combine with the OpenCL check above?

I see that we already have many more scattered all over in this functions. Perhaps some cleanup would be nice at some point. :)

• Also not sure whether return nullptr;

is needed?

• Could you add a reference to the Spec section too.

test/SemaOpenCL/invalid-pipes-cl2.0.cl

3 ↗

(On Diff #79226)

Variable names appear inconsistent with the rest. :)

27 ↗

(On Diff #79226)

this seems to be tested in test/SemaOpenCL/access-qualifier.cl

[Anastasia]

lib/Sema/SemaDecl.cpp

5924 ↗

(On Diff #79292)

The format is typically:
OpenCL v1.2 s6.9.r

5926 ↗

(On Diff #79292)

This line looks inconsistent with above! Put ":" after OpenCL v2.0 s6.9.q.

Also formatting seems off. At least the line width.

[Anastasia]

LGTM! Thanks! Please, address the small nitpick before committing.

lib/Sema/SemaDecl.cpp

5965 ↗

(On Diff #79372)

OpenCL 1.2 spec, p6.9 r -> OpenCL v1.2 s6.9.r

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def err_bad_kernel_param_type : Error<
   "%0 cannot be used as the type of a kernel parameter">;
 def err_record_with_pointers_kernel_param : Error<
   "%select{struct|union}0 kernel parameters may not contain pointers">;
 def note_within_field_of_type : Note<
   "within field of type %0 declared here">;
 def note_illegal_field_declared_here : Note<
   "field of illegal %select{type|pointer type}0 %1 declared here">;
-def err_event_t_global_var : Error<
-  "the event_t type cannot be used to declare a program scope variable">;
 def err_opencl_type_struct_or_union_field : Error<
   "the %0 type cannot be used to declare a structure or union field">;
 def err_event_t_addr_space_qual : Error<
   "the event_t type can only be used with __private address space qualifier">;
 def err_expected_kernel_void_return_type : Error<
   "kernel must have void return type">;
 def err_sampler_initializer_not_integer : Error<
   "sampler_t initialization requires 32-bit integer, not %0">;

 def note_related_result_type_overridden : Note<
   "overridden method returns an instance of its class type">;
 def note_related_result_type_inferred : Note<
   "%select{class|instance}0 method %1 is assumed to return an instance of "
   "its receiver type (%2)">;
 def note_related_result_type_explicit : Note<
   "%select{overridden|current}0 method is explicitly declared 'instancetype'"
   "%select{| and is expected to return an instance of its class type}0">;
+def err_invalid_type_for_program_scope_var : Error<
+  "the %0 type cannot be used to declare a program scope variable">;
 
 }
 
 let CategoryName = "Modules Issue" in {
 def err_module_interface_implementation_mismatch : Error<
   "%select{'module'|'module partition'|'module implementation'}0 declaration "
   "found while %select{not |not |}0building module interface">;
 def err_current_module_name_mismatch : Error<

cfe/trunk/lib/Sema/SemaDecl.cpp

if (!Decomp.bindings().empty()) {
       Name = II;
     }
   } else if (!II) {
     Diag(D.getIdentifierLoc(), diag::err_bad_variable_name)
       << Name;
     return nullptr;
   }
 
+  if (getLangOpts().OpenCL) {
     // OpenCL v2.0 s6.9.b - Image type can only be used as a function argument.
     // OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function
     // argument.
-  if (getLangOpts().OpenCL && (R->isImageType() || R->isPipeType())) {
+    if (R->isImageType() || R->isPipeType()) {
       Diag(D.getIdentifierLoc(),
            diag::err_opencl_type_can_only_be_used_as_function_parameter)
           << R;
       D.setInvalidType();
       return nullptr;
     }
 
-  DeclSpec::SCS SCSpec = D.getDeclSpec().getStorageClassSpec();
-  StorageClass SC = StorageClassSpecToVarDeclStorageClass(D.getDeclSpec());
-
-  // dllimport globals without explicit storage class are treated as extern. We
-  // have to change the storage class this early to get the right DeclContext.
-  if (SC == SC_None && !DC->isRecord() &&
-      hasParsedAttr(S, D, AttributeList::AT_DLLImport) &&
-      !hasParsedAttr(S, D, AttributeList::AT_DLLExport))
-    SC = SC_Extern;
-
-  DeclContext *OriginalDC = DC;
-  bool IsLocalExternDecl = SC == SC_Extern &&
-                           adjustContextForLocalExternDecl(DC);
+    // OpenCL v1.2 s6.9.r:
+    // The event type cannot be used to declare a program scope variable.
+    // OpenCL v2.0 s6.9.q:
+    // The clk_event_t and reserve_id_t types cannot be declared in program scope.
+    if (NULL == S->getParent()) {
+      if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
+        Diag(D.getIdentifierLoc(),
+             diag::err_invalid_type_for_program_scope_var) << R;
+        D.setInvalidType();
+        return nullptr;
+      }
+    }
 
-  if (getLangOpts().OpenCL) {
     // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
     QualType NR = R;
     while (NR->isPointerType()) {
       if (NR->isFunctionPointerType()) {
         Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer_variable);
         D.setInvalidType();
         break;
       }
       NR = NR->getPointeeType();
     }
 
     if (!getOpenCLOptions().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();
       }
     }
+
+    // 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)) {
+      Diag(D.getIdentifierLoc(), diag::err_wrong_sampler_addressspace);
+    }
+
+    // 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();
+      }
+    }
   }
 
+  DeclSpec::SCS SCSpec = D.getDeclSpec().getStorageClassSpec();
+  StorageClass SC = StorageClassSpecToVarDeclStorageClass(D.getDeclSpec());
+
+  // dllimport globals without explicit storage class are treated as extern. We
+  // have to change the storage class this early to get the right DeclContext.
+  if (SC == SC_None && !DC->isRecord() &&
+      hasParsedAttr(S, D, AttributeList::AT_DLLImport) &&
+      !hasParsedAttr(S, D, AttributeList::AT_DLLExport))
+    SC = SC_Extern;
+
+  DeclContext *OriginalDC = DC;
+  bool IsLocalExternDecl = SC == SC_Extern &&
+                           adjustContextForLocalExternDecl(DC);
+
   if (SCSpec == DeclSpec::SCS_mutable) {
     // mutable can only appear on non-static class members, so it's always
     // an error here
     Diag(D.getIdentifierLoc(), diag::err_mutable_nonmember);
     D.setInvalidType();
     SC = SC_None;
   }
 
if (!DC->isRecord() && S->getFnParent() == nullptr) {
     // appear in the declaration specifiers in an external declaration.
     // Global Register+Asm is a GNU extension we support.
     if (SC == SC_Auto || (SC == SC_Register && !D.getAsmLabel())) {
       Diag(D.getIdentifierLoc(), diag::err_typecheck_sclass_fscope);
       D.setInvalidType();
     }
   }
 
-  if (getLangOpts().OpenCL) {
-    // 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)) {
-      Diag(D.getIdentifierLoc(), diag::err_wrong_sampler_addressspace);
-    }
-
-    // OpenCL 1.2 spec, p6.9 r:
-    // The event type cannot be used to declare a program scope variable.
-    // The event type cannot be used with the __local, __constant and __global
-    // address space qualifiers.
-    if (R->isEventT()) {
-      if (S->getParent() == nullptr) {
-        Diag(D.getLocStart(), diag::err_event_t_global_var);
-        D.setInvalidType();
-      }
-
-      if (R.getAddressSpace()) {
-        Diag(D.getLocStart(), diag::err_event_t_addr_space_qual);
-        D.setInvalidType();
-      }
-    }
-  }
-
   bool IsExplicitSpecialization = false;
   bool IsVariableTemplateSpecialization = false;
   bool IsPartialSpecialization = false;
   bool IsVariableTemplate = false;
   VarDecl *NewVD = nullptr;
   VarTemplateDecl *NewTemplate = nullptr;
   TemplateParameterList *TemplateParams = nullptr;
   if (!getLangOpts().CPlusPlus) {

cfe/trunk/test/SemaOpenCL/event_t.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 
-event_t glb_evt; // expected-error {{the event_t type cannot be used to declare a program scope variable}}
+event_t glb_evt; // expected-error {{the 'event_t' type cannot be used to declare a program scope variable}}
 
 constant struct evt_s {
   event_t evt; // expected-error {{the 'event_t' type cannot be used to declare a structure or union field}}
 } evt_str = {0};
 
 void foo(event_t evt); // expected-note {{passing argument to parameter 'evt' here}}
 
 void kernel ker(event_t argevt) { // expected-error {{'event_t' cannot be used as the type of a kernel parameter}}
   event_t e;
   constant event_t const_evt; // expected-error {{the event_t type can only be used with __private address space qualifier}}
   foo(e);
   foo(0);
   foo(5); // expected-error {{passing 'int' to parameter of incompatible type 'event_t'}}
   foo((event_t)1); // expected-error {{cannot cast non-zero value '1' to 'event_t'}}
 }
 

cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl

+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type cannot be used to declare a program scope variable}}

cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
+global pipe int gp;            // expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
+global reserve_id_t rid;          // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
+
 void test1(pipe int *p) {// expected-error {{pipes packet types cannot be of reference type}}
 }
 void test2(pipe p) {// expected-error {{missing actual type specifier for pipe}}
 }
 void test3(int pipe p) {// expected-error {{cannot combine with previous 'int' declaration specifier}}
 }
 void test4() {
   pipe int p; // expected-error {{type 'read_only pipe int' can only be used as a function parameter}}
   //TODO: fix parsing of this pipe int (*p);
 }
 
 void test5(pipe int p) {
   p+p; // expected-error{{invalid operands to binary expression ('read_only pipe int' and 'read_only pipe int')}}
   p=p; // expected-error{{invalid operands to binary expression ('read_only pipe int' and 'read_only pipe int')}}
   &p; // expected-error{{invalid argument type 'read_only pipe int' to unary expression}}
   *p; // expected-error{{invalid argument type 'read_only pipe int' to unary expression}}
 }
 
 typedef pipe int pipe_int_t;
 pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'read_only pipe int') is not allowed}}
+
+bool test_id_comprision(void) {
+  reserve_id_t id1, id2;
+  return (id1 == id2);          // expected-error {{invalid operands to binary expression ('reserve_id_t' and 'reserve_id_t')}}
+}