[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Justas Janickas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc33e296be1da: [OpenCL] Disallows static kernel functions in 
C++ for OpenCL (authored by Topotuna).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109150/new/

https://reviews.llvm.org/D109150

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaOpenCL/storageclass-cl20.cl


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be 
declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9970,8 +9970,7 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if (SC == SC_Static) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9970,8 +9970,7 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if (SC == SC_Static) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

Cool! Thanks


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109150/new/

https://reviews.llvm.org/D109150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Justas Janickas via Phabricator via cfe-commits]

Topotuna updated this revision to Diff 370895.
Topotuna added a comment.

Condition on OpenCL C / C++ for OpenCL version removed because it was redundant.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109150/new/

https://reviews.llvm.org/D109150

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaOpenCL/storageclass-cl20.cl


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be 
declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9969,8 +9969,7 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if (SC == SC_Static) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9969,8 +9969,7 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if (SC == SC_Static) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Justas Janickas via Phabricator via cfe-commits]

Topotuna added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9972
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if ((getLangOpts().getOpenCLCompatibleVersion() >= 120) &&
+(SC == SC_Static)) {

Anastasia wrote:
> I wonder though if we should just drop a version check here completely.
> 
> I don't think we can allow this in the earlier OpenCL versions. The reason 
> why it doesn't apply to earlier versions is that `static`/`extern` wasn't 
> allowed at all so presumably frontend would reject such a case earlier.
> 
> I think the extra check is not doing anything useful here, but only adds 
> extra code to read and maintain.
That is correct. If `static` or `extern` is used in OpenCL C 1.0, diagnostic 
`err_opencl_unknown_type_specifier` is displayed before control flow reaches 
this part of code. And so removing the check here doesn't seem to change the 
compiler behaviour.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109150/new/

https://reviews.llvm.org/D109150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9972
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if ((getLangOpts().getOpenCLCompatibleVersion() >= 120) &&
+(SC == SC_Static)) {

I wonder though if we should just drop a version check here completely.

I don't think we can allow this in the earlier OpenCL versions. The reason why 
it doesn't apply to earlier versions is that `static`/`extern` wasn't allowed 
at all so presumably frontend would reject such a case earlier.

I think the extra check is not doing anything useful here, but only adds extra 
code to read and maintain.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109150/new/

https://reviews.llvm.org/D109150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109150: [OpenCL] Disallows static kernel functions in C++ for OpenCL

[Justas Janickas via Phabricator via cfe-commits]

Topotuna created this revision.
Topotuna added a reviewer: Anastasia.
Herald added subscribers: ldrumm, yaxunl.
Topotuna requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It is disallowed in OpenCL C to declare static kernel functions and
C++ for OpenCL is expected to inherit such behaviour. Error is now
correctly reported in C++ for OpenCL when declaring a static kernel
function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109150

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaOpenCL/storageclass-cl20.cl


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable 
must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be 
declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9969,8 +9969,8 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if ((getLangOpts().getOpenCLCompatibleVersion() >= 120) &&
+(SC == SC_Static)) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }


Index: clang/test/SemaOpenCL/storageclass-cl20.cl
===
--- clang/test/SemaOpenCL/storageclass-cl20.cl
+++ clang/test/SemaOpenCL/storageclass-cl20.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 int G2 = 0;
 global int G3 = 0;
@@ -18,6 +19,9 @@
 extern private float g_private_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 extern generic float g_generic_extern_var; // expected-error {{extern variable must reside in global or constant address space}}
 
+static void kernel bar() { // expected-error{{kernel functions cannot be declared static}}
+}
+
 void kernel foo() {
   constant int L1 = 0;
   local int L2;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9969,8 +9969,8 @@
 
   if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
-if ((getLangOpts().OpenCLVersion >= 120)
-&& (SC == SC_Static)) {
+if ((getLangOpts().getOpenCLCompatibleVersion() >= 120) &&
+(SC == SC_Static)) {
   Diag(D.getIdentifierLoc(), diag::err_static_kernel);
   D.setInvalidType();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits