[OpenCL] Allow extern constant-address-space variable in non-kernel function

[yaxunl]

Summary

OpenCL v1.2 s6.8.p2 - Extern can be used for variables declared in functions.

Differential Revision: http://reviews.llvm.org/D16865

[pxli168]

I read about spec and found that in OpenCL1.1 extern was not support.
The extern was supported in OpenCL1.2 and later in s6.8.p2. You can make the check to fit this and add the reference to comment.

[yaxunl]

Revised as Xiuli suggested.

Diagnostic for extern for OpenCL v1.1 and below already exists, so no change about that.
Added lit test for extern for OpenCL v1.0/v1.1.
Updated comments about OpenCL spec.

[Anastasia]

test/SemaOpenCL/extern-cl1.2.cl

1 ↗

(On Diff #47214)

This is CodeGen not Sema test! Please move to CodeGenOpenCL folder.

2 ↗

(On Diff #47214)

Remove this!

7 ↗

(On Diff #47214)

Do we really need this function?

You can always pass -O0 to disable generated code from disappearing!

test/SemaOpenCL/extern.cl

10

Why do you need this function?

What does it add anything to testing?

[yaxunl]

Revised as Anastasia suggested.
Moved extern test for CL1.2 from SemaOpenCL to CodeGenOpenCL.
Removed one redundant function from sema test for CL1.1.

[yaxunl]

test/SemaOpenCL/extern.cl

8

if we remove usage of foo, no code will be generated for foo even if -O0 is used. I think this is because extern foo is just declaration.

[yaxunl]

Commented at wrong location. Corrected.

test/SemaOpenCL/extern-cl1.2.cl

7 ↗

(On Diff #47214)

if we remove usage of foo, no code will be generated for foo even if -O0 is used. I think this is because extern foo is just declaration.

[Anastasia]

I just have a few small comments, but otherwise LGTM!

include/clang/Basic/DiagnosticSemaKinds.td

7699

. missing

7709

. missing

lib/Sema/SemaDecl.cpp

6616

Extern -> extern

test/SemaOpenCL/extern.cl

3

Could you please initialize the variable here to avoid the second error.

The same applies below.

8

Ok, you are right. Accepted!

[yaxunl]

Revised as Anastasia suggested.

Changed comments.
Added initializers in tests to avoid unnecessary error messages.

[pekka.jaaskelainen]

LGTM.

[yaxunl]

ping

[pxli168]

LGTM!
Thanks

[yaxunl]

this patch is superseded by D17345.

include/clang/Basic/DiagnosticSemaKinds.td

 def err_opencl_implicit_vector_conversion : Error<
   "implicit conversions between vector types (%0 and %1) are not permitted">;
 
-// OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
+// OpenCL v2.0 s6.13.6 - Builtin Pipe Functions
 def err_opencl_builtin_pipe_first_arg : Error<
   "first argument to %0 must be a pipe type">;
 def err_opencl_builtin_pipe_arg_num : Error<

 def err_opencl_builtin_pipe_invalid_access_modifier : Error<
   "invalid pipe access modifier (expecting %0)">;
 
-// OpenCL Section 6.8.g
+// OpenCL v1.1 s6.8.g - extern, static, auto, register is not supported
+// OpenCL v1.2/2.0 s6.8 -  auto, register is not supported
 def err_opencl_unknown_type_specifier : Error<
   "OpenCL does not support the '%0' %select{type qualifier|storage class specifier}1">;
 

lib/Sema/SemaDecl.cpp

         NewVD->setInvalidDecl();
         return;
       }
-      // OpenCL v1.1 s6.5.2 and s6.5.3 no local or constant variables
+      // OpenCL v1.1 s6.5.2 and s6.5.3 - No local or constant variables
       // in functions.
-      if (T.getAddressSpace() == LangAS::opencl_constant ||
+      // OpenCL v1.2 s6.8.p2 - Extern can be used for variables declared
+      // in functions.
+      if ((T.getAddressSpace() == LangAS::opencl_constant &&
+           !NewVD->isLocalExternDecl()) ||
           T.getAddressSpace() == LangAS::opencl_local) {
         FunctionDecl *FD = getCurFunctionDecl();
         if (FD && !FD->hasAttr<OpenCLKernelAttr>()) {

test/CodeGenOpenCL/extern.cl

+// RUN: %clang_cc1 -x cl -cl-std=CL1.2 -emit-llvm -ffake-address-space-map %s -o - | FileCheck %s
+
+// CHECK: @foo = external addrspace(3) constant float
+extern constant float foo;
+
+kernel void test(global float* buf) {
+  buf[0] += foo;
+}
+
+void test2(global float* buf) {
+// CHECK: @bar = external addrspace(3) constant [0 x float]
+  extern constant float bar[];
+  buf[0] += bar[0];
+}
+

test/SemaOpenCL/extern.cl

-// RUN: %clang_cc1 -x cl -cl-std=CL1.2 -emit-llvm -ffake-address-space-map %s -o - -verify | FileCheck %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -x cl -emit-llvm -ffake-address-space-map %s -o - -verify
 
-// CHECK: @foo = external addrspace(3) constant float
-extern constant float foo;
+extern constant float foo; // expected-error{{OpenCL does not support the 'extern' storage class specifier}} expected-error{{variable in constant address space must be initialized}}
 
 kernel void test(global float* buf) {
-  buf[0] += foo;
+  extern constant float bar[10]; // expected-error{{OpenCL does not support the 'extern' storage class specifier}} expected-error{{variable in constant address space must be initialized}}
 }
+