[OpenCL] Improve diagnostics of address spaces for variables inside function

[Anastasia]

Summary

This patch:

1. Prevents local variables to be declared in global AS
2. Diagnoses AS of local variables with an extern storage class as if they would be in a program scope

[mgrang]

test/SemaOpenCL/storageclass-cl20.cl

9

Pointers should align to the right. Like:

global int *GP7 = 0;

[yaxunl]

include/clang/Basic/DiagnosticSemaKinds.td

7653

is it better to use 'function scope' instead of 'non-program scope' ?

test/SemaOpenCL/storageclass-cl20.cl

1

remove -DCL20

23

'extern int G6' refers to a program-scope variable 'int G6', since the default addr space of program-scope var is global, shouldn't 'extern int G6' have global addr space by default instead of private addr space?

[Anastasia]

Fixed error msg and test!

[Anastasia]

Corrected to new error msg in tests too!

[Anastasia]

test/SemaOpenCL/storageclass-cl20.cl

23

Apparently not, because we only apply global as default address space to program (non-function) scope declarations. I am not sure what we should do here though, but this perhaps doesn't belong to this change.

Do we need clarifications with Khronos here?

[yaxunl]

LGTM. Thanks.

test/SemaOpenCL/storageclass-cl20.cl

23

Agree this may need another patch.

The spec is not clear about default addr space for function-scope extern variables. I think better to ask Khronos for clarification.

[Anastasia]

This comment was removed by Anastasia.

[Anastasia]

I am adding a small clean up here for duplicate code!

@sam, could you please re-check again. Thanks!

[yaxunl]

test/SemaOpenCL/storageclass-cl20.cl

14–16

this error msg is confusing. better say 'function scope variable with static storage must ...'

[Anastasia]

test/SemaOpenCL/storageclass-cl20.cl

24

Khronos bug: https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15598

[Anastasia]

test/SemaOpenCL/storageclass-cl20.cl

14–16

Yes, I agree. The error message is not ideal. I will think of better wording here.

[Anastasia]

Improved message text!

[Anastasia]

r262641

include/clang/Basic/DiagnosticSemaKinds.td

 def err_invalid_astype_of_different_size : Error<
   "invalid reinterpretation: sizes of %0 and %1 must match">;
 def err_static_kernel : Error<
   "kernel functions cannot be declared static">;
 def err_opencl_ptrptr_kernel_param : Error<
   "kernel parameter cannot be declared as a pointer to a pointer">;
 def err_opencl_private_ptr_kernel_param : Error<
   "kernel parameter cannot be declared as a pointer to the __private address space">;
-def err_opencl_non_kernel_variable : Error<
-  "non-kernel function variable cannot be declared in %0 address space">;
+def err_opencl_function_variable : Error<
+  "%select{non-kernel function|non-program scope}0 variable cannot be declared in %1 address space">;
 def err_static_function_scope : Error<
   "variables in function scope cannot be declared static">;
 def err_opencl_bitfields : Error<
   "bitfields are not supported in OpenCL">;
 def err_opencl_vla : Error<
   "variable length arrays are not supported in OpenCL">;
 def err_bad_kernel_param_type : Error<
   "%0 cannot be used as the type of a kernel parameter">;

lib/Sema/SemaDecl.cpp

if (T->isBlockPointerType()) {
         return;
       }
     }
     // OpenCL v1.2 s6.5 - All program scope variables must be declared in the
     // __constant address space.
     // 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.
-    if (NewVD->isFileVarDecl() || NewVD->isStaticLocal()) {
+    if (NewVD->isFileVarDecl() || NewVD->isStaticLocal() ||
+        NewVD->hasExternalStorage()) {
       // Program scope pointers can only point to data at the constant or global
        // address space.
       const PointerType *PT = nullptr;
       bool IsPointerDecl = false;
       do {
         if (!T->isSamplerT() &&
             !(T.getAddressSpace() == LangAS::opencl_constant ||
               (T.getAddressSpace() == LangAS::opencl_global &&
if (NewVD->isFileVarDecl() || NewVD->isStaticLocal() ||
         }
         PT = dyn_cast<const PointerType>(T.getTypePtr());
         if (PT) {
           T = PT->getPointeeType();
           IsPointerDecl = true;
         }
       } while (PT);
     } else {
+      if (T.getAddressSpace() == LangAS::opencl_global) {
+        Diag(NewVD->getLocation(), diag::err_opencl_function_variable)
+            << 1 /*is any function*/ << "global";
+        NewVD->setInvalidDecl();
+        return;
+      }
       // OpenCL v1.1 s6.5.2 and s6.5.3 no local or constant variables
       // in functions.
       if (T.getAddressSpace() == LangAS::opencl_constant ||
           T.getAddressSpace() == LangAS::opencl_local) {
         FunctionDecl *FD = getCurFunctionDecl();
         if (FD && !FD->hasAttr<OpenCLKernelAttr>()) {
           if (T.getAddressSpace() == LangAS::opencl_constant)
-            Diag(NewVD->getLocation(), diag::err_opencl_non_kernel_variable)
-                << "constant";
+            Diag(NewVD->getLocation(), diag::err_opencl_function_variable)
+                << 0 /*non-kernel only*/ << "constant";
           else
-            Diag(NewVD->getLocation(), diag::err_opencl_non_kernel_variable)
-                << "local";
+            Diag(NewVD->getLocation(), diag::err_opencl_function_variable)
+                << 0 /*non-kernel only*/ << "local";
           NewVD->setInvalidDecl();
           return;
         }
       }
     }
   }
 
   if (NewVD->hasLocalStorage() && T.isObjCGCWeak()

test/Parser/opencl-storage-class.cl

 // RUN: %clang_cc1 %s -verify -fsyntax-only
 
 void test_storage_class_specs()
 {
   static int a;    // expected-error {{OpenCL does not support the 'static' storage class specifier}}
   register int b;  // expected-error {{OpenCL does not support the 'register' storage class specifier}}
   extern int c;    // expected-error {{OpenCL does not support the 'extern' storage class specifier}}
   auto int d;      // expected-error {{OpenCL does not support the 'auto' storage class specifier}}
 
 #pragma OPENCL EXTENSION cl_clang_storage_class_specifiers : enable
   static int e; // expected-error {{program scope variable must reside in constant address space}}
   register int f;
-  extern int g;
+  extern int g; // expected-error {{program scope variable must reside in constant address space}}
   auto int h;
 }

test/SemaOpenCL/storageclass-cl20.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCL20 -cl-std=CL2.0
 
 static constant int G1 = 0;
 int G2 = 0;
 global int G3 = 0;
 local int G4 = 0;              // expected-error{{program scope variable must reside in global or constant address space}}
 global int **constant GP5 = 0; // expected-error{{program scope pointer must point to global or constant address space}}
 global int *global GP6 = 0;
+global int * GP7 = 0;
 
 void kernel foo() {
   static int S1 = 5;
   static global int S2 = 5;
   static private int S3 = 5;   // expected-error{{program scope variable must reside in global or constant address space}}
   static local int *global S4; // expected-error{{program scope pointer must point to global or constant address space}}
   static constant int *global S5;
 
   constant int L1 = 0;
   local int L2;
+  global int L3; // expected-error{{non-program scope variable cannot be declared in global address space}}
+
+  extern global int G5;
+  extern int G6; // expected-error{{program scope variable must reside in global or constant address space}}
 }

test/SemaOpenCL/storageclass.cl

void kernel foo() {
   // static is not allowed at local scope before CL2.0
   static int S1 = 5;          // expected-error{{variables in function scope cannot be declared static}}
   static constant int S2 = 5; // expected-error{{variables in function scope cannot be declared static}}
 
   constant int L1 = 0;
   local int L2;
 
   auto int L3 = 7; // expected-error{{OpenCL does not support the 'auto' storage class specifier}}
+  global int L4;   // expected-error{{non-program scope variable cannot be declared in global address space}}
+  global int *L5;
 }
 
 static void kernel bar() { // expected-error{{kernel functions cannot be declared static}}
 }
 
 void f() {
   constant int L1 = 0; // expected-error{{non-kernel function variable cannot be declared in constant address space}}
   local int L2;        // expected-error{{non-kernel function variable cannot be declared in local address space}}
   {
     constant int L1 = 0; // expected-error{{non-kernel function variable cannot be declared in constant address space}}
     local int L2;        // expected-error{{non-kernel function variable cannot be declared in local address space}}
   }
+  global int L3; // expected-error{{non-program scope variable cannot be declared in global address space}}
+  extern constant float L4;
 }