[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added a comment.

LGTM.
Please, remove "Change-Id: I910b5c077f5f29e02a1572d9202f0fdbea5280fd" from the 
log message - it not relevant to the project.


Repository:
  rC Clang

https://reviews.llvm.org/D50259



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

[PATCH] D46501: [OpenCL] Guard all half float usage based on cl_khr_fp16 extension

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Headers/opencl-c.h:12097
+#ifdef cl_khr_fp16
 float __ovld vload_half(size_t offset, const __constant half *p);
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0

These built-ins are part of the core specification, which doesn't allow using 
`half` data type directly, but user can declare a pointer to `half` data type 
and use built-ins like vload_half/vstore_half converting `half` data type to 
`float` data type.

cl_khr_fp16 enables regular uses of half data types as well as built-ins 
returning `half` data type instead of `float` - vload/vstore.


Repository:
  rC Clang

https://reviews.llvm.org/D46501



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

[PATCH] D37804: [OpenCL] Handle address space conversion while setting type alignment

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added a comment.

LGTM. Thanks!


https://reviews.llvm.org/D37804



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

[PATCH] D38463: [OpenCL] Fix checking of vector type casting

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D38463



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

[PATCH] D33681: [OpenCL] Allow function declaration with empty argument list.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 118210.
bader edited the summary of this revision.
bader added a comment.

Fix parsing of function declarations with empty parameter list initializers.
This change should fix the failure caught by the buildbot.
Sorry for the long delay.


https://reviews.llvm.org/D33681

Files:
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/function-no-args.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only 
pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global 
reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error 
{{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/function-no-args.cl
===
--- test/SemaOpenCL/function-no-args.cl
+++ test/SemaOpenCL/function-no-args.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// expected-no-diagnostics
+
+global int gi;
+int my_func();
+int my_func() {
+  gi = 2;
+  return gi;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4460,7 +4460,8 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus
+&& !LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5989,7 +5989,8 @@
 else if (RequiresArg)
   Diag(Tok, diag::err_argument_required_after_attribute);
 
-HasProto = ParamInfo.size() || getLangOpts().CPlusPlus;
+HasProto = ParamInfo.size() || getLangOpts().CPlusPlus
+|| getLangOpts().OpenCL;
 
 // If we have the closing ')', eat it.
 Tracker.consumeClose();


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/function-no-args.cl
===
--- test/SemaOpenCL/function-no-args.cl
+++ test/SemaOpenCL/function-no-args.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// expected-no-diagnostics
+
+global int gi;
+int my_func();
+int my_func() {
+  gi = 2;
+  return gi;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4460,7 +4460,8 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus
+&& !LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/

[PATCH] D33681: [OpenCL] Allow function declaration with empty argument list.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 118437.
bader added a comment.

Applied comments.


https://reviews.llvm.org/D33681

Files:
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/func.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only 
pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global 
reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error 
{{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/func.cl
===
--- test/SemaOpenCL/func.cl
+++ test/SemaOpenCL/func.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -triple 
spir-unknown-unknown
 
 // Variadic functions
 void vararg_f(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
@@ -16,6 +16,9 @@
 //Function pointer
 void foo(void*);
 
+// Expect no diagnostics for an empty parameter list.
+void bar();
+
 void bar()
 {
   // declaring a function pointer is an error
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4460,7 +4460,8 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus
+&& !LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5989,7 +5989,8 @@
 else if (RequiresArg)
   Diag(Tok, diag::err_argument_required_after_attribute);
 
-HasProto = ParamInfo.size() || getLangOpts().CPlusPlus;
+HasProto = ParamInfo.size() || getLangOpts().CPlusPlus
+|| getLangOpts().OpenCL;
 
 // If we have the closing ')', eat it.
 Tracker.consumeClose();


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/func.cl
===
--- test/SemaOpenCL/func.cl
+++ test/SemaOpenCL/func.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -triple spir-unknown-unknown
 
 // Variadic functions
 void vararg_f(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
@@ -16,6 +16,9 @@
 //Function pointer
 void foo(void*);
 
+// Expect no diagnostics for an empty parameter list.
+void bar();
+
 void bar()
 {
   // declaring a function pointer is an error
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4460,7 +4460,8 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus

[PATCH] D33681: [OpenCL] Allow function declaration with empty argument list.

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315453: [OpenCL] Allow function declaration with empty 
argument list. (authored by bader).

Changed prior to commit:
  https://reviews.llvm.org/D33681?vs=118437&id=118588#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33681

Files:
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/SemaOpenCL/func.cl
  cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl


Index: cfe/trunk/test/SemaOpenCL/func.cl
===
--- cfe/trunk/test/SemaOpenCL/func.cl
+++ cfe/trunk/test/SemaOpenCL/func.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -triple 
spir-unknown-unknown
 
 // Variadic functions
 void vararg_f(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
@@ -16,6 +16,9 @@
 //Function pointer
 void foo(void*);
 
+// Expect no diagnostics for an empty parameter list.
+void bar();
+
 void bar()
 {
   // declaring a function pointer is an error
Index: cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only 
pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global 
reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error 
{{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -4460,7 +4460,8 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus
+&& !LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {
Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -5989,7 +5989,8 @@
 else if (RequiresArg)
   Diag(Tok, diag::err_argument_required_after_attribute);
 
-HasProto = ParamInfo.size() || getLangOpts().CPlusPlus;
+HasProto = ParamInfo.size() || getLangOpts().CPlusPlus
+|| getLangOpts().OpenCL;
 
 // If we have the closing ')', eat it.
 Tracker.consumeClose();


Index: cfe/trunk/test/SemaOpenCL/func.cl
===
--- cfe/trunk/test/SemaOpenCL/func.cl
+++ cfe/trunk/test/SemaOpenCL/func.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -triple spir-unknown-unknown
 
 // Variadic functions
 void vararg_f(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
@@ -16,6 +16,9 @@
 //Function pointer
 void foo(void*);
 
+// Expect no diagnostics for an empty parameter list.
+void bar();
+
 void bar()
 {
   // declaring a function pointer is an error
Index: cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the ty

[PATCH] D38816: Convert clang::LangAS to a strongly typed enum

[Alexey Bader via Phabricator via cfe-commits]

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

@arichardson, great work! Thanks a lot!


https://reviews.llvm.org/D38816



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

[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

@Anastasia, during the discussion of similar fix 
(https://reviews.llvm.org/D34342).

I found another bug in the CodeGen library. Do you have time to fix it too?
Here is the reproducer from the old review request:

  int get_sampler_initializer(void);
  kernel void foo() {
const sampler_t const_smp_func_init = get_sampler_initializer();
  }


https://reviews.llvm.org/D39129



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

[PATCH] D33353: [OpenCL] An error shall occur if any scalar operand has greater rank than the type of the vector element

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

This issue probably can be fixed by setting SPIR target architecture or just 
enabling cl_khr_fp64 extension.
Unfortunately, I can't check it today.
Could you revert Egor's commit, please?


https://reviews.llvm.org/D33353



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

[PATCH] D33821: [OpenCL] Harden function pointer diagnostics.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.

Improve OpenCL type checking by rejecting function pointer types.


https://reviews.llvm.org/D33821

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/func.cl


Index: test/SemaOpenCL/func.cl
===
--- test/SemaOpenCL/func.cl
+++ test/SemaOpenCL/func.cl
@@ -4,8 +4,15 @@
 void vararg_f(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
 void __vararg_f(int, ...);
 typedef void (*vararg_fptr_t)(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
+// expected-error@-1{{pointers to 
functions are not allowed}}
 int printf(__constant const char *st, ...); // expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
 
+// Struct type with function pointer field
+typedef struct s
+{
+   void (*f)(struct s *self, int *i);   // expected-error{{pointers to 
functions are not allowed}}
+} s_t;
+
 //Function pointer
 void foo(void*);
 
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1881,6 +1881,11 @@
 return QualType();
   }
 
+  if (T->isFunctionType() && getLangOpts().OpenCL) {
+Diag(Loc, diag::err_opencl_function_pointer);
+return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
 return QualType();
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6160,7 +6160,7 @@
 QualType NR = R;
 while (NR->isPointerType()) {
   if (NR->isFunctionPointerType()) {
-Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer_variable);
+Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer);
 D.setInvalidType();
 break;
   }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7268,7 +7268,7 @@
   "invalid conversion between vector type %0 and integer type %1 "
   "of different size">;
 
-def err_opencl_function_pointer_variable : Error<
+def err_opencl_function_pointer : Error<
   "pointers to functions are not allowed">;
 
 def err_opencl_taking_function_address : Error<


Index: test/SemaOpenCL/func.cl
===
--- test/SemaOpenCL/func.cl
+++ test/SemaOpenCL/func.cl
@@ -4,8 +4,15 @@
 void vararg_f(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
 void __vararg_f(int, ...);
 typedef void (*vararg_fptr_t)(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
+// expected-error@-1{{pointers to functions are not allowed}}
 int printf(__constant const char *st, ...); // expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
 
+// Struct type with function pointer field
+typedef struct s
+{
+   void (*f)(struct s *self, int *i);   // expected-error{{pointers to functions are not allowed}}
+} s_t;
+
 //Function pointer
 void foo(void*);
 
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1881,6 +1881,11 @@
 return QualType();
   }
 
+  if (T->isFunctionType() && getLangOpts().OpenCL) {
+Diag(Loc, diag::err_opencl_function_pointer);
+return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
 return QualType();
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6160,7 +6160,7 @@
 QualType NR = R;
 while (NR->isPointerType()) {
   if (NR->isFunctionPointerType()) {
-Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer_variable);
+Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer);
 D.setInvalidType();
 break;
   }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7268,7 +7268,7 @@
   "invalid conversion between vector type %0 and integer type %1 "
   "of different size">;
 
-def err_opencl_function_pointer_variable : Error<
+def err_opencl_function_pointer : Error<
   "pointers to functions are not allowed">;
 
 def err_opencl_taking_function_address : Error<
___
cfe-c

[PATCH] D33821: [OpenCL] Harden function pointer diagnostics.

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304575: [OpenCL] Harden function pointer diagnostics. 
(authored by bader).

Changed prior to commit:
  https://reviews.llvm.org/D33821?vs=101181&id=101242#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33821

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/SemaOpenCL/func.cl


Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -1881,6 +1881,11 @@
 return QualType();
   }
 
+  if (T->isFunctionType() && getLangOpts().OpenCL) {
+Diag(Loc, diag::err_opencl_function_pointer);
+return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
 return QualType();
 
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -6160,7 +6160,7 @@
 QualType NR = R;
 while (NR->isPointerType()) {
   if (NR->isFunctionPointerType()) {
-Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer_variable);
+Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer);
 D.setInvalidType();
 break;
   }
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7268,7 +7268,7 @@
   "invalid conversion between vector type %0 and integer type %1 "
   "of different size">;
 
-def err_opencl_function_pointer_variable : Error<
+def err_opencl_function_pointer : Error<
   "pointers to functions are not allowed">;
 
 def err_opencl_taking_function_address : Error<
Index: cfe/trunk/test/SemaOpenCL/func.cl
===
--- cfe/trunk/test/SemaOpenCL/func.cl
+++ cfe/trunk/test/SemaOpenCL/func.cl
@@ -4,8 +4,15 @@
 void vararg_f(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
 void __vararg_f(int, ...);
 typedef void (*vararg_fptr_t)(int, ...);// expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
+// expected-error@-1{{pointers to 
functions are not allowed}}
 int printf(__constant const char *st, ...); // expected-error {{invalid 
prototype, variadic arguments are not allowed in OpenCL}}
 
+// Struct type with function pointer field
+typedef struct s
+{
+   void (*f)(struct s *self, int *i);   // expected-error{{pointers to 
functions are not allowed}}
+} s_t;
+
 //Function pointer
 void foo(void*);
 


Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -1881,6 +1881,11 @@
 return QualType();
   }
 
+  if (T->isFunctionType() && getLangOpts().OpenCL) {
+Diag(Loc, diag::err_opencl_function_pointer);
+return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
 return QualType();
 
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -6160,7 +6160,7 @@
 QualType NR = R;
 while (NR->isPointerType()) {
   if (NR->isFunctionPointerType()) {
-Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer_variable);
+Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer);
 D.setInvalidType();
 break;
   }
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7268,7 +7268,7 @@
   "invalid conversion between vector type %0 and integer type %1 "
   "of different size">;
 
-def err_opencl_function_pointer_variable : Error<
+def err_opencl_function_pointer : Error<
   "pointers to functions are not allowed">;
 
 def err_opencl_taking_function_address : Error<
Index: cfe/trunk/test/SemaOpenCL/func.cl
===
--- cfe/trunk/test/SemaOpenCL/func.cl
+++ cfe/trunk/test/SemaOpenCL/func.cl
@@ -4,8 +4,15 @@
 void vararg_f(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
 void __vararg_f(int, ...);
 typedef void (*vararg_fptr_t)(int, ...);// expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
+// expec

[PATCH] D33681: Allow function declaration with empty argument list.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 102319.
bader added a comment.

Update one more test missed in the first version of the patch.

Actually it looks like a bug in Parser.
Clang interprets following statement as variable declaration.

  extern pipe write_only int get_pipe();

So the type of the pipe element is function prototype: `int get_pipe()`.

If I understand the intention correctly, clang is expected to treat this 
expression function declaration with pipe return type, which elements are `int`.

Would it acceptable to commit this patch as is and file another bug on Parser?
I need more time to think how to resolve this ambiguity.


https://reviews.llvm.org/D33681

Files:
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/function-no-args.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only 
pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global 
reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global 
write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error 
{{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/function-no-args.cl
===
--- /dev/null
+++ test/SemaOpenCL/function-no-args.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// expected-no-diagnostics
+
+global int gi;
+int my_func();
+int my_func() {
+  gi = 2;
+  return gi;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4355,7 +4355,7 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus  && 
!LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {


Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/function-no-args.cl
===
--- /dev/null
+++ test/SemaOpenCL/function-no-args.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+// expected-no-diagnostics
+
+global int gi;
+int my_func();
+int my_func() {
+  gi = 2;
+  return gi;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4355,7 +4355,7 @@
 
   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+  if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus  && !LangOpts.OpenCL) {
 // Simple void foo(), where the incoming T is the result type.
 T = Context.getFunctionNoProtoType(T, EI);
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33989: [OpenCL] Allow targets to select address space per type

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:1041
+default:
+  return LangAS::Default;
+}

yaxunl wrote:
> I think the default (including even_t, clk_event_t, queue_t, reserved_id_t) 
> should be global since these opaque OpenCL objects are pointers to some 
> shared resources. These pointers may be an automatic variable themselves but 
> the memory they point to should be global. Since these objects are 
> dynamically allocated, assuming them in private address space implies runtime 
> needs to maintain a private memory pool for each work item and allocate 
> objects from there. Considering the huge number of work items in typical 
> OpenCL applications, it would be very inefficient to implement these objects 
> in private memory pool. On the other hand, a global memory pool seems much 
> reasonable.
> 
> Anastasia/Alexey, any comments on this? Thanks.
I remember we discussed this a couple of time in the past. 
The address space for variables of these types is not clearly stated in the 
spec, so I think the right way to treat it - it's implementation defined.
On the other hand your reasoning on using global address space as default AS 
makes sense in general - so can we put additional clarification to the spec to 
align it with the proposed implementation?



Comment at: lib/Basic/Targets.cpp:2367
 
-  LangAS::ID getOpenCLImageAddrSpace() const override {
+  virtual LangAS::ID
+  getOpenCLTypeAddrSpace(BuiltinType::Kind K) const override {

I think the rule LLVM applies is: use virtual in base classes and override w/o 
virtual in derived classes.
'virtual' is not necessary here.


https://reviews.llvm.org/D33989



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

[PATCH] D34235: [OpenCL] Fix OpenCL and SPIR version metadata generation.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.

OpenCL and SPIR version metadata must be generated once per module instead of 
once per mangled global value.


https://reviews.llvm.org/D34235

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/spir_version.cl

Index: test/CodeGenOpenCL/spir_version.cl
===
--- test/CodeGenOpenCL/spir_version.cl
+++ test/CodeGenOpenCL/spir_version.cl
@@ -10,17 +10,18 @@
 // RUN: %clang_cc1 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -cl-std=CL2.0 | FileCheck %s --check-prefix=CHECK-AMDGCN-CL20
 
 kernel void foo() {}
+kernel void bar() {}
 
-// CHECK-SPIR-CL10: !opencl.spir.version = !{[[SPIR:![0-9]+]]}
-// CHECK-SPIR-CL10: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
-// CHECK-SPIR-CL10: [[SPIR]] = !{i32 1, i32 2}
-// CHECK-SPIR-CL10: [[OCL]] = !{i32 1, i32 0}
-// CHECK-SPIR-CL12: !opencl.spir.version = !{[[VER:![0-9]+]]}
-// CHECK-SPIR-CL12: !opencl.ocl.version = !{[[VER]]}
+// CHECK-SPIR-CL10-DAG: !opencl.spir.version = !{[[SPIR:![0-9]+]]}
+// CHECK-SPIR-CL10-DAG: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
+// CHECK-SPIR-CL10-DAG: [[SPIR]] = !{i32 1, i32 2}
+// CHECK-SPIR-CL10-DAG: [[OCL]] = !{i32 1, i32 0}
+// CHECK-SPIR-CL12-DAG: !opencl.spir.version = !{[[VER:![0-9]+]]}
+// CHECK-SPIR-CL12-DAG: !opencl.ocl.version = !{[[VER]]}
 // CHECK-SPIR-CL12: [[VER]] = !{i32 1, i32 2}
 
-// CHECK-SPIR-CL20: !opencl.spir.version = !{[[VER:![0-9]+]]}
-// CHECK-SPIR-CL20: !opencl.ocl.version = !{[[VER]]}
+// CHECK-SPIR-CL20-DAG: !opencl.spir.version = !{[[VER:![0-9]+]]}
+// CHECK-SPIR-CL20-DAG: !opencl.ocl.version = !{[[VER]]}
 // CHECK-SPIR-CL20: [[VER]] = !{i32 2, i32 0}
 
 // CHECK-AMDGCN-CL10-NOT: !opencl.spir.version
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7344,8 +7344,6 @@
 };
 }
 
-static void appendOpenCLVersionMD (CodeGen::CodeGenModule &CGM);
-
 void AMDGPUTargetCodeGenInfo::setTargetAttributes(
 const Decl *D,
 llvm::GlobalValue *GV,
@@ -7402,8 +7400,6 @@
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
-
-  appendOpenCLVersionMD(M);
 }
 
 unsigned AMDGPUTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
@@ -8074,8 +8070,6 @@
 public:
   SPIRTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
 : TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
-  void emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
-CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
 };
 
@@ -8090,41 +8084,6 @@
 }
 }
 
-/// Emit SPIR specific metadata: OpenCL and SPIR version.
-void SPIRTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
- CodeGen::CodeGenModule &CGM) const {
-  llvm::LLVMContext &Ctx = CGM.getModule().getContext();
-  llvm::Type *Int32Ty = llvm::Type::getInt32Ty(Ctx);
-  llvm::Module &M = CGM.getModule();
-  // SPIR v2.0 s2.12 - The SPIR version used by the module is stored in the
-  // opencl.spir.version named metadata.
-  llvm::Metadata *SPIRVerElts[] = {
-  llvm::ConstantAsMetadata::get(
-  llvm::ConstantInt::get(Int32Ty, CGM.getLangOpts().OpenCLVersion / 100)),
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, (CGM.getLangOpts().OpenCLVersion / 100 > 1) ? 0 : 2))};
-  llvm::NamedMDNode *SPIRVerMD =
-  M.getOrInsertNamedMetadata("opencl.spir.version");
-  SPIRVerMD->addOperand(llvm::MDNode::get(Ctx, SPIRVerElts));
-  appendOpenCLVersionMD(CGM);
-}
-
-static void appendOpenCLVersionMD(CodeGen::CodeGenModule &CGM) {
-  llvm::LLVMContext &Ctx = CGM.getModule().getContext();
-  llvm::Type *Int32Ty = llvm::Type::getInt32Ty(Ctx);
-  llvm::Module &M = CGM.getModule();
-  // SPIR v2.0 s2.13 - The OpenCL version used by the module is stored in the
-  // opencl.ocl.version named metadata node.
-  llvm::Metadata *OCLVerElts[] = {
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, CGM.getLangOpts().OpenCLVersion / 100)),
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, (CGM.getLangOpts().OpenCLVersion % 100) / 10))};
-  llvm::NamedMDNode *OCLVerMD =
-  M.getOrInsertNamedMetadata("opencl.ocl.version");
-  OCLVerMD->addOperand(llvm::MDNode::get(Ctx, OCLVerElts));
-}
-
 unsigned SPIRTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
   return llvm::CallingConv::SPIR_KERNEL;
 }
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1321,6 +1321,9 @@
   /// Emits target specific Metadata for global declarations.
   void EmitTargetMetadata();
 
+  /// Emits OpenCL specific Metadata e.g. OpenCL version.
+  void EmitOpenCLMetadata();
+
   /// Emit the llvm.gcov metad

[PATCH] D34235: [OpenCL] Fix OpenCL and SPIR version metadata generation.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 102789.
bader added a comment.

Applied Akira's comment: re-used cached Int32Ty type.


https://reviews.llvm.org/D34235

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/spir_version.cl

Index: test/CodeGenOpenCL/spir_version.cl
===
--- test/CodeGenOpenCL/spir_version.cl
+++ test/CodeGenOpenCL/spir_version.cl
@@ -10,17 +10,18 @@
 // RUN: %clang_cc1 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -cl-std=CL2.0 | FileCheck %s --check-prefix=CHECK-AMDGCN-CL20
 
 kernel void foo() {}
+kernel void bar() {}
 
-// CHECK-SPIR-CL10: !opencl.spir.version = !{[[SPIR:![0-9]+]]}
-// CHECK-SPIR-CL10: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
-// CHECK-SPIR-CL10: [[SPIR]] = !{i32 1, i32 2}
-// CHECK-SPIR-CL10: [[OCL]] = !{i32 1, i32 0}
-// CHECK-SPIR-CL12: !opencl.spir.version = !{[[VER:![0-9]+]]}
-// CHECK-SPIR-CL12: !opencl.ocl.version = !{[[VER]]}
+// CHECK-SPIR-CL10-DAG: !opencl.spir.version = !{[[SPIR:![0-9]+]]}
+// CHECK-SPIR-CL10-DAG: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
+// CHECK-SPIR-CL10-DAG: [[SPIR]] = !{i32 1, i32 2}
+// CHECK-SPIR-CL10-DAG: [[OCL]] = !{i32 1, i32 0}
+// CHECK-SPIR-CL12-DAG: !opencl.spir.version = !{[[VER:![0-9]+]]}
+// CHECK-SPIR-CL12-DAG: !opencl.ocl.version = !{[[VER]]}
 // CHECK-SPIR-CL12: [[VER]] = !{i32 1, i32 2}
 
-// CHECK-SPIR-CL20: !opencl.spir.version = !{[[VER:![0-9]+]]}
-// CHECK-SPIR-CL20: !opencl.ocl.version = !{[[VER]]}
+// CHECK-SPIR-CL20-DAG: !opencl.spir.version = !{[[VER:![0-9]+]]}
+// CHECK-SPIR-CL20-DAG: !opencl.ocl.version = !{[[VER]]}
 // CHECK-SPIR-CL20: [[VER]] = !{i32 2, i32 0}
 
 // CHECK-AMDGCN-CL10-NOT: !opencl.spir.version
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7344,8 +7344,6 @@
 };
 }
 
-static void appendOpenCLVersionMD (CodeGen::CodeGenModule &CGM);
-
 void AMDGPUTargetCodeGenInfo::setTargetAttributes(
 const Decl *D,
 llvm::GlobalValue *GV,
@@ -7402,8 +7400,6 @@
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
-
-  appendOpenCLVersionMD(M);
 }
 
 unsigned AMDGPUTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
@@ -8074,8 +8070,6 @@
 public:
   SPIRTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
 : TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
-  void emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
-CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
 };
 
@@ -8090,41 +8084,6 @@
 }
 }
 
-/// Emit SPIR specific metadata: OpenCL and SPIR version.
-void SPIRTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
- CodeGen::CodeGenModule &CGM) const {
-  llvm::LLVMContext &Ctx = CGM.getModule().getContext();
-  llvm::Type *Int32Ty = llvm::Type::getInt32Ty(Ctx);
-  llvm::Module &M = CGM.getModule();
-  // SPIR v2.0 s2.12 - The SPIR version used by the module is stored in the
-  // opencl.spir.version named metadata.
-  llvm::Metadata *SPIRVerElts[] = {
-  llvm::ConstantAsMetadata::get(
-  llvm::ConstantInt::get(Int32Ty, CGM.getLangOpts().OpenCLVersion / 100)),
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, (CGM.getLangOpts().OpenCLVersion / 100 > 1) ? 0 : 2))};
-  llvm::NamedMDNode *SPIRVerMD =
-  M.getOrInsertNamedMetadata("opencl.spir.version");
-  SPIRVerMD->addOperand(llvm::MDNode::get(Ctx, SPIRVerElts));
-  appendOpenCLVersionMD(CGM);
-}
-
-static void appendOpenCLVersionMD(CodeGen::CodeGenModule &CGM) {
-  llvm::LLVMContext &Ctx = CGM.getModule().getContext();
-  llvm::Type *Int32Ty = llvm::Type::getInt32Ty(Ctx);
-  llvm::Module &M = CGM.getModule();
-  // SPIR v2.0 s2.13 - The OpenCL version used by the module is stored in the
-  // opencl.ocl.version named metadata node.
-  llvm::Metadata *OCLVerElts[] = {
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, CGM.getLangOpts().OpenCLVersion / 100)),
-  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
-  Int32Ty, (CGM.getLangOpts().OpenCLVersion % 100) / 10))};
-  llvm::NamedMDNode *OCLVerMD =
-  M.getOrInsertNamedMetadata("opencl.ocl.version");
-  OCLVerMD->addOperand(llvm::MDNode::get(Ctx, OCLVerElts));
-}
-
 unsigned SPIRTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
   return llvm::CallingConv::SPIR_KERNEL;
 }
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1321,6 +1321,9 @@
   /// Emits target specific Metadata for global declarations.
   void EmitTargetMetadata();
 
+  /// Emits OpenCL specific Metadata e.g. OpenCL version.
+  void EmitOpenCLMetadata();
+
   /// Emit the llvm.gcov metadata used to tell L

[PATCH] D33989: [OpenCL] Allow targets to select address space per type

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:1041
+default:
+  return LangAS::Default;
+}

yaxunl wrote:
> bader wrote:
> > yaxunl wrote:
> > > I think the default (including even_t, clk_event_t, queue_t, 
> > > reserved_id_t) should be global since these opaque OpenCL objects are 
> > > pointers to some shared resources. These pointers may be an automatic 
> > > variable themselves but the memory they point to should be global. Since 
> > > these objects are dynamically allocated, assuming them in private address 
> > > space implies runtime needs to maintain a private memory pool for each 
> > > work item and allocate objects from there. Considering the huge number of 
> > > work items in typical OpenCL applications, it would be very inefficient 
> > > to implement these objects in private memory pool. On the other hand, a 
> > > global memory pool seems much reasonable.
> > > 
> > > Anastasia/Alexey, any comments on this? Thanks.
> > I remember we discussed this a couple of time in the past. 
> > The address space for variables of these types is not clearly stated in the 
> > spec, so I think the right way to treat it - it's implementation defined.
> > On the other hand your reasoning on using global address space as default 
> > AS makes sense in general - so can we put additional clarification to the 
> > spec to align it with the proposed implementation?
> I think it is unnecessary to specify the implementation details in the OpenCL 
> spec.
> 
> It is also unnecessary for SPIR-V spec since the pointee address space of 
> OpenCL opaque struct is not encoded in SPIR-V.
> 
> We can use the commonly accepted address space definition in the TargetInfo 
> base class. If a target chooses to use different address space for specific 
> opaque objects, it can override it in its own virtual function.
> I think it is unnecessary to specify the implementation details in the OpenCL 
> spec.

Agree, but my point was about specifying the intention in the specification.
For instance, OpenCL spec says that image objects are located in global memory.
It says nothing about objects like events, queues, etc., so I assumed that if 
it says nothing an implementation is allowed to choose the memory region, but 
it might be false assumption.


https://reviews.llvm.org/D33989



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.

Constant samplers are handled as static variables and clang's code generation
library, which leads to llvm::unreachable. We bypass emitting sampler variable
as static since it's translated to a function call later.


https://reviews.llvm.org/D34342

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenOpenCL/sampler.cl


Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -58,4 +58,11 @@
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 5)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+
+  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  // CHECK: [[CONST_SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
+  // CHECK: store %opencl.sampler_t addrspace(2)* [[CONST_SAMP]], 
%opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR:%[a-zA-Z0-9]+]]
+  fnc4smp(const_smp);
+  // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR]]
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -161,6 +161,10 @@
   // needs to be emitted like a static variable, e.g. a function-scope
   // variable in constant address space in OpenCL.
   if (D.getStorageDuration() != SD_Automatic) {
+// Static sampler variables translated to function calls.
+if (D.getType()->isSamplerT())
+  return;
+
 llvm::GlobalValue::LinkageTypes Linkage =
 CGM.getLLVMLinkageVarDefinition(&D, /*isConstant=*/false);
 


Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -58,4 +58,11 @@
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 5)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+
+  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  // CHECK: [[CONST_SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
+  // CHECK: store %opencl.sampler_t addrspace(2)* [[CONST_SAMP]], %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR:%[a-zA-Z0-9]+]]
+  fnc4smp(const_smp);
+  // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR]]
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
 }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -161,6 +161,10 @@
   // needs to be emitted like a static variable, e.g. a function-scope
   // variable in constant address space in OpenCL.
   if (D.getStorageDuration() != SD_Automatic) {
+// Static sampler variables translated to function calls.
+if (D.getType()->isSamplerT())
+  return;
+
 llvm::GlobalValue::LinkageTypes Linkage =
 CGM.getLLVMLinkageVarDefinition(&D, /*isConstant=*/false);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33945: [OpenCL] Add support for missing sub_group functions.

[Alexey Bader via Phabricator via cfe-commits]

bader requested changes to this revision.
bader added a comment.
This revision now requires changes to proceed.

Please, split this patch into two parts:

1. Improve diagnostics on extension enabling.
2. Add missing `sub_group_*` built-in functions.




Comment at: Sema/SemaChecking.cpp:1091-1092
   case Builtin::BIwork_group_reserve_write_pipe:
+if (SemaBuiltinReserveRWPipe(*this, TheCall, false))
+  return ExprError();
+// Since return type of reserve_read/write_pipe built-in function is

I suggest leaving `SemaBuiltinReserveRWPipe` as is (i.e. two parameter) and 
modify the check for `sub_group_*` functions only:
```
if (checkOpenCLSubgroupExt(S, TheCall) || SemaBuiltinReserveRWPipe(*this, 
TheCall))
  return ExprError();
```

It think it's more readable than looking at SemaBuiltinReserveRWPipe 
declaration or leaving the comment like this:
```
if (SemaBuiltinReserveRWPipe(*this, TheCall, false /*isSubgroup*/))
```

Please, apply the same approach to `SemaBuiltinCommitRWPipe`.

In addition to that, it makes sense to set the `TheCall` type inside the 
`SemaBuiltinReserveRWPipe` to avoid code duplication.



Comment at: SemaOpenCL/cl20-device-side-enqueue.cl:219
+  buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
+  buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected 'ndrange_t' argument type}}
+}

Please, add a test case(s) on invalid block parameters to cover the checks you 
added for new `sub_group_` built-ins..



Comment at: SemaOpenCL/sub-group-bifs.cl:1
+// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL2.0
+

I don't think it makes sense to add this test. This test look like a duplicate 
of test cases added to SemaOpenCL/cl20-device-side-enqueue.cl.




Comment at: clang/Basic/Builtins.def:1402-1403
 LANGBUILTIN(get_kernel_preferred_work_group_size_multiple, "i.", "tn", 
OCLC20_LANG)
+LANGBUILTIN(get_kernel_max_sub_group_size_for_ndrange, "i.", "tn", OCLC20_LANG)
+LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "i.", "tn", OCLC20_LANG)
 

This built-in functions should return unsigned integers: "i." -> "Ui.".
Please, fix `get_kernel_work_group_size` and 
`get_kernel_preferred_work_group_size_multiple` too.



Comment at: clang/Basic/DiagnosticSemaKinds.td:8423-8424
   "illegal call to enqueue_kernel, incorrect argument types">;
 def err_opencl_enqueue_kernel_expected_type : Error<
-  "illegal call to enqueue_kernel, expected %0 argument type">;
+  "illegal call to %0, expected %1 argument type">;
 def err_opencl_enqueue_kernel_local_size_args : Error<

Since, this message is not specific to enqueue_kernel anymore, I suggest either 
rename it or better re-use existing diagnostics if possible. I think there are 
already message to report argument mismatch + probably additional note 
diagnostics that hints correct argument type.


https://reviews.llvm.org/D33945



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/CodeGenOpenCL/sampler.cl:62
+
+  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  // CHECK: [[CONST_SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)

yaxunl wrote:
> what if address of const_smp is taken and assigned to a pointer to sampler_t 
> ? Do we have diagnosis in place?
AFAIK, we have diagnostics for both:
- declaration of a pointer to sampler
- taking address of sampler variable


https://reviews.llvm.org/D34342



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

[PATCH] D34235: [OpenCL] Fix OpenCL and SPIR version metadata generation.

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/CodeGenOpenCL/spir_version.cl:13
 kernel void foo() {}
+kernel void bar() {}
 

Anastasia wrote:
> Would the original code produce duplicate version metadata here or is it just 
> for overloaded functions? Would it make sense to add `CHECK-NOT` to make sure 
> they are not generated twice?
The original code will duplicate the metadata here. Something like:
```
!opencl.ocl.version = !{!0, !0}
```
One per global value - function in case.

Existing check is already good enough as it checks exactly for one metadata:
```
// CHECK-SPIR-CL10-DAG: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
```
It was passing with buggy code, since test contains exactly one function (i.e. 
global value). Now we have two global values and fix is required to pass 
existing check. 


https://reviews.llvm.org/D34235



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Wow...
Nice catch.
For some reason I can't reproduce the problem neither.
I will drop source code change, but I'd like to modify the test anyway.
https://reviews.llvm.org/rL277024 added test/CodeGenOpenCL/sampler.cl and 
modified test/SemaOpenCL/sampler_t.cl.
I want to move the parts of the test/SemaOpenCL/sampler_t.cl, which are 
supposed to pass semantic analysis to test/CodeGenOpenCL/sampler.cl and 
validate the LLVM IR after code generation.
Are you OK with this?


https://reviews.llvm.org/D34342



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

I think I found the way to reproduce the issue. There are two pre-requisites: 
sampler must be declared in the constant address space and clang must be build 
in Debug mode.
I'll fix the test and add the test cases from test/SemaOpenCL/sampler_t.cl as 
mentioned earlier.


https://reviews.llvm.org/D34342



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 103421.
bader added a comment.
This revision is now accepted and ready to land.

Added test case reproducing the issue described in the description.
Removed test cases from test/SemaOpenCL/sampler_t.cl covered by 
test/CodeGenOpenCL/sampler.cl.

While I was moving one test case from test/SemaOpenCL/sampler_t.cl, I found 
another bug in CodeGen library that crashes the compilation if sampler is 
initialized with non-constant expression.

Here is a short reproducer:

  int get_sampler_initializer(void);
  kernel void foo() {
const sampler_t const_smp_func_init = get_sampler_initializer();
  }

The problem is that clang does not discard this code as invalid, but CodeGen 
library expects sampler initializer to be a constant expression:

  llvm::Value *
  CodeGenModule::createOpenCLIntToSamplerConversion(const Expr *E,
CodeGenFunction &CGF) {
llvm::Constant *C = EmitConstantExpr(E, E->getType(), &CGF); // for the 
reproducer expression here is CallExpr.
auto SamplerT = getOpenCLRuntime().getSamplerType();
auto FTy = llvm::FunctionType::get(SamplerT, {C->getType()}, false);
return CGF.Builder.CreateCall(CreateRuntimeFunction(FTy,
  "__translate_sampler_initializer"),
  {C});
  }

There are two ways to handle this issue:

1. Implement diagnostics allowing only compile time constant initializers.
2. Add non-constant sampler initializer support to CodeGen library.

OpenCL specification examples give me impression that samplers declared inside 
OpenCL programs must be known at compile time.
On the other hand OpenCL allows samplers passed via kernel parameters to be 
unknown at compile time.

Thoughts?


https://reviews.llvm.org/D34342

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenOpenCL/sampler.cl
  test/SemaOpenCL/sampler_t.cl


Index: test/SemaOpenCL/sampler_t.cl
===
--- test/SemaOpenCL/sampler_t.cl
+++ test/SemaOpenCL/sampler_t.cl
@@ -46,36 +46,11 @@
 
 void kernel ker(sampler_t argsmp) {
   local sampler_t smp; // expected-error{{sampler type cannot be used with the 
__local and __global address space qualifiers}}
-  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
-  const sampler_t const_smp2;
-  const sampler_t const_smp3 = const_smp;
-  const sampler_t const_smp4 = f();
   const sampler_t const_smp5 = 1.0f; // expected-error{{initializing 'const 
sampler_t' with an expression of incompatible type 'float'}}
   const sampler_t const_smp6 = 0x1LL; // expected-error{{sampler_t 
initialization requires 32-bit integer, not 'long long'}}
 
-  foo(glb_smp);
-  foo(glb_smp2);
-  foo(glb_smp3);
-  foo(glb_smp4);
-  foo(glb_smp5);
-  foo(glb_smp6);
-  foo(glb_smp7);
-  foo(glb_smp8);
-  foo(glb_smp9);
-  foo(smp);
-  foo(sampler_str.smp);
-  foo(const_smp);
-  foo(const_smp2);
-  foo(const_smp3);
-  foo(const_smp4);
-  foo(const_smp5);
-  foo(const_smp6);
-  foo(argsmp);
-  foo(5);
   foo(5.0f); // expected-error {{passing 'float' to parameter of incompatible 
type 'sampler_t'}}
-  sampler_t sa[] = {argsmp, const_smp}; // expected-error {{array of 
'sampler_t' type is invalid in OpenCL}}
-  foo(sa[0]);
-  foo(bad());
+  sampler_t sa[] = {argsmp, glb_smp}; // expected-error {{array of 'sampler_t' 
type is invalid in OpenCL}}
 }
 
 void bad(sampler_t*); // expected-error{{pointer to type 'sampler_t' is 
invalid in OpenCL}}
Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -20,6 +20,8 @@
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 // CHECK-NOT: glb_smp
 
+int get_sampler_initializer(void);
+
 void fnc4smp(sampler_t s) {}
 // CHECK: define spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* %
 
@@ -58,4 +60,20 @@
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 5)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+
+  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  fnc4smp(const_smp);
+   // CHECK: [[CONST_SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
+  // CHECK: store %opencl.sampler_t addrspace(2)* [[CONST_SAMP]], 
%opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR:%[a-zA-Z0-9]+]]
+  fnc4smp(const_smp);
+  // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR]]
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+
+  constant sampler_t constant_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+ 

[PATCH] D33681: [OpenCL] Allow function declaration with empty argument list.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Ping.

Although this patch is already accepted, I'd like to confirm that I can commit 
the latest update with changes in test/SemaOpenCL/invalid-pipes-cl2.0.cl.

Thanks.


https://reviews.llvm.org/D33681



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

[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

[Alexey Bader via Phabricator via cfe-commits]

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

Hi Sam,

Sorry for the delay. LGTM, I have only minor refactoring suggestion.

Thanks,
Alexey




Comment at: lib/CodeGen/CGBlocks.cpp:1065-1067
+  llvm::Value *FuncPtr;
 
+  if (!CGM.getLangOpts().OpenCL) {

I think it would be more readable if we merge this if statement with the if 
statement at the line #1103.
It's used to initialize FuncPtr for non-OpenCL languages and the first use of 
this variable is in the else block of if statement at the line #1103.
If I didn't miss something it should reasonable to combine this if block with 
'else' block at the line #1106.


https://reviews.llvm.org/D43783



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

[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/Sema/address_spaces.c:17
   int *_AS1 _AS2 *Z;  // expected-error {{multiple address spaces specified 
for type}}
+  int *_AS1 _AS1 *M;
 

I think it might be valuable to give a warning or remark to user. 
Using the same address space qualifier multiple times is not something OpenCL C 
developers are supposed to do.



Repository:
  rC Clang

https://reviews.llvm.org/D47630



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

[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/Sema/address_spaces.c:17
   int *_AS1 _AS2 *Z;  // expected-error {{multiple address spaces specified 
for type}}
+  int *_AS1 _AS1 *M;
 

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > bader wrote:
> > > > I think it might be valuable to give a warning or remark to user. 
> > > > Using the same address space qualifier multiple times is not something 
> > > > OpenCL C developers are supposed to do.
> > > > 
> > > The test is obviously a bit contrived, but it could happen by mistake, or 
> > > as a result of some typedef or macro combination. It also cannot go 
> > > wrong, so there's no harm in it happening.
> > > 
> > > I see your point, though. A warning feels like a bit much, so I'm not 
> > > sure what else to use. A note?
> > Just checked for const qualifier we get a warning:
> >   warning: duplicate 'const' declaration specifier 
> > [-Wduplicate-decl-specifier]
> > 
> > We could do the same... not sure if we could try to share the diagnostic as 
> > well.
> I have a patch ready that adds a warning in the same warning group and uses 
> that. I'm not sure about reusing the other one since address spaces don't 
> have to be declaration specifiers.
> 
> The warning is 'multiple identical address spaces specified for type', 
> similar to the error. Is that acceptable?
Sounds good to me. Could you share your patch, please?


Repository:
  rC Clang

https://reviews.llvm.org/D47630



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

[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC335103: [Sema] Allow creating types with multiple of the 
same addrspace. (authored by bader, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47630?vs=150683&id=152033#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47630

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaType.cpp
  test/Sema/address_spaces.c
  test/SemaOpenCL/address-spaces.cl

Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -62,4 +62,6 @@
   __private __local int *var2;  // expected-error {{multiple address spaces specified for type}}
   __local private_int_t var3;   // expected-error {{multiple address spaces specified for type}}
   __local private_int_t *var4;  // expected-error {{multiple address spaces specified for type}}
+  __private private_int_t var5; // expected-warning {{multiple identical address spaces specified for type}}
+  __private private_int_t *var6;// expected-warning {{multiple identical address spaces specified for type}}
 }
Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -14,6 +14,7 @@
 
   int _AS1 _AS2 *Y;   // expected-error {{multiple address spaces specified for type}}
   int *_AS1 _AS2 *Z;  // expected-error {{multiple address spaces specified for type}}
+  int *_AS1 _AS1 *M;  // expected-warning {{multiple identical address spaces specified for type}}
 
   _AS1 int local; // expected-error {{automatic variable qualified with an address space}}
   _AS1 int array[5];  // expected-error {{automatic variable qualified with an address space}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5758,14 +5758,6 @@
  SourceLocation AttrLoc) {
   if (!AddrSpace->isValueDependent()) { 
 
-// If this type is already address space qualified, reject it.
-// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
-// by qualifiers for two or more different address spaces."
-if (T.getAddressSpace() != LangAS::Default) {
-  Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
-  return QualType();
-}
-
 llvm::APSInt addrSpace(32);
 if (!AddrSpace->isIntegerConstantExpr(addrSpace, Context)) {
   Diag(AttrLoc, diag::err_attribute_argument_type)
@@ -5796,6 +5788,20 @@
 LangAS ASIdx =
 getLangASFromTargetAS(static_cast(addrSpace.getZExtValue()));
 
+// If this type is already address space qualified with a different
+// address space, reject it.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
+// by qualifiers for two or more different address spaces."
+if (T.getAddressSpace() != LangAS::Default) {
+  if (T.getAddressSpace() != ASIdx) {
+Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
+return QualType();
+  } else
+// Emit a warning if they are identical; it's likely unintended.
+Diag(AttrLoc,
+ diag::warn_attribute_address_multiple_identical_qualifiers);
+}
+
 return Context.getAddrSpaceQualType(T, ASIdx);
   }
 
@@ -5817,15 +5823,6 @@
 /// space for the type.
 static void HandleAddressSpaceTypeAttribute(QualType &Type,
 const AttributeList &Attr, Sema &S){
-  // If this type is already address space qualified, reject it.
-  // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
-  // qualifiers for two or more different address spaces."
-  if (Type.getAddressSpace() != LangAS::Default) {
-S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
-Attr.setInvalid();
-return;
-  }
-
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
   // qualified by an address-space qualifier."
   if (Type->isFunctionType()) {
@@ -5888,6 +5885,21 @@
   llvm_unreachable("Invalid address space");
 }
 
+// If this type is already address space qualified with a different
+// address space, reject it.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
+// qualifiers for two or more different address spaces."
+if (Type.getAddressSpace() != LangAS::Default) {
+  if (Type.getAddressSpace() != ASIdx) {
+S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
+Attr.setInvalid();
+return;
+  } else
+// Emit a warning if they are identical; it's likely unintended.
+S.Diag(Attr.getLoc(),
+   diag::warn_attribute_address_multiple_identical_qualifiers);
+}
+
 Type = S.Conte

[PATCH] D54858: [OpenCL] Improve diagnostics for address spaces in template instantiation

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Sema/TreeTransform.h:5276
 
+// Return type cann't be qualified with an address space.
+if (ResultType.getAddressSpace() != LangAS::Default) {

"cann't" - typo?
cann't - > can't or cannot.



Comment at: test/CodeGenOpenCLCXX/template-address-spaces.cl:28
   sintptrgl.foo();
   //sintgl.foo();
 }

I think it should comment should also be removed.


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

https://reviews.llvm.org/D54858



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

[PATCH] D59219: [PR41007][OpenCL] Allow printf and toolchain reserved variadic functions in C++

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: cfe/trunk/test/SemaOpenCL/extensions.cl:31
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=CL2.0 -finclude-default-header
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++ -finclude-default-header
 

Shoudn't we move this test to test/SemaOpenCLCXX?
Does `-finclude-default-header` include opencl-c.h? I think it's an overkill to 
test that OpenCL C++ allows printf. Ideally we should minimize number of times 
we parsing 11500+ lines header in LIT tests.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59219



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

[PATCH] D59219: [PR41007][OpenCL] Allow printf and toolchain reserved variadic functions in C++

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

> May be test/Driver/include-default-header.cl would make more sense to show 
> the intent?

test/Headers/opencl-c-header.cl sounds like good candidate.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59219



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

[PATCH] D59486: [OpenCL] Improve testing of default header in C++ mode

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

I see seven OpenCL C tests using -finclude-default-header option and AFAIK, 
only test/Driver/include-default-header.cl doesn't parse it. Could you also 
update OpenCL C tests?
I think clang/test/Headers/opencl-c-header.cl and 
test/Driver/include-default-header.cl fully cover this feature. 
All other tests seems to use this option to simplify the test, but with 
additional cost on parsing this header.

grep -r include-default-header clang/test/

clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir-unknown-unknown -emit-llvm -o - -finclude-default-header -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash 
-ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-MOD 
%s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir-unknown-unknown -emit-llvm -o - -finclude-default-header -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash 
-ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-MOD 
%s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header 
-fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash 
-ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 
--check-prefix=CHECK-MOD %s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header 
-fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash 
-ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 
--check-prefix=CHECK-MOD %s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir64-unknown-unknown -emit-llvm -o - -cl-std=CL1.2 -finclude-default-header 
-fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ftime-report %s 2>&1 
| FileCheck --check-prefix=CHECK --check-prefix=CHECK-MOD %s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple amdgcn--amdhsa 
-O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -ftime-report %s 2>&1 | 
FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple 
spir64-unknown-unknown -emit-llvm -o - -cl-std=CL1.2 -finclude-default-header 
-fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ftime-report %s 2>&1 
| FileCheck --check-prefix=CHECK --check-prefix=CHECK-MOD %s
clang/test/Headers/opencl-c-header.cl:// RUN: %clang_cc1 -triple amdgcn--amdhsa 
-O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -ftime-report %s 2>&1 | 
FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
clang/test/Driver/include-default-header.cl:// RUN: %clang -save-temps -x cl 
-Xclang -cl-std=CL2.0 -Xclang -finclude-default-header -emit-llvm -S -### %s
clang/test/Driver/include-default-header.cl:// CHECK-NOT: 
finclude-default-header
clang/test/Driver/include-default-header.cl:// Make sure we don't pass 
-finclude-default-header to any commands other than the driver.
clang/test/SemaOpenCL/as_type.cl:// RUN: %clang_cc1 %s -emit-llvm -triple 
spir-unknown-unknown -finclude-default-header -o - -verify -fsyntax-only
clang/test/SemaOpenCL/printf-format-string-warnings.cl:// RUN: %clang_cc1 %s 
-verify -pedantic -fsyntax-only -cl-std=CL2.0 -finclude-default-header
clang/test/SemaOpenCL/extensions.cl:// Test with -finclude-default-header, 
which includes opencl-c.h. opencl-c.h
clang/test/SemaOpenCL/extensions.cl:// RUN: %clang_cc1 %s -triple 
amdgcn-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL2.0 
-finclude-default-header
clang/test/SemaOpenCL/extensions.cl:// RUN: %clang_cc1 %s -triple 
spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=c++ 
-finclude-default-header
clang/test/CodeGenOpenCL/builtins.cl:// RUN: %clang_cc1 %s 
-finclude-default-header -cl-std=CL2.0 -O0 -emit-llvm -o - -triple 
"spir-unknown-unknown" | FileCheck %s
clang/test/CodeGenOpenCL/size_t.cl:// RUN: %clang_cc1 %s -cl-std=CL2.0 
-finclude-default-header -emit-llvm -O0 -triple spir-unknown-unknown -o - | 
FileCheck --check-prefix=SZ32 %s
clang/test/CodeGenOpenCL/size_t.cl:// RUN: %clang_cc1 %s -cl-std=CL2.0 
-finclude-default-header -emit-llvm -O0 -triple spir64-unknown-unknown -o - | 
FileCheck --check-prefix=SZ64 --check-prefix=SZ64ONLY %s
clang/test/CodeGenOpenCL/size_t.cl:// RUN: %clang_cc1 %s -cl-std=CL2.0 
-finclude-default-header -emit-llvm -O0 -triple amdgcn -o - | FileCheck 
--check-prefix=SZ64 --check-prefix=AMDGCN %s
clang/test/CodeGenOpenCL/size_t.cl:// RUN: %clang_cc1 %s -cl-std=CL2.0 
-finclude-default-header -emit-llvm -O0 -triple amdgcn---opencl -o - | 
FileCheck --check-prefix=SZ64 --check-prefix=AMDGCN %s




Comment at: test/Driver/include-default-header.cl:2
+// RUN: %clang -save-temps -x cl -Xclang -cl-std=

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D59492



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

[PATCH] D59486: [OpenCL] Improve testing of default header in C++ mode

[Alexey Bader via Phabricator via cfe-commits]

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

OpenCL C++ part looks good. Thanks!




Comment at: test/Headers/opencl-c-header.cl:57-65
 char f(char x) {
-#if __OPENCL_C_VERSION__ != CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
   return convert_char_rte(x);
 
 #else //__OPENCL_C_VERSION__
   ndrange_t t;
   return ctz(x);

This test looks strange.
It checks that convert_char_rte is compiled if OpenCL version is not 2.0, but 
AFAIK it's not deprecated in OpenCL C 2.0, so I don't know why test is written 
in this way (i.e. ifdef-else-endif).

I think checks should look like this:
```
char f(char x) {
// Check check OpenCL C 2.0 and OpenCL C++ functionality 
#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
  ndrange_t t;
  x = ctz(x);
#endif
  return convert_char_rte(x);
}
```

Probably it's better to fix separately.


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

https://reviews.llvm.org/D59486



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

[PATCH] D59544: [OpenCL] Minor improvements in default header testing

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D59544



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Shouldn't we invalidate Var declaration?



Comment at: test/CodeGenOpenCLCXX/addrspace-of-this.cl:154
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an onbject in local address space
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))

onbject  -> object



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

I guess this declaration should be disallowed for non-POD types. Can we add a 
check for that to some test/Sema* test?


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

Anastasia wrote:
> bader wrote:
> > I guess this declaration should be disallowed for non-POD types. Can we add 
> > a check for that to some test/Sema* test?
> What should be disallowed specifically? Declaring non-POD types in the local 
> address space?
Something like 

```
class C {
  int i = 0;
};

kernel void test() {
  __local C c; // error
}
```


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

https://reviews.llvm.org/D59646



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

To give more context to the question, I'd like to clarify the use case of new 
attributes.

As @Fznamznon already mentioned in previous comments SYCL is not supposed to 
expose any non-standard extensions to a user.

Here is code example of the SYCL program, which demonstrate the need for these 
attributes:

  C++
  int foo(int x) { return ++x; }
  int bar(int x) { throw std::exception("CPU code only!"); }
  …
  using namespace cl::sycl;
  queue Q;
  buffer a(range<1>{1024});
  Q.submit([&](handler& cgh) {
auto A = a.get_access(cgh);
cgh.parallel_for(range<1>{1024}, [=](id<1> index) {
  A[index] = index[0] * 2 + index[1] + foo(42);
});
  }
  ...

SYCL compiler need to compile lambda function passed to 
`cl::sycl::handler::parallel_for` method and function `foo` called from this 
lambda function.

NOTE: compiler must ignore `bar` function when we "device" part of the single 
source code.

Our current approach is to add an attribute, which SYCL runtime will use to 
mark code passed to `cl::sycl::handler::parallel_for` as "kernel functions". 
Obviously runtime library can't mark `foo` as "device" code - this is a 
compiler job: to traverse all symbols accessible from kernel functions and add 
them to the "device part" of the code.

Here is a link to the code in the SYCL runtime using `sycl_kernel` attribute: 
https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L267

I'm quite sure something similar should happen for other "single source" 
programming models like OpenMP/CUDA, except these attributes are exposed to the 
user and there is a specific requirement on attributes/pragma/keyword names.

What we are looking for is whether we should add SYCL specific code or we can 
come up with something more generic to avoid logic/code duplication with 
already existing functionality.

BTW: Mariya, I think we might need to use `sycl_device` attribute to mark 
functions, which are called from the different translation units, i.e. compiler 
can't identify it w/o user's help.
SYCL specification proposes to use special macro as "device function marker", 
but I guess we can have additional "spellings" in the clang.

NOTE2: @Anastasia, https://reviews.llvm.org/D60454 makes impossible to replace 
`sycl_kernel` attribute with `__kernel` attribute. I mean we still can enable 
it for SYCL extension, but we will need SYCL specific customization in this 
case as we apply `kernel` attribute to a template function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added inline comments.



Comment at: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp:5
+
+#if defined(EXPECT_WARNINGS)
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}

I think you can use `__SYCL_DEVICE_ONLY__` define instead.



Comment at: clang/test/SemaSYCL/device-attrubutes.cpp:8-11
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();

This duplicates clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp.
Maybe it make sense to test case when both attributes are applied to the same 
function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/test/SemaSYCL/device-attrubutes.cpp:8-11
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();

Fznamznon wrote:
> bader wrote:
> > This duplicates clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp.
> > Maybe it make sense to test case when both attributes are applied to the 
> > same function.
> From current documentation: sycl_kernel can be applied to function which can 
> be directly called by the host and will be compiled for device, sycl_device 
> applies to device functions which **cannot** be directly called by the 
> host... 
> I think if we want add this test case we need clarify how this case will be 
> handled by the compiler.
> From current documentation: sycl_kernel can be applied to function which can 
> be directly called by the host and will be compiled for device, sycl_device 
> applies to device functions which cannot be directly called by the host... 
> I think if we want add this test case we need clarify how this case will be 
> handled by the compiler.

I would expect `sycl_kernel` to supersede `sycl_device` attribute. Basically 
`sycl_kernel` include functionality of `sycl_device` attribute and provides 
additional host accessibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added a comment.

LGTM. One minor comment.




Comment at: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp:3
+// Now pretend that we're compiling a C++ file. There should be warnings.
+// RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c++ %s
+

No need to pass "EXPECT_WARNINGS" define.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1468386 , @Fznamznon wrote:

> > Ok, my question is whether you are planning to duplicate the same logic as 
> > for OpenCL kernel which doesn't really seem like an ideal design choice. Is 
> > this the only difference then we can simply add an extra check for SYCL 
> > compilation mode in this template handling case. The overall interaction 
> > between OpenCL and SYCL implementation is still a very big unknown to me so 
> > it's not very easy to judge about the implementations details...
>
> Of course, if nothing prevents us to re-use OpenCL kernel attribute for SYCL 
> I assume it would be good idea. 
>  But I'm thinking about the situation with https://reviews.llvm.org/D60454 . 
> If we re-use OpenCL kernel attributes - we affected by OpenCL-related changes 
> and OpenCL-related changes shouldn't violate SYCL semantics. Will it be 
> usable for SYCL/OpenCL clang developers? @bader , what do you think about it?


I also think it's worth trying. We should be able to cover "SYCL semantics" 
with LIT test to avoid regressions by OpenCL related changes. E.g. add a test 
case checking that -fsycl-is-device option disables restriction on applying 
`__kernel` to template functions.
I want to confirm that everyone is okay to enable `__kernel` keyword for SYCL 
extension and cover SYCL use cases with additional regression tests. IIRC, on 
yesterday call, @keryell, said that having SYCL specific attributes useful for 
separation of concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1467279 , @aaron.ballman 
wrote:

> In D60455#1464324 , @Fznamznon wrote:
>
> > Applied comments from @aaron.ballman and @keryell
> >
> > - Introduced a C++11 and C2x style spelling in the clang namespace. I 
> > didn't find path to add two namespaces to attribute (like 
> > [[clang::sycl::device]]) so [[clang::sycl_device]] spelling is added.
> > - Since both attributes have spellings and possible can be used as some 
> > "standard" outlining in Clang/LLVM I added documetation.
> > - Added more test cases.
> > - As @bader mentioned sycl_device can be used to mark functions, which are 
> > called from the different translation units so I added simple handling of 
> > this attribute in Sema.
>
>
> I'm confused -- I thought @bader also said "...SYCL is not supposed to expose 
> any non-standard extensions to a user." -- these attributes are not standards 
> based (WG21 and WG14 have nothing to say about them), so are these attributes 
> considered "non-standard extensions" or not?


@aaron.ballman sorry for confusion.
SYCL specification doesn't require user to annotate "device functions" with an 
attribute - it says following (from section 6.9.1 SYCL functions and methods 
linkage, https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf, page 251):

> The default behavior in SYCL applications is that all the definitions and 
> declarations of the functions and methods
>  are available to the SYCL compiler, in the same translation unit. When this 
> is not the case, all the symbols that
>  need to be exported to a SYCL library or from a C++ library to a SYCL 
> application need to be defined using the
>  macro: SYCL_EXTERNAL.
>  The SYCL_EXTERNAL macro will only be defined if the implementation supports 
> offline linking. The macro is
>  implementation-defined, but the following restrictions apply:
> 
> • SYCL_EXTERNAL can only be used on functions;
>  • the function cannot use raw pointers as parameter or return types. 
> Explicit pointer classes must be used instead;
>  • externally defined functions cannot call a 
> cl::sycl::parallel_for_work_item method;
>  • externally defined functions cannot be called from a 
> cl::sycl::parallel_for_work_group scope.
> 
> The SYCL linkage mechanism is optional and implementation defined.

The idea I had is that to define `SYCL_EXTERNAL` macro as `sycl_device` 
attribute.

BTW, I noticed that `SYCL_EXTERNAL` puts additional requirements `sycl_device` 
doesn't meet:

> • SYCL_EXTERNAL can only be used on functions;

I think our implementation doesn't have such limitations and able to support 
more use cases.
Anyway, we can make `sycl_device` attribute implicit for now and return to the 
implementation of cross translation unit dependencies later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
bader added reviewers: keryell, Naghasan.
Herald added subscribers: cfe-commits, ebevhan.
Herald added a project: clang.

-fsycl-is-device enables compilation of the device part of SYCL source
file.

Patch by Mariya Podchishchaeva 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57768

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1056,6 +1056,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2875,6 +2875,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3150,6 +3150,9 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// SYCL options
+def sycl_device_only : Flag<["--"], "sycl-device-only">,
+  HelpText<"Compile SYCL code for device only">;
 
 include "CC1Options.td"
 
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -836,8 +836,14 @@
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===--===//
+// SYCL Options
+//===--===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 
//===--===//
 // cc1as-only Options
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -215,6 +215,8 @@
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate 
transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1056,6 +1056,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2875,6 +2875,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386754 , @Naghasan wrote:

> LGTM
>
> Side note: might be good to also involve @Anastasia, as some of the future 
> patches will overlap with OpenCL.


Sure. I'll add @Anastasia as a reviewer to the relevant patches.

Thanks,
Alexey


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386924 , @ABataev wrote:

> This definitely requires a test.


@ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
`-fopenmp-is-device` options, but I wasn't able to find a dedicated test. Could 
you suggest some examples testing similar functionality, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386941 , @ABataev wrote:

> In D57768#1386933 , @bader wrote:
>
> > In D57768#1386924 , @ABataev wrote:
> >
> > > This definitely requires a test.
> >
> >
> > @ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
> > `-fopenmp-is-device` options, but I wasn't able to find a dedicated test. 
> > Could you suggest some examples testing similar functionality, please?
>
>
> There are several similar tests:
>  OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There 
> is no absolutely the same test for OpenMP, since OpenMP has mo similar req 
> for the offloading.


@ABataev thanks for the pointers. The uploaded patch adds two options:

- fsycl-is-device (front-end option)
- sycl-device-only (driver option)

The driver tests you mention validate driver logic enabled by new options, 
which is not part of this test and I was going to add it later.
I can split the patch and remove new driver option and leave only front-end 
option.
Another option is to add driver logic that invokes the front-end compiler in 
"device only" mode.
Which option do you prefer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D58277: [OpenCL] Change type of block pointer for OpenCL

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354337: [OpenCL] Change type of block pointer for OpenCL 
(authored by bader, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58277?vs=186988&id=187363#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58277

Files:
  cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
  cfe/trunk/test/CodeGenOpenCL/blocks.cl
  cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
@@ -635,7 +635,9 @@
 
   case Type::BlockPointer: {
 const QualType FTy = cast(Ty)->getPointeeType();
-llvm::Type *PointeeType = ConvertTypeForMem(FTy);
+llvm::Type *PointeeType = CGM.getLangOpts().OpenCL
+  ? CGM.getGenericBlockLiteralType()
+  : ConvertTypeForMem(FTy);
 unsigned AS = Context.getTargetAddressSpace(FTy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
Index: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -11,7 +11,7 @@
 
 // For a block global variable, first emit the block literal as a global variable, then emit the block variable itself.
 // COMMON: [[BL_GLOBAL:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INV_G:@[^ ]+]] to i8*) to i8 addrspace(4)*) }
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: @block_G = addrspace(1) constant %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*)
 
 // For anonymous blocks without captures, emit block literals as global variable.
 // COMMON: [[BLG1:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* {{@[^ ]+}} to i8*) to i8 addrspace(4)*) }
@@ -77,9 +77,9 @@
   // COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
   // COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // COMMON: store i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* [[INVL1:@__device_side_enqueue_block_invoke[^ ]*]] to i8*) to i8 addrspace(4)*), i8 addrspace(4)** %block.invoke
-  // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to void ()*
-  // B64: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 }>* %block to void ()*
-  // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast void ()* [[BL]] to i8 addrspace(4)*
+  // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to %struct.__opencl_block_literal_generic*
+  // B64: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 }>* %block to %struct.__opencl_block_literal_generic*
+  // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast %struct.__opencl_block_literal_generic* [[BL]] to i8 addrspace(4)*
   // COMMON-LABEL: call i32 @__enqueue_kernel_basic(
   // COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* byval [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVLK1:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
@@ -95,8 +95,8 @@
   // COMMON: [[WAIT_EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %event_wait_list to %opencl.clk_event_t{{.*}}* addrspace(4)*
   // COMMON: [[EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %clk_event to %opencl.clk_event_t{{.*}}* addrspace(4)*
   // COMMON: store i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* [[INVL2:@__device_side_enqueue_block_invoke[^ ]*]] to i8*) to i8 addrspace(4)*), i8 addrspace(4)** %block.invoke
-  // COMMON: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 187751.
bader added a comment.

Applied comments from @ABataev.

- Split changes into two patches. This part contains front-end option enabling 
device specific macro. Changes adding driver option will be sent in a separate 
patch.
- Added LIT test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/sycl-macro.cpp


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck %s
+
+// CHECK:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) 
\
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -836,8 +836,14 @@
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===--===//
+// SYCL Options
+//===--===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 
//===--===//
 // cc1as-only Options
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -217,6 +217,8 @@
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate 
transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck %s
+
+// CHECK:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX 

[PATCH] D57768: [SYCL] Add clang front-end option to enable SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 187785.
bader added a comment.

Add check that SYCL specific macro is not enabled by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/sycl-macro.cpp


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -E -dM | FileCheck %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck 
--check-prefix=CHECK-SYCL %s
+
+// CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) 
\
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+
   // Set CUDA mode for OpenMP target NVPTX if specified in options
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && T.isNVPTX() &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -836,8 +836,14 @@
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
 
-} // let Flags = [CC1Option]
+//===--===//
+// SYCL Options
+//===--===//
 
+def fsycl_is_device : Flag<["-"], "fsycl-is-device">,
+  HelpText<"Generate code for SYCL device.">;
+
+} // let Flags = [CC1Option]
 
 
//===--===//
 // cc1as-only Options
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -217,6 +217,8 @@
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate 
transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
+LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")


Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -E -dM | FileCheck %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+
+// CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1057,6 +1057,12 @@
 Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
   }
 
+  // Define indicating that the source file is being compiled with a SYCL
+  // device compiler which doesn't produce host binary.
+  if (LangOpts.SYCLIsDevice) {
+Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
+  }
+
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
 #define OPENCLEXT(Ext) \
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2879,6 +2879,8 @@
   << Opts.OMPHostIRFile;
   }
 
+  Opts.SYCLIsDevice = A

[PATCH] D57768: [SYCL] Add clang front-end option to enable SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader closed this revision.
bader added a comment.

Committed: https://reviews.llvm.org/rL354773


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D36410: [OpenCL] Handle taking address of block captures

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In https://reviews.llvm.org/D36410#855282, @Anastasia wrote:

> Ok, I will update it to be implicitly generic then. Just to be sure, @bader 
> do you agree on this too?




> An alternative approached could be (in case we want to allow this code) to 
> **assume captures are located in the generic AS implicitly**. However, in 
> this case programmers should be advised that erroneous AS casts can occur 
> further that can't be diagnosed by the frontend (i.e. if capture address is 
> used further in the operations of a different address space to where the 
> captures physical location is).

I don't have a strong opinion on this topic, but I'm worried about relying on 
implicit assumptions, which might make code less portable.

How the alternative approach is this suppose to work for enqueue_kernel? Do we 
assume that the captured variables passed by generic pointer and compiler will 
assign the correct address space based on the explicit casts in the block?
There are some restrictions on kernel parameters: 6.9.a. Arguments to kernel 
functions declared in a program that are pointers must be declared with the 
global, constant or local qualifier.

> I feel forbidding user taking address of captured variable is too restrictive 
> and make blocks less useful.

I have no information how widely blocks are used by the OpenCL community and 
how important this feature is.


https://reviews.llvm.org/D36410



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

[PATCH] D36410: [OpenCL] Handle taking address of block captures

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In https://reviews.llvm.org/D36410#856716, @yaxunl wrote:

> The captured variable is still passed by value. The address taking is on the 
> duplicate of the captured variable, not on the original variable.


In this case address of captured variables should point to the private address 
space, as function parameters reside in private address space (see "6.5 Address 
Space Qualifiers" paragraph 3).
This makes unclear to me how capturing works for variables declared in the 
address spaces other than private (e.g. `local int a;`).

> The OpenCL spec does not forbid taking address of captured variable. 
> Forbidding taking address of captured variable would violate the spec.

I'm not sure yet, but it might be OpenCL spec inconsistency issue.


https://reviews.llvm.org/D36410



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

[PATCH] D37804: [OpenCL] Handle address space conversion while setting type alignment

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/CodeGenOpenCL/vectorLoadStore.cl:7
+typedef float float4 __attribute((ext_vector_type(4)));
+;
 

Can we remove this line?



Comment at: test/CodeGenOpenCL/vectorLoadStore.cl:15
+
+// CHECK: define spir_func void @alignment()
+void alignment() {

Please, set SPIR target in clang options to make this check reliable.


https://reviews.llvm.org/D37804



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/test/SemaSYCL/device-attributes.cpp:3
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' 
attribute only applies to functions}}
+__attribute((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' 
attribute only applies to functions}}

aaron.ballman wrote:
> Fznamznon wrote:
> > aaron.ballman wrote:
> > > I'd like to see some more tests covering less obvious scenarios. Can I 
> > > add this attribute to a lambda? What about a member function? How does it 
> > > work with virtual functions? That sort of thing.
> > Actually there is no restrictions for adding this attribute to any function 
> > to outline device code so I just checked the simplest variant.
> > 
> > But I'm working on new patch which will put some requirements on function 
> > which is marked with `sycl_kernel` attribute. 
> > This new patch will add generation of OpenCL kernel from function marked 
> > with `sycl_kernel` attribute. The main idea of this approach is described 
> > in this [[ 
> > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md#lowering-of-lambda-function-objects-and-named-function-objects
> >  | document ]] (in this document generated kernel is called "kernel 
> > wrapper").
> > And to be able to generate OpenCL kernel using function marked with 
> > `sycl_kernel` attribute we put some requirements on this function, for 
> > example it must be a template function. You can find these requirements and 
> > example of proper function which can be marked with `sycl_kernel` in this 
> > [[ https://github.com/intel/llvm/pull/177#discussion_r290451286 | comment 
> > ]] .
> > 
> > 
> > Actually there is no restrictions for adding this attribute to any function 
> > to outline device code so I just checked the simplest variant.
> 
> So there are no concerns about code like:
> ```
> struct Base {
>   __attribute__((sycl_kernel)) virtual void foo();
>   virtual void bar();
> };
> 
> struct Derived : Base {
>   void foo() override;
>   __attribute__((sycl_kernel)) void bar() override;
> };
> 
> void f(Base *B, Derived *D) {
>   // Will all of these "do the right thing"?
>   B->foo();
>   B->bar();
> 
>   D->foo();
>   D->bar();
> }
> ```
> Actually there is no restrictions for adding this attribute to any function 
> to outline device code so I just checked the simplest variant.
> But I'm working on new patch which will put some requirements on function 
> which is marked with sycl_kernel attribute.

@aaron.ballman, sorry for confusing. The  usage scenarios should have been 
articulated more accurately.
We have only four uses of this attribute in our implementation:
https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L538 
(lines 538-605).
All four uses are applied to member functions of `cl::sycl::handler` class and 
all of them have similar prototype (which is mentioned by Mariya in the 
previous 
[comment](https://github.com/intel/llvm/pull/177#discussion_r290451286):

```
namespace cl { namespace sycl {
class handler {
  template 
  __attribute__((sycl_kernel)) void sycl_kernel_function(KernelType 
KernelFuncObj) {
KernelFuncObj();
  }
};
}}
```

Here is the list of SYCL device compiler expectations with regard to the 
function marked with `sycl_kernel` attribute.
- Function template with at least one parameter is expected. The 
compiler generates OpenCL kernel and uses first template parameter as unique 
name to the generated OpenCL kernel. Host application uses this unique name to 
invoke the OpenCL kernel generated for the `sycl_kernel_function` specialized 
by this name and KernelType (which might be a lambda type).
- Function must have at least one parameter. First parameter expected 
to be a function object type (named or unnamed i.e. lambda). Compiler uses 
function object type field to generate OpenCL kernel parameters.

Aaron, I hope it makes more sense now.

We don't plan in any use cases other than in SYCL standard library 
implementation mentioned above.
If I understand you concerns correctly, you want to be sure that clang 
prohibits other uses of this attribute, which are not intended. Right?
What is the best way to do this? Add more negative tests cases and make sure 
that clang generate error diagnostic messages?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1017
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Anastasia wrote:
> Ok, I thought the earlier request was not to add undocumented attributes with 
> the spelling?
> 
> Also did `__kernel` attribute not work at the end?
> 
> I can't quite get where the current disconnect comes from but I find it 
> extremely unhelpful.
Hi @Anastasia, let me try to help.

> Ok, I thought the earlier request was not to add undocumented attributes with 
> the spelling?

Right. @Fznamznon, could you document `sycl_kernel` attribute, please?

> Also did __kernel attribute not work at the end?

Maria left a comment with the summary of our experiment: 
https://reviews.llvm.org/D60455#1472705. There is a link to pull request, where 
@keryell and @agozillon expressed preference to have separate SYCL attributes. 
Let me copy their feedback here:

@keryell :

> Thank you for the experiment.
> That looks like a straight forward change.
> The interesting part is that it does not expose any advantage from reusing 
> OpenCL __kernel marker So I am not more convinced that it is the way to 
> go, because we would use any other keyword or attribute and it would be the 
> same...
> 

@agozillon :

> Just my two cents, I think a separation of concerns and having separate 
> attributes will simplify things long-term.
> 
> While possibly similar just now, the SYCL specification is evolving and may 
> end up targeting more than just OpenCL. So the semantics of the attributes 
> may end up being quite different, even if at the moment the SYCL attribute is 
> there mostly just to mark something for outlining.
> 
> If it doesn't then the case for refactoring and merging them in a future 
> patch could be brought up again.

To summarize: we don't have good arguments to justify re-use of OpenCL 
`__kernel` keyword for SYCL mode requested by @aaron.ballman here 
https://reviews.llvm.org/D60455#1469150.

> I can't quite get where the current disconnect comes from but I find it 
> extremely unhelpful.

Let me know how I can help here.

Additional note. I've submitted initial version of SYCL compiler design 
document to the GItHub: 
https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md.
 Please, take a look and let me know if you have questions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1513499 , @Anastasia wrote:

> //**Design question**//: since you are not aware what functions are to be run 
> on a device while parsing them, at what point do you plan to diagnose the 
> invalid behavior from the standard C++ i.e. using function pointers in kernel 
> code is likely to cause issues?


We are going to use DeviceDiagBuilder and related infrastructure implemented in 
Clang to diagnose device side code errors in OpenMP/CUDA modes.
More details are in the comments here:
https://clang.llvm.org/doxygen/classclang_1_1Sema_1_1DeviceDiagBuilder.html#details

> Also do you need to outline the data structures too? For example classes used 
> on device are not allowed to have virtual function.

Yes. This restriction is already implemented in our code base on GitHub.




Comment at: clang/include/clang/Basic/Attr.td:1017
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Anastasia wrote:
> Anastasia wrote:
> > Fznamznon wrote:
> > > bader wrote:
> > > > Anastasia wrote:
> > > > > Ok, I thought the earlier request was not to add undocumented 
> > > > > attributes with the spelling?
> > > > > 
> > > > > Also did `__kernel` attribute not work at the end?
> > > > > 
> > > > > I can't quite get where the current disconnect comes from but I find 
> > > > > it extremely unhelpful.
> > > > Hi @Anastasia, let me try to help.
> > > > 
> > > > > Ok, I thought the earlier request was not to add undocumented 
> > > > > attributes with the spelling?
> > > > 
> > > > Right. @Fznamznon, could you document `sycl_kernel` attribute, please?
> > > > 
> > > > > Also did __kernel attribute not work at the end?
> > > > 
> > > > Maria left a comment with the summary of our experiment: 
> > > > https://reviews.llvm.org/D60455#1472705. There is a link to pull 
> > > > request, where @keryell and @agozillon expressed preference to have 
> > > > separate SYCL attributes. Let me copy their feedback here:
> > > > 
> > > > @keryell :
> > > > 
> > > > > Thank you for the experiment.
> > > > > That looks like a straight forward change.
> > > > > The interesting part is that it does not expose any advantage from 
> > > > > reusing OpenCL __kernel marker So I am not more convinced that it 
> > > > > is the way to go, because we would use any other keyword or attribute 
> > > > > and it would be the same...
> > > > > 
> > > > 
> > > > @agozillon :
> > > > 
> > > > > Just my two cents, I think a separation of concerns and having 
> > > > > separate attributes will simplify things long-term.
> > > > > 
> > > > > While possibly similar just now, the SYCL specification is evolving 
> > > > > and may end up targeting more than just OpenCL. So the semantics of 
> > > > > the attributes may end up being quite different, even if at the 
> > > > > moment the SYCL attribute is there mostly just to mark something for 
> > > > > outlining.
> > > > > 
> > > > > If it doesn't then the case for refactoring and merging them in a 
> > > > > future patch could be brought up again.
> > > > 
> > > > To summarize: we don't have good arguments to justify re-use of OpenCL 
> > > > `__kernel` keyword for SYCL mode requested by @aaron.ballman here 
> > > > https://reviews.llvm.org/D60455#1469150.
> > > > 
> > > > > I can't quite get where the current disconnect comes from but I find 
> > > > > it extremely unhelpful.
> > > > 
> > > > Let me know how I can help here.
> > > > 
> > > > Additional note. I've submitted initial version of SYCL compiler design 
> > > > document to the GItHub: 
> > > > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md.
> > > >  Please, take a look and let me know if you have questions.
> > > >> Ok, I thought the earlier request was not to add undocumented 
> > > >> attributes with the spelling?
> > > > Right. @Fznamznon, could you document sycl_kernel attribute, please?
> > > 
> > > Do we really need add documentation for attribute which is not presented 
> > > in SYCL spec and used for internal implementation details only because it 
> > > has spelling?
> > > 
> > > Ok, I thought the earlier request was not to add undocumented 
> > > attributes with the spelling?
> > > 
> > > Right. @Fznamznon, could you document sycl_kernel attribute, please?
> > > 
> > > Do we really need add documentation for attribute which is not presented 
> > > in SYCL spec and used for internal implementation details only because it 
> > > has spelling?
> > 
> > 
> > 
> > You are adding an attribute that is exposed to the programmers that use 
> > clang to compile their code, so unless you come up with some way to reject 
> > it in the non-toolchain mode it has to be documented. And for clang it will 
> > become "hidden" SYCL dialect so absolutely not different to `__kernel`.
> > 
> > Another aspect to consider is that clang used `TypePrinter` in diagnostics 
> > and even though printing 

[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:3059
/*IndexTypeQuals*/ 0);
   SL = StringLiteral::Create(Context, Str, StringLiteral::Ascii,
  /*Pascal*/ false, ResTy, Loc);

Will it work if we fix this issue inside StringLiteral::Create method?
I just hope it will help us avoid code duplication.


https://reviews.llvm.org/D46049



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

[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Sema/SemaExpr.cpp:3056
+  if (LangOpts.OpenCL)
+ResTy = Context.getAddrSpaceQualType(ResTy, LangAS::opencl_constant);
   ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,

Do we still need this?



https://reviews.llvm.org/D46049



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

[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/AST/Expr.cpp:870
+  if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default)
+Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant);
+

As `Ty` is passed by value, shouldn't we accept only data located in constant 
address space?



Comment at: lib/Sema/SemaExpr.cpp:3057
+ResTy = Context.getAddrSpaceQualType(ResTy, LangAS::opencl_constant);
   ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,
/*IndexTypeQuals*/ 0);

String type can only be a constant arrays.
Can we set constant address space inside this getter for OpenCL language?
Or we might want constant array in other address spaces e.g. private? 


https://reviews.llvm.org/D46049



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

[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/AST/Expr.cpp:870
+  if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default)
+Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant);
+

Anastasia wrote:
> bader wrote:
> > As `Ty` is passed by value, shouldn't we accept only data located in 
> > constant address space?
> Do you mean to assert? Currently it should be passed with `constant` AS but I 
> thought the idea is to modify this function so we can accept `Default` AS too 
> but then replace by `constant`.
> 
Yes, but according to your other comment this idea doesn't work.

> I have added the address space to the creation of StringLiteral, but 
> unfortunately it doesn't seems like we can remove similar code in other 
> places because **QualType created for StringLiteral is also used elsewhere 
> and has to match (contain right address space).** I.e. here is it used 
> further down to create PredefinedExpr.






https://reviews.llvm.org/D46049



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
bader added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, ebevhan, yaxunl.
Herald added a project: clang.
bader marked an inline comment as done.
bader added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10090
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+  isLHSPointer && isRHSPointer) {

Alternative approach would be to remove `S.getLangOpts().OpenCL` to enable this 
check for all modes.

@Anastasia, do you know if it's safe to do?


This change enables OpenCL diagnostics for the pointers annotated with
address space attribute SYCL mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80317

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaSYCL/address-space-arithmetic.cpp


Index: clang/test/SemaSYCL/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type 
 ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,7 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+  isLHSPointer && isRHSPointer) {
 const PointerType *lhsPtr = LHSExpr->getType()->castAs();
 const PointerType *rhsPtr = RHSExpr->getType()->castAs();
 if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {


Index: clang/test/SemaSYCL/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,7 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+  isLHSPointer && isRHSPointer) {
 const PointerType *lhsPtr = LHSExpr->getType()->castAs();
 const PointerType *rhsPtr = RHSExpr->getType()->castAs();
 if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader marked an inline comment as done.
bader added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10090
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+  isLHSPointer && isRHSPointer) {

Alternative approach would be to remove `S.getLangOpts().OpenCL` to enable this 
check for all modes.

@Anastasia, do you know if it's safe to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265487.
bader added a comment.

Enable diagnostics for non-OpenCL modes and applied refactoring proposed by 
John.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address-space-arithmetic.cpp


Index: clang/test/Sema/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/Sema/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type 
 ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,15 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();
+Qualifiers TQ = T.getQualifiers();
+// Address spaces overlap if at least one of them is a superset of another
+return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
+  }
+
   /// Returns gc attribute of this type.
   inline Qualifiers::GC getObjCGCAttr() const;
 


Index: clang/test/Sema/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/Sema/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,15 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();
+Qualifiers TQ = T.getQualifiers();
+// Address spaces overlap if at least one of them is a superset of another
+return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
+  }
+
   /// Returns gc attribute of this type.
   inline Qualifiers::GC getObjCGCAttr() const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader marked 2 inline comments as done.
bader added a comment.

Thanks for the review.
I've enabled the diagnostics for all the modes.
I also applied the refactoring suggested by @rjmccall. Hopefully I understand 
it correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265547.
bader added a comment.

- Added C test case with address_space attribute.
- Move isAddressSpaceOverlapping from PointerType to QualType.
- Move C++ test case to clang/test/SemaCXX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_spaces.c
  clang/test/SemaCXX/address-space-arithmetic.cpp

Index: clang/test/SemaCXX/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -74,6 +74,10 @@
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
+char* sub(_AS1 char *x,  _AS2 char *y) {
+  return x - y; // expected-error {{arithmetic operation with operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
 struct SomeStruct {
   int a;
   long b;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -11444,8 +11442,7 @@
 if (LCanPointeeTy != RCanPointeeTy) {
   // Treat NULL constant as a special case in OpenCL.
   if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
-const PointerType *LHSPtr = LHSType->castAs();
-if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->castAs())) {
+if (!LCanPointeeTy.isAddressSpaceOverlapping(RCanPointeeTy)) {
   Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSType << RHSType << 0 /* comparison */
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2391,7 +2391,7 @@
 return TC_NotApplicable;
   auto SrcPointeeType = SrcPtrType->getPointeeType();
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
 msg = diag::err_bad_cxx_cast_addr_space_mismatch;
 return TC_Failed;
   }
@@ -2434,9 +2434,9 @@
   const PointerType *SrcPPtr = cast(SrcPtr);
   QualType DestPPointee = DestPPtr->getPointeeType();
   QualType SrcPPointee = SrcPPtr->getPointeeType();
-  if (Nested ? DestPPointee.getAddressSpace() !=
-   SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+  if (Nested
+  ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace()
+  : !DestPPointee.isAddressSpaceOverlapping(SrcPPointee)) {
 Self.Diag(OpRange.getBegin(), DiagID)
 << SrcType << DestType << Sema::AA_Casting
 << SrcExpr.get()->getSourceRange();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,21 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  /// OpenCL C defines conversion rules for pointers to different address spaces
+  /// and notion of overlapping address spaces.
+  /// CL1.1 or CL1.2:
+  ///   address spaces overlap iff they are they same.
+  /// Op

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader marked 2 inline comments as done.
bader added inline comments.



Comment at: clang/include/clang/AST/Type.h:1069
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();

rjmccall wrote:
> It's idiomatic to take `QualType` by value rather than `const &`.
> 
> Can you rewrite the `PointerType` method to delegate to this?  Assuming it 
> isn't completely dead, that is.
It isn't completely dead, but there were just a few uses of the `PointerType` 
method, so I've updated all of them to avoid code duplication in two classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265579.
bader marked an inline comment as done.
bader added a comment.

Fix formatting in clang/test/Sema/address_spaces.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_spaces.c
  clang/test/SemaCXX/address-space-arithmetic.cpp

Index: clang/test/SemaCXX/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -74,6 +74,10 @@
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
+char *sub(_AS1 char *x, _AS2 char *y) {
+  return x - y; // expected-error {{arithmetic operation with operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
 struct SomeStruct {
   int a;
   long b;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -11444,8 +11442,7 @@
 if (LCanPointeeTy != RCanPointeeTy) {
   // Treat NULL constant as a special case in OpenCL.
   if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
-const PointerType *LHSPtr = LHSType->castAs();
-if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->castAs())) {
+if (!LCanPointeeTy.isAddressSpaceOverlapping(RCanPointeeTy)) {
   Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSType << RHSType << 0 /* comparison */
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2391,7 +2391,7 @@
 return TC_NotApplicable;
   auto SrcPointeeType = SrcPtrType->getPointeeType();
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
 msg = diag::err_bad_cxx_cast_addr_space_mismatch;
 return TC_Failed;
   }
@@ -2434,9 +2434,9 @@
   const PointerType *SrcPPtr = cast(SrcPtr);
   QualType DestPPointee = DestPPtr->getPointeeType();
   QualType SrcPPointee = SrcPPtr->getPointeeType();
-  if (Nested ? DestPPointee.getAddressSpace() !=
-   SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+  if (Nested
+  ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace()
+  : !DestPPointee.isAddressSpaceOverlapping(SrcPPointee)) {
 Self.Diag(OpRange.getBegin(), DiagID)
 << SrcType << DestType << Sema::AA_Casting
 << SrcExpr.get()->getSourceRange();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,21 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  /// OpenCL C defines conversion rules for pointers to different address spaces
+  /// and notion of overlapping address spaces.
+  /// CL1.1 or CL1.2:
+  ///   address spaces overlap iff they are they same.
+  /// OpenCL C v2.0 s6.5.5 adds:
+  ///   __generic overlaps with any addr

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe95ee300c053: [SYCL] Prohibit arithmetic operations for 
incompatible pointers (authored by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_spaces.c
  clang/test/SemaCXX/address-space-arithmetic.cpp

Index: clang/test/SemaCXX/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -74,6 +74,10 @@
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
+char *sub(_AS1 char *x, _AS2 char *y) {
+  return x - y; // expected-error {{arithmetic operation with operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
 struct SomeStruct {
   int a;
   long b;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -11444,8 +11442,7 @@
 if (LCanPointeeTy != RCanPointeeTy) {
   // Treat NULL constant as a special case in OpenCL.
   if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
-const PointerType *LHSPtr = LHSType->castAs();
-if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->castAs())) {
+if (!LCanPointeeTy.isAddressSpaceOverlapping(RCanPointeeTy)) {
   Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSType << RHSType << 0 /* comparison */
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2391,7 +2391,7 @@
 return TC_NotApplicable;
   auto SrcPointeeType = SrcPtrType->getPointeeType();
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
 msg = diag::err_bad_cxx_cast_addr_space_mismatch;
 return TC_Failed;
   }
@@ -2434,9 +2434,9 @@
   const PointerType *SrcPPtr = cast(SrcPtr);
   QualType DestPPointee = DestPPtr->getPointeeType();
   QualType SrcPPointee = SrcPPtr->getPointeeType();
-  if (Nested ? DestPPointee.getAddressSpace() !=
-   SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+  if (Nested
+  ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace()
+  : !DestPPointee.isAddressSpaceOverlapping(SrcPPointee)) {
 Self.Diag(OpRange.getBegin(), DiagID)
 << SrcType << DestType << Sema::AA_Casting
 << SrcExpr.get()->getSourceRange();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,21 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  /// OpenCL C defines conversion rules for pointers to different address spaces
+  /// and notion of overlapping address spaces.
+  /// CL1.1 or CL1.2:
+  ///   address spaces overlap iff they are they same.
+  /// OpenCL C v2.0 s6.5.5 adds:
+  ///  

[PATCH] D74387: [OpenMP][SYCL] Improve diagnosing of unsupported types usage

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf6cc662: [OpenMP][SYCL] Improve diagnosing of 
unsupported types usage (authored by Fznamznon, committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Headers/nvptx_device_math_sin.c
  clang/test/Headers/nvptx_device_math_sin.cpp
  clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
  clang/test/SemaSYCL/float128.cpp

Index: clang/test/SemaSYCL/float128.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/float128.cpp
@@ -0,0 +1,96 @@
+// RUN: %clang_cc1 -triple spir64 -fsycl -fsycl-is-device -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsycl -fsycl-is-device -fsyntax-only %s
+
+typedef __float128 BIGTY;
+
+template 
+class Z {
+public:
+  // expected-note@+1 {{'field' defined here}}
+  T field;
+  // expected-note@+1 2{{'field1' defined here}}
+  __float128 field1;
+  using BIGTYPE = __float128;
+  // expected-note@+1 {{'bigfield' defined here}}
+  BIGTYPE bigfield;
+};
+
+void host_ok(void) {
+  __float128 A;
+  int B = sizeof(__float128);
+  Z<__float128> C;
+  C.field1 = A;
+}
+
+void usage() {
+  // expected-note@+1 3{{'A' defined here}}
+  __float128 A;
+  Z<__float128> C;
+  // expected-error@+2 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  // expected-error@+1 {{'field1' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  C.field1 = A;
+  // expected-error@+1 {{'bigfield' requires 128 bit size 'Z::BIGTYPE' (aka '__float128') type support, but device 'spir64' does not support it}}
+  C.bigfield += 1.0;
+
+  // expected-error@+1 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  auto foo1 = [=]() {
+__float128 AA;
+// expected-note@+2 {{'BB' defined here}}
+// expected-error@+1 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+auto BB = A;
+// expected-error@+1 {{'BB' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+BB += 1;
+  };
+
+  // expected-note@+1 {{called by 'usage'}}
+  foo1();
+}
+
+template 
+void foo2(){};
+
+// expected-note@+3 {{'P' defined here}}
+// expected-error@+2 {{'P' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+// expected-note@+1 2{{'foo' defined here}}
+__float128 foo(__float128 P) { return P; }
+
+template 
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc();
+}
+
+int main() {
+  // expected-note@+1 {{'CapturedToDevice' defined here}}
+  __float128 CapturedToDevice = 1;
+  host_ok();
+  kernel([=]() {
+decltype(CapturedToDevice) D;
+// expected-error@+1 {{'CapturedToDevice' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+auto C = CapturedToDevice;
+Z<__float128> S;
+// expected-error@+1 {{'field1' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+S.field1 += 1;
+// expected-error@+1 {{'field' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+S.field = 1;
+  });
+
+  kernel([=]() {
+// expected-note@+1 2{{called by 'operator()'}}
+usage();
+// expected-note@+1 {{'' defined here}}
+BIGTY ;
+// expected-note@+3 {{called by 'operator()'}}
+// expected-error@+2 2{{'foo' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+// expected-error@+1 {{'' requires 128 bit size 'BIGTY' (aka '__float128') type support, but device 'spir64' does not support it}}
+auto A = foo();
+  });
+
+  kernel([=]() {
+Z<__float128> S;
+foo2<__float128>();
+auto A = sizeof(CapturedToDevice);
+  });
+
+  return 0;
+}
Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -7,18 +7,23 @@
 struct T {
   char a;
 #ifndef _ARCH_PPC
+  // expected-note@+1 {{'f' defined here}}
   __float128 f;
 #else
+  // expected-note@+1 {{'f' defined here}}
   long double f;
 #endif
   char c;
   T() : a(12), f(15) {}
 #ifndef _ARCH_P

[PATCH] D80829: [OpenMP][SYCL] Do not crash on attempt to diagnose unsupported type use

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd85b7d66887: [OpenMP][SYCL] Do not crash on attempt to 
diagnose unsupported type use (authored by Fznamznon, committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80829

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp


Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -120,3 +120,14 @@
 long double qa, qb;
 decltype(qa + qb) qc;
 double qd[sizeof(-(-(qc * 2)))];
+
+struct A { };
+
+template 
+struct A_type { typedef A type; };
+
+template 
+struct B {
+  enum { value = bool(Sp::value) || bool(Tp::value) };
+  typedef typename A_type::type type;
+};
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1725,6 +1725,9 @@
   }
 
   auto CheckType = [&](QualType Ty) {
+if (Ty->isDependentType())
+  return;
+
 if ((Ty->isFloat16Type() && !Context.getTargetInfo().hasFloat16Type()) ||
 ((Ty->isFloat128Type() ||
   (Ty->isRealFloatingType() && Context.getTypeSize(Ty) == 128)) &&


Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -120,3 +120,14 @@
 long double qa, qb;
 decltype(qa + qb) qc;
 double qd[sizeof(-(-(qc * 2)))];
+
+struct A { };
+
+template 
+struct A_type { typedef A type; };
+
+template 
+struct B {
+  enum { value = bool(Sp::value) || bool(Tp::value) };
+  typedef typename A_type::type type;
+};
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1725,6 +1725,9 @@
   }
 
   auto CheckType = [&](QualType Ty) {
+if (Ty->isDependentType())
+  return;
+
 if ((Ty->isFloat16Type() && !Context.getTargetInfo().hasFloat16Type()) ||
 ((Ty->isFloat128Type() ||
   (Ty->isRealFloatingType() && Context.getTypeSize(Ty) == 128)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
bader added reviewers: Anastasia, keryell, Naghasan, asavonic, Fznamznon.
Herald added subscribers: cfe-commits, ebevhan, yaxunl.
Herald added a project: clang.

This change enables conversion between OpenCL address spaces and default
address space similar to conversions between OpenCL generic and OpenCL
global/local/private address spaces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80932

Files:
  clang/include/clang/AST/Type.h
  clang/test/SemaOpenCLCXX/address-space-lambda.cl
  clang/test/SemaOpenCLCXX/pr-45472.cpp
  clang/test/SemaSYCL/address-space-parameter-conversions.cpp

Index: clang/test/SemaSYCL/address-space-parameter-conversions.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/address-space-parameter-conversions.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -verify -fsyntax-only %s
+
+void bar(int &Data) {}
+void bar2(int &Data) {}
+void bar(__attribute__((opencl_private)) int &Data) {}
+void foo(int *Data) {}
+void foo2(int *Data) {}
+void foo(__attribute__((opencl_private)) int *Data) {}
+
+template 
+void tmpl(T *t) {}
+
+void usages() {
+  __attribute__((opencl_global)) int *GLOB;
+  __attribute__((opencl_private)) int *PRIV;
+  __attribute__((opencl_local)) int *LOC;
+  int *NoAS;
+
+  bar(*GLOB);
+  bar2(*GLOB);
+
+  bar(*PRIV);
+  bar2(*PRIV);
+
+  bar(*NoAS);
+  bar2(*NoAS);
+
+  bar(*LOC);
+  bar2(*LOC);
+
+  foo(GLOB);
+  foo2(GLOB);
+  foo(PRIV);
+  foo2(PRIV);
+  foo(NoAS);
+  foo2(NoAS);
+  foo(LOC);
+  foo2(LOC);
+
+  tmpl(GLOB);
+  tmpl(PRIV);
+  tmpl(NoAS);
+  tmpl(LOC);
+
+  (void)static_cast(GLOB);
+  (void)static_cast(GLOB);
+  // FIXME: determine if we can warn on the below conversions.
+  int *i = GLOB;
+  void *v = GLOB;
+  (void)i;
+  (void)v;
+
+  // expected-error@+1{{address space is negative}}
+  __attribute__((address_space(-1))) int *TooLow;
+  // expected-error@+1{{unknown type name '__generic'}}
+  __generic int *IsGeneric;
+}
Index: clang/test/SemaOpenCLCXX/pr-45472.cpp
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/pr-45472.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+// FIXME: expected error is not emitted due to wrong address space inference
+// see https://bugs.llvm.org/show_bug.cgi?id=45472 for more details.
+// XFAIL: *
+
+__kernel void test_qual() {
+  auto priv3 = []() __global {}; //expected-note{{candidate function not viable: 'this' object is in address space '__private', but method expects object in address space '__global'}} //expected-note{{conversion candidate of type 'void (*)()'}}
+  priv3();   //expected-error{{no matching function for call to object of type}}
+}
\ No newline at end of file
Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- clang/test/SemaOpenCLCXX/address-space-lambda.cl
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -31,8 +31,6 @@
 //CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const __generic'
   auto priv2 = []() __generic {};
   priv2();
-  auto priv3 = []() __global {}; //expected-note{{candidate function not viable: 'this' object is in address space '__private', but method expects object in address space '__global'}} //expected-note{{conversion candidate of type 'void (*)()'}}
-  priv3(); //expected-error{{no matching function for call to object of type}}
 
   __constant auto const1 = []() __private{}; //expected-note{{candidate function not viable: 'this' object is in address space '__constant', but method expects object in address space '__private'}} //expected-note{{conversion candidate of type 'void (*)()'}}
   const1(); //expected-error{{no matching function for call to object of type '__constant (lambda at}}
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -488,7 +488,12 @@
   /// Returns true if the address space in these qualifiers is equal to or
   /// a superset of the address space in the argument qualifiers.
   bool isAddressSpaceSupersetOf(Qualifiers other) const {
-return isAddressSpaceSupersetOf(getAddressSpace(), other.getAddressSpace());
+return isAddressSpaceSupersetOf(getAddressSpace(),
+other.getAddressSpace()) ||
+   (!hasAddressSpace() &&
+(other.getAddressSpace() == LangAS::opencl_private ||
+ other.getAddressSpace() == LangAS::opencl_local ||
+ other.getAddressSpace() == LangAS::opencl_global));
   }
 
   /// Determines if these qualifiers compatibly include another set.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D82174: [OpenCL] Add global_device and global_host address spaces

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d27be8dbaff: [OpenCL] Add global_device and global_host 
address spaces (authored by bader).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82174

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AddressSpaces.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/language_address_space_attribute.cpp
  clang/test/CodeGenCXX/mangle-address-space.cpp
  clang/test/CodeGenOpenCL/address-spaces-conversions.cl
  clang/test/CodeGenOpenCL/address-spaces.cl
  clang/test/SemaOpenCL/usm-address-spaces-conversions.cl
  clang/test/SemaTemplate/address_space-dependent.cpp

Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@
 
 template 
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388595)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388593)}}
 }
 
 template 
@@ -101,7 +101,7 @@
   car<1, 2, 3>(); // expected-note {{in instantiation of function template specialization 'car<1, 2, 3>' requested here}}
   HasASTemplateFields<1> HASTF;
   neg<-1>(); // expected-note {{in instantiation of function template specialization 'neg<-1>' requested here}}
-  correct<0x73>();
+  correct<0x71>();
   tooBig<8388650>(); // expected-note {{in instantiation of function template specialization 'tooBig<8388650>' requested here}}
 
   __attribute__((address_space(1))) char *x;
Index: clang/test/SemaOpenCL/usm-address-spaces-conversions.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/usm-address-spaces-conversions.cl
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -cl-std=CL2.0 -DGENERIC
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -cl-std=CL2.0 -DCONSTANT
+// RUN: %clang_cc1 %s -ffake-address-space-map -verify -pedantic -fsyntax-only -cl-std=CL2.0 -DLOCAL
+
+/* USM (unified shared memory) extension for OpenCLC 2.0 adds two new address
+ * spaces: global_device and global_host that are a subset of __global address
+ * space. As ISO/IEC TR 18037 5.1.3 declares - it's possible to implicitly
+ * convert a subset address space to a superset address space, while conversion
+ * in a reversed direction could be achived only with an explicit cast */
+
+#ifdef GENERIC
+#define AS_COMP __generic
+#else
+#define AS_COMP __global
+#endif // GENERIC
+
+#ifdef CONSTANT
+#define AS_INCOMP __constant
+#elif LOCAL
+#define AS_INCOMP __local
+#else // PRIVATE
+#define AS_INCOMP __private
+#endif // CONSTANT
+
+void test(AS_COMP int *arg_comp,
+  __attribute__((opencl_global_device)) int *arg_device,
+  __attribute__((opencl_global_host)) int *arg_host) {
+  AS_COMP int *var_glob1 = arg_device;
+  AS_COMP int *var_glob2 = arg_host;
+  AS_COMP int *var_glob3 = (AS_COMP int *)arg_device;
+  AS_COMP int *var_glob4 = (AS_COMP int *)arg_host;
+  arg_device = (__attribute__((opencl_global_device)) int *)arg_comp;
+  arg_host = (__attribute__((opencl_global_host)) int *)arg_comp;
+#ifdef GENERIC
+  // expected-error@+6{{assigning '__generic int *__private' to '__global_device int *__private' changes address space of pointer}}
+  // expected-error@+6{{assigning '__generic int *__private' to '__global_host int *__private' changes address space of pointer}}
+#else
+  // expected-error@+3{{assigning '__global int *__private' to '__global_device int *__private' changes address space of pointer}}
+  // expected-error@+3{{assigning '__global int *__private' to '__global_host int *__private' changes address space of pointer}}
+#endif // GENERIC
+  arg_device = arg_comp;
+  arg_host = arg_comp;
+
+#ifdef CONSTANT
+  // expected-error@+15{{initializing '__constant int *__private' with an expression of type '__global_device int *__private' changes address space of pointer}}
+  // expected-error@+15{{initializing '__constant int *__private' wit

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Alexey Bader via Phabricator via cfe-commits]

bader marked an inline comment as done.
bader added a comment.

In D80932#2068863 , @Anastasia wrote:

>




> Why? Can you explain what you are trying to achieve with this?

I think @asavonic can provide more detailed answer, but IIRC we spent a lot 
time trying to marry template meta-programming with OpenCL address space 
deductions, but even simplest template functions from C++ standard library 
breaks compilation. It's important to note one important difference in design 
goals for SYCL and C++ for OpenCL. SYCL aims to enable compilation of regular 
C++ as much as possible where as one of the C++ for OpenCL goals is to keep 
compatibility with OpenCL C. These two goals might lead to different design 
decisions. As we still wanted to re-use OpenCL address spaces for mapping 
explicitly annotated pointers to SPIR-V address spaces we decided to adopt 
alternative approach (inspired by OpenMP and CUDA modes) with using C++ Sema + 
additional semantic for non-standard attributes (like address spaces - e.g. 
https://reviews.llvm.org/D80317) and types. Address space deduction required by 
SPIR-V specification is implemented CodeGen module and it takes < 200 lines in 
a few files.

> Default address space is normally an address space of automatic storage i.e. 
> private in OpenCL. If you look at the address space map of targets most of 
> them map default and private address spaces to the same value. Converting 
> between private and other named address space is not allowed in OpenCL see 
> section 6.5. I don't believe this change is doing anything legitimate.

AFAIK, OpenCL compiler deduce address space at Sema for all types of storage 
and even automatic (https://godbolt.org/z/-5QoBd). This change is legitimate 
for OpenCL because OpenCL doesn't use default address space and therefore it 
should be a non-functional change for OpenCL mode. I had to move on check from 
the OpenCL C++ test because this changes exposes a bug in the compiler 
(https://bugs.llvm.org/show_bug.cgi?id=45472), but other than that all existing 
OpenCL tests pass.

In SYCL mode we use default address space and apply conversion rules defined 
for generic address space, which is casting from generic to non-generic address 
space is allowed if memory is allocated in the same address space, otherwise 
behavior is undefined. In most cases, users don't need to worry about 
annotating pointers manually. They access memory through accessors which are 
annotated using template parameters, so template meta-programming will prevent 
invalid conversion. There is also an options to get a raw pointer and/or 
annotate a pointer using multi_ptr class, but in this case it's user's 
responsibility to make sure that conversion is valid. This might be required 
only in cases when compiler is not able to automatically optimize memory 
accesses to generic address space.

I was going to upstream CodeGen part with mapping "default address space" to 
LLVM address space in a separate patch. In the nutshell we would like to map 
the default address space to LLVM address space 4 for SPIR target (the same 
change is done for AMDGPU 
).
 NOTE: variables with automatic storage duration in default address space (i.e. 
w/o explicit annotation) do not use address space map, but rather data layout 
string to set the LLVM address space for `alloca` instructions - the default 
value is 0, which is treated as private by SPIR-V translator.

I can work on updating this patch with CodeGen changes if you think both 
patches should be committed together.




Comment at: clang/test/SemaOpenCLCXX/address-space-lambda.cl:34
   priv2();
-  auto priv3 = []() __global {}; //expected-note{{candidate function not 
viable: 'this' object is in address space '__private', but method expects 
object in address space '__global'}} //expected-note{{conversion candidate of 
type 'void (*)()'}}
-  priv3(); //expected-error{{no matching function for call to object of type}}

Anastasia wrote:
> Here we are testing expected behavior. I believe we already had a discussion 
> about this.
Yes, we discussed this here: https://github.com/intel/llvm/pull/1039. 
I agree that behavior is expected, but I think that this checks works in spite 
of a bug @Fznamznon described here 
https://github.com/intel/llvm/pull/1039#discussion_r402905578 and in the bug 
report https://bugs.llvm.org/show_bug.cgi?id=45472.
This patch exposes the bug and I moved this check to a separate file to keep 
all checks active.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80932



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

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Alexey Bader via Phabricator via cfe-commits]

bader marked an inline comment as done.
bader added a comment.

In D80932#2074792 , @Anastasia wrote:

> I think my biggest problem is that I don't understand how you can just run 
> C++ libraries on OpenCL devices. If we were able to compile plain C++ for 
> OpenCL devices would we not just do it? But OpenCL devices have certain 
> constraints. Also aren't libraries typically customized for performance 
> depending on the range of HW they are being run on? I see the modifications 
> are inevitable.


The approach we use won't enable all C++ libraries, but it unblocks template 
metaprogramming patterns which conceptually are valid code. We can use STL 
parts of C++ standard library without re-writing them, which is a huge win for 
C++ developers.

We employ address space inference pass 
(https://llvm.org/doxygen/InferAddressSpaces_8cpp_source.html) to improve the 
performance of C++ libraries. Users can help the compiler analysis and optimize 
code by using SYCL `multi_ptr` template class specialized with particular 
memory region.

>>> We plan to support C++ libraries with C++ for OpenCL
>> 
>> With the direction taken so far, C++ for OpenCL can't properly implement or 
>> use basic C++ due to this.Small example using a `is_same` implementation 
>> (https://godbolt.org/z/CLFV6z):
>> 
>>   template
>>   struct is_same {
>>   static constexpr int value = 0;
>>   };
>>   
>>   template
>>   struct is_same {
>>   static constexpr int value = 1;
>>   };
>>   
>>   void foo(int p) {
>>   static_assert(is_same::value, "int is not an int?"); 
>> // Fails: p is '__private int' != 'int'
>>   static_assert(is_same::value, "int* is not an 
>> int*?");  // Fails: p is '__private int*' != '__generic int*'
>>   }
>> 
>> 
>> So yes, you could implement `std::is_same` but it won't work as one would 
>> expect.
>> 
>> That's the reason why SYCL actively tries to prevent changing any type, this 
>> would prevent the compilation of valid C++ code without a fundamental reason 
>> (e.g. the target is not able to support it).
>> 
>> My point is only that the approach taken for C++ for OpenCL is not suitable 
>> for SYCL IMO
>> 
>>> Default address space is a Clang concept that is generally used for an 
>>> automatic storage.
>> 
>> That's not true in CUDA. On the other hand they actively avoids using 
>> address space.
> 
> True and I don't think CUDA uses `isAddressSpaceSupersetOf` in their 
> implementation? Perhaps SYCL can use the same flow?
> 
>> Plus in `isAddressSpaceSupersetOf` you have:
>> 
>>   // Consider pointer size address spaces to be equivalent to default.
>>   ((isPtrSizeAddressSpace(A) || A == LangAS::Default) &&
>>   (isPtrSizeAddressSpace(B) || B == LangAS::Default))
>> 
>> 
>> An alternative would be to clone the address space. But that would be shame 
>> to not find a way to reuse the opencl one as down the line, they map to the 
>> same thing.
> 
> I agree that reusing functionality is preferable however it is subject to 
> whether it is sufficiently similar. If we end up adding a lot of code to 
> diverge special cases or even worse regress existing functionality by adding 
> new one then perhaps this is not a viable solution.
> 
> The problem is that I understand very little of the design at this point to 
> suggest anything. All I know is that there are magic pointer classes in SYCL 
> spec that represent address spaces somehow but what I see in this review is a 
> change to OpenCL address space model that isn't governed by any spec or 
> explained in any documentation. Perhaps it is easy and transparent to you but 
> I see very little correlation. :(

@Anastasia, if we make this change specific to SYCL mode, will it address your 
concerns?

If not, we will go with the alternative proposed by Victor.
I'd like to mention that just a few hours ago we accepted a PR extending 
address space attributes in SYCL mode to enable efficient code generation of 
the code using USM extension (Unified Shared Memory) on targets w/o hardware 
support for this feature. I think this might be relevant information to take a 
decision.




Comment at: clang/include/clang/AST/Type.h:493
+other.getAddressSpace()) ||
+   (!hasAddressSpace() &&
+(other.getAddressSpace() == LangAS::opencl_private ||

Naghasan wrote:
> Not sure how to make it better, but this may not be true depending on what is 
> allowed by the language.
You are right. For some reason I was sure that these address spaces are 
available only in SYCL and OpenCL modes. I update the patch to enable this 
conversion for SYCL mode only to avoid any impact on OpenCL mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80932



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D80932#2080631 , @Anastasia wrote:

> > @Anastasia, if we make this change specific to SYCL mode, will it address 
> > your concerns?
>
> I can't answer this question for the reasons I have explained above.


Sorry, I'm not sure that I get your concern correctly, so let me clarify: is it 
allowing conversion between pointers w/ and w/o address space annotation in 
non-SYCL mode or using OpenCL address space attributes in SYCL mode?

Just to help you to understand the proposed design, I created the full patch 
for address space handling in SYCL: https://github.com/bader/llvm/pull/18. 
There are few CodeGen tests validating LLVM IR for SPIR target. Fee free to ask 
any questions on this PR.

I also merged it with https://reviews.llvm.org/D71016, which lowers C++ 
function object into OpenCL kernel in this PR (just revert last two commits - 
https://github.com/bader/llvm/pull/18/files/b2772482370844f33093a70e7d17a318caab49ce).
 
It's not directly related to this review, but completes the picture of 
producing LLVM IR for SYCL kernels.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80932



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

[PATCH] D81641: [SYCL][OpenMP] Implement thread-local storage restriction

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D81641#2097433 , @Fznamznon wrote:

> Seems that `test/OpenMP/nvptx_target_codegen.cpp` is completely not 
> formatted. If I apply suggestion from pre-merge checks, this will look like a 
> big unrelated to this patch change and it will contradict with the whole file 
> style.


I formatted this file here: 
https://github.com/llvm/llvm-project/commit/93cd4115799cefa698833ca7a2f1899243d94c77.
Could you rebase your patch, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81641



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

[PATCH] D81641: [SYCL][OpenMP] Implement thread-local storage restriction

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0bdcd95bf20f: [SYCL][OpenMP] Implement thread-local storage 
restriction (authored by Fznamznon, committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81641

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/OpenMP/nvptx_prohibit_thread_local.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/SemaSYCL/prohibit-thread-local.cpp

Index: clang/test/SemaSYCL/prohibit-thread-local.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/prohibit-thread-local.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64 -verify -fsyntax-only %s
+
+thread_local const int prohobit_ns_scope = 0;
+thread_local int prohobit_ns_scope2 = 0;
+thread_local const int allow_ns_scope = 0;
+
+struct S {
+  static const thread_local int prohibit_static_member;
+  static thread_local int prohibit_static_member2;
+};
+
+struct T {
+  static const thread_local int allow_static_member;
+};
+
+void foo() {
+  // expected-error@+1{{thread-local storage is not supported for the current target}}
+  thread_local const int prohibit_local = 0;
+  // expected-error@+1{{thread-local storage is not supported for the current target}}
+  thread_local int prohibit_local2;
+}
+
+void bar() { thread_local int allow_local; }
+
+void usage() {
+  // expected-note@+1 {{called by}}
+  foo();
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)prohobit_ns_scope;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)prohobit_ns_scope2;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)S::prohibit_static_member;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)S::prohibit_static_member2;
+}
+
+template 
+__attribute__((sycl_kernel))
+// expected-note@+2 2{{called by}}
+void
+kernel_single_task(Func kernelFunc) { kernelFunc(); }
+
+int main() {
+  // expected-note@+1 2{{called by}}
+  kernel_single_task([]() { usage(); });
+  return 0;
+}
Index: clang/test/OpenMP/nvptx_target_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_codegen.cpp
@@ -160,7 +160,7 @@
 // CHECK: [[EXIT]]
 // CHECK: ret void
 
-// CHECK: define {{.*}}void [[T2:@__omp_offloading_.+foo.+l200]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]], i[[SZ:32|64]] [[ID:%[a-zA-Z_]+]])
+// CHECK: define {{.*}}void [[T2:@__omp_offloading_.+foo.+l200]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]])
 // CHECK: [[AA_ADDR:%.+]] = alloca i[[SZ]],
 // CHECK: store i[[SZ]] [[ARG1]], i[[SZ]]* [[AA_ADDR]],
 // CHECK: [[AA_CADDR:%.+]] = bitcast i[[SZ]]* [[AA_ADDR]] to i16*
@@ -200,7 +200,7 @@
 #pragma omp target if (1)
   {
 aa += 1;
-id = aa;
+aa += 2;
   }
 
 // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l310}}_worker()
Index: clang/test/OpenMP/nvptx_prohibit_thread_local.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_prohibit_thread_local.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-linux -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple x86_64-unknown-linux -fopenmp-targets=nvptx64-nvidia-cuda %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-host.bc -fsyntax-only
+
+thread_local const int prohobit_ns_scope = 0;
+thread_local int prohobit_ns_scope2 = 0;
+thread_local const int allow_ns_scope = 0;
+
+struct S {
+  static const thread_local int prohibit_static_member;
+  static thread_local int prohibit_static_member2;
+};
+
+struct T {
+  static const thread_local int allow_static_member;
+};
+
+void foo() {
+  // expected-error@+1{{thread-local storage is not supported for the current target}}
+  thread_local const int prohibit_local = 0;
+  // expected-error@+1{{thread-local storage is not supported for the current target}}
+  thread_local int prohibit_local2;
+}
+
+void bar() { thread_local int allow_local; }
+
+void usage() {
+  // expected-note@+1 {{called by}}
+  foo();
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)prohobit_ns_scope;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)prohobit_ns_scope2;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)S::prohibit_static_member;
+  // expected-error@+1 {{thread-local storage is not supported for the current target}}
+  (void)S::prohibit_static_member2;
+}
+
+int main() {
+  // expected-note@+2 2{{calle

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D80932#2085848 , @Anastasia wrote:

> In D80932#2083185 , @bader wrote:
>
> > In D80932#2080631 , @Anastasia 
> > wrote:
> >
> > > > @Anastasia, if we make this change specific to SYCL mode, will it 
> > > > address your concerns?
> > >
> > > I can't answer this question for the reasons I have explained above.
> >
> >
> > Sorry, I'm not sure that I get your concern correctly, so let me clarify: 
> > is it allowing conversion between pointers w/ and w/o address space 
> > annotation in non-SYCL mode or using OpenCL address space attributes in 
> > SYCL mode?
> >
> > Just to help you to understand the proposed design, I created the full 
> > patch for address space handling in SYCL: 
> > https://github.com/bader/llvm/pull/18. There are few CodeGen tests 
> > validating LLVM IR for SPIR target. Fee free to ask any questions on this 
> > PR.
>
>
> Thanks! This is good but it is only an implementation of the design. Deducing 
> the design from an implementation is time-consuming and not sure it is even 
> feasible. I don't want to waste our time to provide you detailed feedback 
> based on my interpretation of your design and invalid assumptions. I don't 
> want to bind you to any particular format and we don't have any strict 
> requirement for this in LLVM either, but I would encourage you to take a look 
> at some of RFC threads sent to cfe-dev that explain new design concepts. 
> Perhaps, they can help you to understand what information can be provided as 
> a starting point to new design discussions.
>
> Particularly I would suggest covering these two points:
>
> - SYCL specifies address spaces as classes and it is not very obvious how did 
> you come from libraries classes to modifications in Clang? I believe there 
> are a number of design choices that you could make. One might think that you 
> can just implement address space classes using existing Clang functionality. 
> But if it is not sufficient it is important to understand what modifications 
> are required and why.
> - Other aspects that are important to highlight whether your design has any 
> limitations. For example, I don't quite understand your need for 
> `SPIR*SYCLDeviceTargetInfo`. Is there anything in your design that limits 
> compilation to one particular target?
>
>   Overall, I see that there are a lot of changes in CodeGen that are related 
> to the language semantic. I believe that CodeGen is supposed to be dialing 
> primarily with target-specific logic and I don't know if you should change 
> them to query target specific details instead. Also most of your CodeGen 
> changes are not related to OpenCL so I would make sure to loop @rjmccall in 
> this thread.


@Anastasia, sorry for the delay.
I've sent an email with RFC covering these questions to cfe-dev mailing list - 
http://lists.llvm.org/pipermail/cfe-dev/2020-June/066047.html.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80932



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

[PATCH] D39936: [OpenCL] Add extensions cl_intel_subgroups and cl_intel_subgroups_short

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added a comment.

LGTM. Thanks!


https://reviews.llvm.org/D39936



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

[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/docs/UsersManual.rst:3602
 
+Linking is done using ``spirv-link`` from `the SPIRV-Tools project
+`_. Similar to other 
external

Anastasia wrote:
> bader wrote:
> > @Anastasia, sorry for late feedback.
> > I think being able to link SPIR-V modules is a great feature, but I have a 
> > concerns regarding `spirv-link` tool.
> > The documentation says that the linker tool is still under development and 
> > from our experience this tool had issues blocking us from using it for SYCL 
> > mode. The last time new features were added to this tool is almost 4 year 
> > ago.
> > Do you know if there are any plans for to finish the development and if ? 
> > Are you aware of any "real-world usages" of this tool? Have you tried to 
> > use it for SPIR-V module produced from C++ (e.g. C++ for OpenCL)?
> > I think supporting SPIR-V extensions like [[ 
> > https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_linkonce_odr.asciidoc
> >  | SPV_KHR_linkonce_odr ]] is quite important for code size and JIT 
> > compilation time reduction. As this extension was ratified recently, I 
> > suppose `spirv-link` doesn't support it yet.
> Hi Alexey,
> 
> Sorry for the late reply. Do you have any other suggestions about the tools 
> that can be used for linking SPIR-V binaries? 
> 
> I am not in contact with the maintainers but it is an open-source project so 
> I imagine contributions to enhance or improve functionality should be 
> welcome... unless you have other experiences?
> 
> Do you have any other suggestions about the tools that can be used for 
> linking SPIR-V binaries?

I'm unaware of other tools for SPIR-V binaries linking. To link SPIR-V binaries 
in our toolchain, we translate them to/from LLVM IR to link LLVM IR.

> I am not in contact with the maintainers but it is an open-source project so 
> I imagine contributions to enhance or improve functionality should be 
> welcome... unless you have other experiences?

I talked to the maintainers (but it was quite long time ago) and they told me 
that there are no active contributors to this tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116266

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

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87b28f5092f2: [clang][NFC] Extract the 
EmitAssemblyHelper::TargetTriple member (authored by psamolysov-intel, 
committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122587

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -695,7 +698,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -965,7 +967,6 @@
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1054,10 +1055,8 @@
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
   bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO &&
-   !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
@@ -1338,7 +1337,6 @@
 
   // Register the target library analysis directly and give it a customized
   // preset TLI.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
@@ -1474,8 +1472,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -695,7 +698,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -965,7 +967,6 @@
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1054,10 +1055,8 @@
   // Emit a module summary by default for Regular LTO except for ld6

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118935

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

[PATCH] D36044: [OpenCL] -cl-ext option can overwrite OpenCL features imported from a module

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: test/SemaOpenCL/extensions-import.cl:14
+
+#ifdef FP64
+// expected-no-diagnostics

You can use `#ifdef cl_khr_fp64` to check if extension is supported.


https://reviews.llvm.org/D36044



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

[PATCH] D36259: [OpenCL] Remove extra select functions from opencl-c.h

[Alexey Bader via Phabricator via cfe-commits]

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

Out of curiosity: how did you detect this?
Can we use the same approach for writing tests?


https://reviews.llvm.org/D36259



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

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added inline comments.



Comment at: lib/AST/Expr.cpp:4000-4004
+  if (auto AT = T->getAs()) {
+return AT->getValueType();
+  } else {
+return T;
+  }

No need in else branch after return:
```
if (...) {
  return AT->getValueType();
}

return T;
```

http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return



Comment at: test/SemaOpenCL/atomic-ops.cl:1
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify 
-fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify 
-fsyntax-only -triple=amdgcn-amdhsa-amd-opencl

It's a pity, we have to parse the whole opencl-c.h file to get two enums and 
one typedef...


https://reviews.llvm.org/D28691



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

[PATCH] D36327: [OpenCL] Allow targets emit optimized pipe functions for power of 2 type sizes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Hi Sam,

What do you think about implementing this optimization in target specific 
optimization pass? Since size/alignment is saved as function parameter in LLVM 
IR, the optimization can be done in target specific components w/o adding 
additional conditions to generic library.

Thanks,
Alexey


https://reviews.llvm.org/D36327



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

[PATCH] D36327: [OpenCL] Allow targets emit optimized pipe functions for power of 2 type sizes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

@rsmith do you have an opinion on what would be the right place for the kind of 
proposed optimization?
It looks like it can be implemented as target independent optimization, acting 
only for target with specified properties - in this case target must provide 
required built-in functions.


https://reviews.llvm.org/D36327



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

[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Headers/opencl-c.h:16020
+// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID.
+#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t))
 bool __ovld is_valid_reserve_id(reserve_id_t reserve_id);

b-sumner wrote:
> yaxunl wrote:
> > bader wrote:
> > > yaxunl wrote:
> > > > bader wrote:
> > > > > yaxunl wrote:
> > > > > > bader wrote:
> > > > > > > yaxunl wrote:
> > > > > > > > yaxunl wrote:
> > > > > > > > > bader wrote:
> > > > > > > > > > yaxunl wrote:
> > > > > > > > > > > Anastasia wrote:
> > > > > > > > > > > > echuraev wrote:
> > > > > > > > > > > > > yaxunl wrote:
> > > > > > > > > > > > > > Anastasia wrote:
> > > > > > > > > > > > > > > yaxunl wrote:
> > > > > > > > > > > > > > > > Anastasia wrote:
> > > > > > > > > > > > > > > > > Looks good from my side.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > @yaxunl , since you originally committed 
> > > > > > > > > > > > > > > > > this. Could you please verify that changing 
> > > > > > > > > > > > > > > > > from `SIZE_MAX` to `0` would be fine.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Btw, we have a similar definition for 
> > > > > > > > > > > > > > > > > `CLK_NULL_EVENT`.
> > > > > > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation 
> > > > > > > > > > > > > > > > detail and not part of the spec. I would 
> > > > > > > > > > > > > > > > suggest to remove it from this header file.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to 
> > > > > > > > > > > > > > > > be defined but does not define its value. 
> > > > > > > > > > > > > > > > Naturally a valid id starts from 0 and 
> > > > > > > > > > > > > > > > increases. I don't see significant advantage to 
> > > > > > > > > > > > > > > > change CLK_NULL_RESERVE_ID from __SIZE_MAX to 0.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Is there any reason that this change is needed?
> > > > > > > > > > > > > > > I don't see issues to commit things outside of 
> > > > > > > > > > > > > > > spec as soon as they prefixed properly with "__". 
> > > > > > > > > > > > > > >  But I agree it would be nice to see if it's any 
> > > > > > > > > > > > > > > useful and what the motivation is for having 
> > > > > > > > > > > > > > > different implementation.
> > > > > > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that 
> > > > > > > > > > > > > > the implementation uses one specific bit of a 
> > > > > > > > > > > > > > reserve id to indicate that the reserve id is 
> > > > > > > > > > > > > > valid. Not all implementations assume that. 
> > > > > > > > > > > > > > Actually I am curious why that is needed too.
> > > > > > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id 
> > > > > > > > > > > > > is valid if significant bit equal to one. 
> > > > > > > > > > > > > `CLK_NULL_RESERVE_ID refers to an invalid 
> > > > > > > > > > > > > reservation, so if `CLK_NULL_RESERVE_ID equal to 0, 
> > > > > > > > > > > > > we can be sure that significant bit doesn't equal to 
> > > > > > > > > > > > > 1 and it is invalid reserve id. Also it is more 
> > > > > > > > > > > > > obviously if CLK_**NULL**_RESERVE_ID is equal to 0.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I 
> > > > > > > > > > > > > understand previous implementation also assumes that 
> > > > > > > > > > > > > one specific bit was of a reverse id was used to 
> > > > > > > > > > > > > indicate that the reserve id is valid. So, we just 
> > > > > > > > > > > > > increased reserve id size by one bit on 32-bit 
> > > > > > > > > > > > > platforms and by 33 bits on 64-bit platforms. 
> > > > > > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 
> > > > > > > > > > > > 0, but spec doesn't define it of course.
> > > > > > > > > > > In our implementation, valid reserve id starts at 0 and 
> > > > > > > > > > > increasing linearly until `__SIZE_MAX-1`. This change 
> > > > > > > > > > > will break our implementation.
> > > > > > > > > > > 
> > > > > > > > > > > However, we can modify our implementation to adopt this 
> > > > > > > > > > > change since it brings about benefits overall.
> > > > > > > > > > Ideally it would be great to have unified implementation, 
> > > > > > > > > > but we can define device specific value for 
> > > > > > > > > > CLK_NULL_RESERVE_ID by using ifdef directive.
> > > > > > > > > How about
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > __attribute__((const)) size_t __clk_null_reserve_id();
> > > > > > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id()
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > I think the spec does not require it to be compile time 
> > > > > > > > > constant. Then each library can implement its own 
> > > > > > > > > __clk_null_res

[PATCH] D36676: Remove -finclude-default-header in OpenCL atomic tests

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM! Thanks.


https://reviews.llvm.org/D36676



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

[PATCH] D36327: [OpenCL] Allow targets emit optimized pipe functions for power of 2 type sizes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In https://reviews.llvm.org/D36327#840616, @yaxunl wrote:

> In https://reviews.llvm.org/D36327#839809, @rjmccall wrote:
>
> > Could you just implement this in SimplifyLibCalls?  I assume there's some 
> > way to fill in TargetLibraryInfo appropriately for a platform.  Is that too 
> > late for your linking requirements?
>
>
> Both the optimized and generic versions of __read_pipe function contains call 
> of other library functions and are complicate enough not to be generated 
> programmatically. amdgpu target does not have the capability to link in 
> library code after LLVM codegen. The linking has to be done before 
> SimplifyLibCalls.


If I understand correctly, SimplifyLibCalls is LLVM IR transformation, so it 
works before linking and LLVM codegen (e.g. InstCombine passes run this 
transformation). This pass is doing something similar to what you are trying to 
achieve for __read_pipe builti-ins: pow(2.0, x) -> llvm.exp2(x).


https://reviews.llvm.org/D36327



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

[PATCH] D36951: [OpenCL][5.0.0 Release] Release notes for OpenCL in Clang

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM, except one nit.




Comment at: docs/ReleaseNotes.rst:146
+-  Extended Clang builtins with required ``cl_khr_subgroups`` support.
+-  Now interpreting empty argument function as void argument list in OpenCL 
code.
+-  Add ``intel_reqd_sub_group_size`` attribute support.

As far as I remember, this change was reverted as it had caused some 
regressions. I haven't fix them yet, so I users still have to explicitly 
specify void for empty parameter list.


https://reviews.llvm.org/D36951



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

[PATCH] D71962: Fix crash in getFullyQualifiedName for inline namespace

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
bader added reviewers: bkramer, ilya-biryukov.
Herald added subscribers: cfe-commits, ebevhan.
Herald added a project: clang.

The ICE happens when the most outer namespace is an inline namespace.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71962

Files:
  clang/lib/AST/QualTypeNames.cpp
  clang/unittests/Tooling/QualTypeNamesTest.cpp


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -223,6 +223,17 @@
   "}\n"
   );
 
+  TypeNameVisitor InlineNamespace;
+  InlineNamespace.ExpectedQualTypeNames["c"] = "B::C";
+  InlineNamespace.runOver("inline namespace A {\n"
+  "  namespace B {\n"
+  "class C {};\n"
+  "  }\n"
+  "}\n"
+  "using namespace A::B;\n"
+  "C c;\n",
+  TypeNameVisitor::Lang_CXX11);
+
   TypeNameVisitor AnonStrucs;
   AnonStrucs.ExpectedQualTypeNames["a"] = "short";
   AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -192,7 +192,7 @@
   // Ignore inline namespace;
   NS = dyn_cast(NS->getDeclContext());
 }
-if (NS->getDeclName()) {
+if (NS && NS->getDeclName()) {
   return createNestedNameSpecifier(Ctx, NS, WithGlobalNsPrefix);
 }
 return nullptr;  // no starting '::', no anonymous


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -223,6 +223,17 @@
   "}\n"
   );
 
+  TypeNameVisitor InlineNamespace;
+  InlineNamespace.ExpectedQualTypeNames["c"] = "B::C";
+  InlineNamespace.runOver("inline namespace A {\n"
+  "  namespace B {\n"
+  "class C {};\n"
+  "  }\n"
+  "}\n"
+  "using namespace A::B;\n"
+  "C c;\n",
+  TypeNameVisitor::Lang_CXX11);
+
   TypeNameVisitor AnonStrucs;
   AnonStrucs.ExpectedQualTypeNames["a"] = "short";
   AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -192,7 +192,7 @@
   // Ignore inline namespace;
   NS = dyn_cast(NS->getDeclContext());
 }
-if (NS->getDeclName()) {
+if (NS && NS->getDeclName()) {
   return createNestedNameSpecifier(Ctx, NS, WithGlobalNsPrefix);
 }
 return nullptr;  // no starting '::', no anonymous
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71962: Fix crash in getFullyQualifiedName for inline namespace

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG128f39da932b: Fix crash in getFullyQualifiedName for inline 
namespace (authored by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71962

Files:
  clang/lib/AST/QualTypeNames.cpp
  clang/unittests/Tooling/QualTypeNamesTest.cpp


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -223,6 +223,17 @@
   "}\n"
   );
 
+  TypeNameVisitor InlineNamespace;
+  InlineNamespace.ExpectedQualTypeNames["c"] = "B::C";
+  InlineNamespace.runOver("inline namespace A {\n"
+  "  namespace B {\n"
+  "class C {};\n"
+  "  }\n"
+  "}\n"
+  "using namespace A::B;\n"
+  "C c;\n",
+  TypeNameVisitor::Lang_CXX11);
+
   TypeNameVisitor AnonStrucs;
   AnonStrucs.ExpectedQualTypeNames["a"] = "short";
   AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -192,7 +192,7 @@
   // Ignore inline namespace;
   NS = dyn_cast(NS->getDeclContext());
 }
-if (NS->getDeclName()) {
+if (NS && NS->getDeclName()) {
   return createNestedNameSpecifier(Ctx, NS, WithGlobalNsPrefix);
 }
 return nullptr;  // no starting '::', no anonymous


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -223,6 +223,17 @@
   "}\n"
   );
 
+  TypeNameVisitor InlineNamespace;
+  InlineNamespace.ExpectedQualTypeNames["c"] = "B::C";
+  InlineNamespace.runOver("inline namespace A {\n"
+  "  namespace B {\n"
+  "class C {};\n"
+  "  }\n"
+  "}\n"
+  "using namespace A::B;\n"
+  "C c;\n",
+  TypeNameVisitor::Lang_CXX11);
+
   TypeNameVisitor AnonStrucs;
   AnonStrucs.ExpectedQualTypeNames["a"] = "short";
   AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -192,7 +192,7 @@
   // Ignore inline namespace;
   NS = dyn_cast(NS->getDeclContext());
 }
-if (NS->getDeclName()) {
+if (NS && NS->getDeclName()) {
   return createNestedNameSpecifier(Ctx, NS, WithGlobalNsPrefix);
 }
 return nullptr;  // no starting '::', no anonymous
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Does it make sense to implement such diagnostics in clang Sema, considering 
that OpenCL does not allow recursion?
We implemented similar diagnostics for SYCL programming model and would be like 
to upstream it to clang later 
(https://github.com/intel/llvm/commit/4efe9fcf2dc6f6150b5b477b0f8320ea13a7f596).
 Can we somehow leverage this work for the compiler?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72362



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

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

[Alexey Bader via Phabricator via cfe-commits]

bader added a subscriber: Naghasan.
bader added a comment.

In D72362#1817682 , @lebedev.ri wrote:

> In D72362#1817599 , @bader wrote:
>
> > Does it make sense to implement such diagnostics in clang Sema, considering 
> > that OpenCL does not allow recursion?
> >  We implemented similar diagnostics for SYCL programming model and would be 
> > like to upstream it to clang later 
> > (https://github.com/intel/llvm/commit/4efe9fcf2dc6f6150b5b477b0f8320ea13a7f596).
> >  Can we somehow leverage this work for the compiler?
>
>
> Implementing it elsewhere will be more restrictive in the future - somehow i 
> suspect
>  it will be easier to make clang-tidy CTU-aware rather than clang sema.
>
> That being said, is SYCL inherently single-TU, does it not support
>  linking multiple separately compiled object files together?


SYCL doesn't require multi-TU support. AFAIK, ComputeCPP implementation is 
signle-TU. +@Naghasan to confirm/clarify.
The open source implementation I referred to, does support linking separately 
compiled object files, but still I think detecting single-TU recursion in clang 
is very useful.
Is it possible to have both: intra-TU diagnostics in clang and inter-TU 
diagnostics in clang-tidy tool? Share any infrastructure (e.g. recursion 
detection)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72362



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

[PATCH] D88303: [clang][codegen] Remove the insertion of `correctly-rounded-divide-sqrt-fp-math` fn-attr.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

I'm okay with this change in general. One note though.

There are two aspects of this change:

1. I agree that `cl-fp32-correctly-rounded-divide-sqrt` is OpenCL specific and 
nothing should not be added in other modes.
2. I have concerns WRT removing the attribute in OpenCL mode. Most of the 
OpenCL back-ends are out-of-tree and might still rely on a function attribute. 
In addition to that https://reviews.llvm.org/D22940 handles division part of 
the requirement, but I don't know if sqrt computation will be handled correctly.

Can we split the patch into two: first to add attribute only in OpenCL language 
mode and second one removing function attribute in OpenCL mode as well?
In case we need to rollback the patch, I expect no issues with the first part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88303

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

[PATCH] D88303: [clang][codegen] Remove the insertion of `correctly-rounded-divide-sqrt-fp-math` fn-attr.

[Alexey Bader via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88303

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

[PATCH] D88424: [clang][codegen] Remove the insertion of `correctly-rounded-divide-sqrt-fp-math` fn-attr.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

You might need add [OpenCL] tag to the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88424

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