[OpenCL] queue_t and ndrange_t can't be defined in program scope.

[cycheng]

Summary

OpenCL v2.0 s6.5.1 and OpenCL v3.0 s6.11.w both state: Program scope variables can be defined with any valid OpenCL C data type except for those in Other Built-in Data Types.

This patch adds the required check in diagnoseOpenCLTypes.

[Anastasia]

clang/lib/Sema/SemaDecl.cpp

6844

To make things concise I would suggest dropping OpenCL v2.0 quote since it is identical to OpenCL v3.0.

6853

Suggest swapping those as string comparisons are more costly.

However, does it work if there is a typedef of ndrange_t to some other name? Although it feels like the same issue might apply to other types diagnosed here...

clang/test/SemaOpenCL/invalid-ndrange.cl

1

I imagine we won't have a lot of functionality to test in this file and invalid-queue.cl? Then could we combine them in 1 file with a more generic name like invalid_types.cl or invalid_enqueue_types.cl.

This can spare us extra clang invocations...

[cycheng]

• Remove OpenCL 2.0 description
• Merge test files into single test file

[cycheng]

previous update was using wrong file, sorry!!

[Anastasia]

clang/test/SemaOpenCL/invalid-types.cl

5 ↗

(On Diff #382816)

if you create a type alias through typedef for queue_t or ndrange_t, will you still get the diagnostic?

[Anastasia]

clang/test/SemaOpenCL/invalid-types.cl

10 ↗

(On Diff #382816)

Did you mean to use ndrange_t or queue_t here? Otherwise, it doesn't seem that your commit is related to the image types...

[cycheng]

clang/lib/Sema/SemaDecl.cpp

6853

does it work if there is a typedef of ndrange_t to some other name?

No, should we recursively check it?
Or do we have better way to handle this?

clang/test/SemaOpenCL/invalid-types.cl

5 ↗

(On Diff #382816)

Sorry for late reply!
No, I didn't catch that case, I am not sure how to handle this? Could you point out some code for reference?

10 ↗

(On Diff #382816)

Ah... sorry it's an accident. I will remove it.

[Anastasia]

clang/lib/Sema/SemaDecl.cpp

6853

We should be able to do this by using canonical type in the checks:
https://clang.llvm.org/doxygen/classclang_1_1QualType.html#a900520b154ba282febbaa7267020ca3f

clang/lib/Sema/SemaDecl.cpp

if (R->isImageType() || R->isPipeType()) {
     return false;
   }
 
   // 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.
+  // OpenCL v2.0 s6.5.1:
+  // Variables defined at program scope and static variables inside a function
+  // can also be declared in the global address space. They can be defined with
+  // any valid OpenCL C data type except for those in Other Built-in Data Types.
+  // OpenCL v3.0 s6.11.w:
+  // Program scope variables can be defined with any valid OpenCL C data type
+  // except for those in Other Built-in Data Types. 
   if (NewVD->hasGlobalStorage() && !NewVD->isStaticLocal()) {
-    if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
+    if (R->isReserveIDT() || R->isClkEventT() || R->isEventT() ||
+        R.getUnqualifiedType().getAsString() == "ndrange_t" || R->isQueueT()) {
       Se.Diag(NewVD->getLocation(),
               diag::err_invalid_type_for_program_scope_var)
           << R;
       NewVD->setInvalidDecl();
       return false;
     }
   }
 

clang/test/SemaOpenCL/invalid-ndrange.cl

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

clang/test/SemaOpenCL/invalid-queue.cl

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