[OpenCL] Enable program scope variables for OpenCL2.0

[Anastasia]

Summary

I would like to commit this change that enables program scope variables (PSVs) – an OpenCL2.0 feature (Section 6.5.1).

There are three major changes in this patch.

First change removes virtual SC_OpenCLWorkGroupLocal storage type specifier as it conflicts with static
local variables now and prevents diagnosing static local address space variables correctly. Also it doesn’t seem really
necessary since we can identify local AS variable by looking at the AS attribute itself.

Second change makes static local variables as well as global variables allowed in OpenCL2.0 (OpenCL2.0 s6.8 and s6.5.1).

Last change improves diagnostics of allowed ASes for variables in different scopes:

Global or static local variables have to be in global or constant ASes (OpenCL1.2 s6.5, OpenCL2.0 s6.5.1);
Non-kernel function variables can’t be declared in local or constant ASes (OpenCL1.1 s6.5.2 and s6.5.3).

[pekka.jaaskelainen]

Could only find style nitpicks in this one.

include/clang/Basic/DiagnosticSemaKinds.td

7473

variable

lib/Sema/SemaDecl.cpp

6443

There are white space issues in this patch block.

6469

Is there really no simpler way to find the FunctionDecl?

6472

Is there a reason not to use the ! operator instead of comparing to false?

[bader]

include/clang/Basic/DiagnosticSemaKinds.td

7472–7473

I think we can do better diagnostics. Original message hinted on way to fix that error - add 'constant' qualifier.

In our implementation we have 4 kinds of diagnostics (probably we need only 2 of them):
def err_opencl_global_invalid_addr_space : Error<
"program scope variables must reside in constant address space">;
def err_opencl20_global_invalid_addr_space : Error<
"program scope variables must reside in global or constant address space">;
def err_program_scope_variable_non_constant : Error<
"program scope variables are required to be declared in constant address space">;
def err_program_scope_variable_non_constant_or_global : Error<
"program scope variables are required to be declared either in constant or global address space">;

They used for OpenCL 1.2 and OpenCL 2.0 sources correspondingly.

test/SemaOpenCL/storageclass.cl

9–11

Don't you think it's better to put OpenCL 2.0 test into separate file?

[Anastasia]

Thanks! The review comments are addressed in this update!

[Anastasia]

r248906 and r250892

include/clang/Basic/DiagnosticSemaKinds.td

   "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_static_function_scope : Error<
   "variables in function scope cannot be declared static">;
 def err_opencl_bitfields : Error<

 def err_wrong_sampler_addressspace: Error<
   "sampler type cannot be used with the __local and __global address space qualifiers">;
 def err_opencl_global_invalid_addr_space : Error<
-  "global variables must have a constant address space qualifier">;
+  "program scope variables has wrong address space qualifier">;
 def err_opencl_no_main : Error<"%select{function|kernel}0 cannot be called 'main'">;
 def err_opencl_kernel_attr :
   Error<"attribute %0 can only be applied to a kernel function">;

include/clang/Basic/Specifiers.h

     SC_PrivateExtern,
 
     // These are only legal on variables.
-    SC_OpenCLWorkGroupLocal,
     SC_Auto,
     SC_Register
   };

lib/AST/Decl.cpp

   case SC_None:                 break;
   case SC_Auto:                 return "auto";
   case SC_Extern:               return "extern";
-  case SC_OpenCLWorkGroupLocal: return "<<work-group-local>>";
   case SC_PrivateExtern:        return "__private_extern__";
   case SC_Register:             return "register";
   case SC_Static:               return "static";

lib/AST/DeclPrinter.cpp

     case SC_Extern: Out << "extern "; break;
     case SC_Static: Out << "static "; break;
     case SC_PrivateExtern: Out << "__private_extern__ "; break;
-    case SC_Auto: case SC_Register: case SC_OpenCLWorkGroupLocal:
+    case SC_Auto: case SC_Register:
       llvm_unreachable("invalid for functions");
     }
 

lib/CodeGen/CGDecl.cpp

     // Don't emit it now, allow it to be emitted lazily on its first use.
     return;
 
-  if (D.getStorageClass() == SC_OpenCLWorkGroupLocal)
+  if (D.getType().getAddressSpace() == LangAS::opencl_local)
     return CGM.getOpenCLRuntime().EmitWorkGroupLocalVarDecl(*this, D);
 
   assert(D.hasLocalStorage());

lib/Sema/SemaDecl.cpp

   }
 
   if (getLangOpts().OpenCL) {
-    // Set up the special work-group-local storage class for variables in the
-    // OpenCL __local address space.
-    if (R.getAddressSpace() == LangAS::opencl_local) {
-      SC = SC_OpenCLWorkGroupLocal;
-    }
-
     // OpenCL v1.2 s6.9.b p4:
     // The sampler type cannot be used with the __local and __global address
     // space qualifiers.

         break;
       case SC_PrivateExtern:
         llvm_unreachable("C storage class in c++!");
-      case SC_OpenCLWorkGroupLocal:
-        llvm_unreachable("OpenCL storage class in c++!");
       }
     }    
 

       case SC_Static:
       case SC_Extern:
       case SC_PrivateExtern:
-      case SC_OpenCLWorkGroupLocal:
         break;
       }
     } else if (SC == SC_Register) {

   // This includes arrays of objects with address space qualifiers, but not
   // automatic variables that point to other address spaces.
   // ISO/IEC TR 18037 S5.1.2
-  if (NewVD->hasLocalStorage() && T.getAddressSpace() != 0) {
+  if (!getLangOpts().OpenCL
+      && NewVD->hasLocalStorage() && T.getAddressSpace() != 0) {
     Diag(NewVD->getLocation(), diag::err_as_qualified_auto_decl);
     NewVD->setInvalidDecl();
     return;
   }
-
-  // OpenCL v1.2 s6.5 - All program scope variables must be declared in the
-  // __constant address space.
-  if (getLangOpts().OpenCL && NewVD->isFileVarDecl()
-      && T.getAddressSpace() != LangAS::opencl_constant
-      && !T->isSamplerT()){
-    Diag(NewVD->getLocation(), diag::err_opencl_global_invalid_addr_space);
-    NewVD->setInvalidDecl();
-    return;
-  }
   
   // OpenCL v1.2 s6.8 -- The static qualifier is valid only in program
   // scope.
-  if ((getLangOpts().OpenCLVersion >= 120)
+  if ((getLangOpts().OpenCLVersion == 120)
       && NewVD->isStaticLocal()) {
     Diag(NewVD->getLocation(), diag::err_static_function_scope);
     NewVD->setInvalidDecl();
     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 (getLangOpts().OpenCL){
+    if (NewVD->isFileVarDecl()){
+      if (!T->isSamplerT()
+          && !(T.getAddressSpace() == LangAS::opencl_constant
+               || (T.getAddressSpace() == LangAS::opencl_global
+                && getLangOpts().OpenCLVersion == 200))){
+          Diag(NewVD->getLocation(), diag::err_opencl_global_invalid_addr_space);
+          NewVD->setInvalidDecl();
+          return;
+        }
+    }else{
+      // 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->isStaticLocal()
+          && T.getAddressSpace() != LangAS::opencl_global
+          && T.getAddressSpace() != LangAS::opencl_constant){
+        Diag(NewVD->getLocation(), diag::err_opencl_global_invalid_addr_space);
+        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){
+        DeclContext *EnclDC = NewVD->getDeclContext();
+        while (!isa<FunctionDecl>(EnclDC))
+          EnclDC = EnclDC->getParent();
+        FunctionDecl *FD = dyn_cast<FunctionDecl>(EnclDC);
+        if (FD && (FD->hasAttr<OpenCLKernelAttr>() == false)) {
+          if (T.getAddressSpace() == LangAS::opencl_constant)
+            Diag(NewVD->getLocation(), diag::err_opencl_non_kernel_variable)
+            << "constant";
+          else
+            Diag(NewVD->getLocation(), diag::err_opencl_non_kernel_variable)
+            << "local";
+          NewVD->setInvalidDecl();
+          return;
+        }
+      }
+    }
+  }
+
   if (NewVD->hasLocalStorage() && T.isObjCGCWeak()
       && !NewVD->hasAttr<BlocksAttr>()) {
     if (getLangOpts().getGC() != LangOptions::NonGC)

 
   // OpenCL 1.1 6.5.2: "Variables allocated in the __local address space inside
   // a kernel function cannot be initialized."
-  if (VDecl->getStorageClass() == SC_OpenCLWorkGroupLocal) {
+  if (VDecl->getType().getAddressSpace() == LangAS::opencl_local) {
     Diag(VDecl->getLocation(), diag::err_local_cant_init);
     VDecl->setInvalidDecl();
     return;

   case SC_Register:
     Error = 4;
     break;
-  case SC_OpenCLWorkGroupLocal:
-    llvm_unreachable("Unexpected storage class");
   }
   if (Error != -1) {
     Diag(VD->getOuterLocStart(), diag::err_for_range_storage_class)

test/Parser/opencl-storage-class.cl

   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;
+  static int e; // expected-error {{program scope variables has wrong address space qualifier}}
   register int f;
   extern int g;
   auto int h;

test/SemaOpenCL/storageclass.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCL20 -cl-std=CL2.0
 
 static constant int A = 0;
 
-int X = 0; // expected-error{{global variables must have a constant address space qualifier}}
+int G1 = 0;// expected-error{{program scope variables has wrong address space qualifier}}
 
-// static is not allowed at local scope.
+global int G2 = 0;
+#ifndef CL20
+// expected-error@-2{{program scope variables has wrong address space qualifier}}
+#endif
+
+local int G3 = 0;// expected-error{{program scope variables has wrong address space qualifier}}
+
+// static is not allowed at local scope before CL2.0
 void kernel foo() {
-  static int X = 5; // expected-error{{variables in function scope cannot be declared static}} 
+  static int S1 = 5;
+#ifndef CL20
+  // expected-error@-2{{variables in function scope cannot be declared static}}
+#else
+  // expected-error@-4{{program scope variables has wrong address space qualifier}}
+#endif
+
+  static global int S2 = 5;
+#ifndef CL20
+  // expected-error@-2{{variables in function scope cannot be declared static}}
+#endif
+
+  static private int S3 = 5;
+#ifndef CL20
+  // expected-error@-2{{variables in function scope cannot be declared static}}
+#else
+  // expected-error@-4{{program scope variables has wrong address space qualifier}}
+#endif
+
+  constant int G4 = 0;
+  local int L1;
+
   auto int Y = 7; // expected-error{{OpenCL does not support the 'auto' storage class specifier}}
 }
 
 static void kernel bar() { // expected-error{{kernel functions cannot be declared static}}
 }
+
+void f(){
+  constant int G5 = 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}}
+}

tools/libclang/CIndex.cpp

     return CX_SC_Static;
   case SC_PrivateExtern:
     return CX_SC_PrivateExtern;
-  case SC_OpenCLWorkGroupLocal:
-    return CX_SC_OpenCLWorkGroupLocal;
   case SC_Auto:
     return CX_SC_Auto;
   case SC_Register: