[PATCH] D42844: [OpenCL] Add test for atomic pointers.

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Yes, we don't have anything OpenCL specific for atomic pointers. 
But we have OpenCL specific in pointers and atomics separately and we have 
address spaces.
Why not to test all at once?


Repository:
  rC Clang

https://reviews.llvm.org/D42844



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: cfe-commits, Anastasia, ebevhan.
Herald added a project: clang.

SYCL runtime is supposed to use sycl_kernel attribute to mark functions in
the single source which are supposed to be compiled for the device.
Compiler is supposed to understand if there are other functions/variables
which are supposed to be compiled for the device and mark them with
sycl_device attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/kernel-attribute.cpp


Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify -pedantic %s
+// expected-no-diagnostics
+
+__attribute((sycl_kernel)) void foo() {
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, 
SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, 
SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, 
SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,9 @@
   case ParsedAttr::AT_Flatten:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_SYCLKernel:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_Format:
 handleFormatAttr(S, D, AL);
 break;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -291,6 +291,7 @@
 def MicrosoftExt : LangOpt<"MicrosoftExt">;
 def Borland : LangOpt<"Borland">;
 def CUDA : LangOpt<"CUDA">;
+def SYCL : LangOpt<"SYCLIsDevice">;
 def COnly : LangOpt<"CPlusPlus", 1>;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
@@ -995,6 +996,20 @@
   let Documentation = [Undocumented];
 }
 
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
+
+def SYCLKernel : InheritableAttr {
+  let Spellings = [GNU<"sycl_kernel">];
+  let Subjects = SubjectList<[Function]>;
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
+
 def C11NoReturn : InheritableAttr {
   let Spellings = [Keyword<"_Noreturn">];
   let Subjects = SubjectList<[Function], ErrorDiag>;


Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify -pedantic %s
+// expected-no-diagnostics
+
+__attribute((sycl_kernel)) void foo() {
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,9 @@
   case ParsedAttr::AT_Flatt

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Hi all,

I assume that something to mark the code which is supposed to be offloaded 
already is implemented for OpenMP and CUDA. For example I found CUDA-specific 
CUDADevice attribute.
I can't just use CUDA-specific attributes for SYCL but I think that these 
resources for marking device code could be unified and re-used for all 
supported single-source offload programming models (like SYCL, CUDA, OpenMP 
etc).
Please let me know If you have any suggestions about it.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;

aaron.ballman wrote:
> Is there a reason to not also introduce a C++11 and C2x style spelling in the 
> `clang` namespace? e.g., `[[clang::sycl_device]]`
I don't think that it makes sense because these attributes not for public 
consumption. These attributes is needed to separate code which is supposed to 
be offloaded from regular host code. I think SYCLDevice attribute actually 
doesn't need a spelling because it will be added only implicitly by compiler.



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

aaron.ballman wrote:
> No new, undocumented attributes, please.
As I said, these attributes are not for public consumption. Should I add 
documentation in this case too?


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1459935 , @Anastasia wrote:

> I was just wondering since SYCL is intended to be a single source C++ for 
> OpenCL and this attribute is for internal use is it possible to just reuse 
> existing OpenCL kernel attribute?
>
> I have raised a couple of related questions on the earlier RFC btw but there 
> hasn't been any follow up since then I believe.


Good point. But in SYCL actually two types of functions that can be compiled 
for device: SYCL kernels and device functions. SYCL kernels are analog for 
OpenCL kernels, these are some functions that can be called from host. SYCL 
kernels should also have OpenCL kernel attribute to be OpenCL kernels. But if 
SYCL kernel calls any function - this function is compiled for device and 
called a “device function”. We need to detect it in compiler. And "device 
function" cannot be called from host and cannot be OpenCL kernel. I think there 
is no attribute for SYCL "device functions" in OpenCL because there is no 
"device functions".
So, I think I can reuse OpenCL kernel attribute (possible with __kernel 
keyword) for SYCL kernels but I still need SYCLDevice attribute to mark "device 
functions".
What do you think about it?


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 194878.
Fznamznon added a comment.

Applied comments from @aaron.ballman and @keryell

- Introduce 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attrubutes.cpp

Index: clang/test/SemaSYCL/device-attrubutes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attrubutes.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_device]] int gv = 0;
+__attribute((sycl_device)) int gv1 = 0;
+[[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}}
+
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+__attribute((sycl_device(1))) void foo1(); // expected-error {{'sycl_device' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_device(1)]] void foo3(); // expected-error {{'sycl_device' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling a C++ file. There should be warnings.
+// RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c++ %s
+
+#if defined(EXPECT_WARNINGS)
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,12 @@
   case ParsedAttr::AT_Flatten:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_SYCLDevice:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_SYCLKernel:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_Format:
 handleFormatAttr(S, D, AL);
 break;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -253,6 +253,48 @@
   }];
 }
 
+def SYCLDeviceDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The sycl_device attribute specifies function which is supposed to be 

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon 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();

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.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 195128.
Fznamznon added a comment.

Applied comments from @bader


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_device]] int gv = 0;
+__attribute((sycl_device)) int gv1 = 0;
+[[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}}
+
+__attribute((sycl_kernel)) __attribute((sycl_device)) void foo();
+[[clang::sycl_kernel]] [[clang::sycl_device]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+__attribute((sycl_device(1))) void foo1(); // expected-error {{'sycl_device' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_device(1)]] void foo3(); // expected-error {{'sycl_device' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling a C++ file. There should be warnings.
+// RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,12 @@
   case ParsedAttr::AT_Flatten:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_SYCLDevice:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_SYCLKernel:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_Format:
 handleFormatAttr(S, D, AL);
 break;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -253,6 +253,50 @@
   }];
 }
 
+def SYCLDeviceDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The sycl_device attribute specifies function which is supposed to be compiled
+for the device and cannot be directly called by the host. Here is code example
+of the SYCL program, which demonstrates the need for this attribute:
+
+.. code-block:: c++
+
+  int foo(int x) { return ++x; }
+
+  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);
+});
+  }
+
+Code is passed to parallel_for is called "kernel function" and defines some
+entry poi

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 195134.
Fznamznon added a comment.

Applied comment from @bader.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_device]] int gv = 0;
+__attribute((sycl_device)) int gv1 = 0;
+[[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}}
+
+__attribute((sycl_kernel)) __attribute((sycl_device)) void foo();
+[[clang::sycl_kernel]] [[clang::sycl_device]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+__attribute((sycl_device(1))) void foo1(); // expected-error {{'sycl_device' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_device(1)]] void foo3(); // expected-error {{'sycl_device' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling a C++ file. There should be warnings.
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_device' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+__attribute((sycl_device)) void foo1();
+[[clang::sycl_kernel]] void foo2();
+[[clang::sycl_device]] void foo3();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,12 @@
   case ParsedAttr::AT_Flatten:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_SYCLDevice:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_SYCLKernel:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_Format:
 handleFormatAttr(S, D, AL);
 break;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -253,6 +253,50 @@
   }];
 }
 
+def SYCLDeviceDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The sycl_device attribute specifies function which is supposed to be compiled
+for the device and cannot be directly called by the host. Here is code example
+of the SYCL program, which demonstrates the need for this attribute:
+
+.. code-block:: c++
+
+  int foo(int x) { return ++x; }
+
+  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);
+});
+  }
+
+Code is passed to parallel_for is called "kernel function" and defines some
+entry point to device code 

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1467018 , @Anastasia wrote:

> Just to understand how this will work. I would imagine you can have a device 
> function definition preceding its use. When the function is being parsed it's 
> not known yet whether it will be called from the device or not, so it won't 
> be possible to set the language mode correctly and hence provide the right 
> diagnostics. So is the plan to launch a separate parsing phase then just to 
> extract the call graph and annotate the device functions?


I'm not an expert in clang terminology but I will try briefly explain our 
current implementation approach. 
In SYCL all kernel functions should be template functions so these functions 
have a deferred instantiation. If we found that we instantiated a sycl kernel 
function - we add it to a special array with sycl device functions (you can see 
the corresponding code here 

 and here 
,
 actually `AddSyclKernel` adds declaration to the special array inside the 
Sema).  After performing
pending instantiations we run a recursive AST visitor for each SYCL kernel to 
mark all device functions and add them to a special array with SYCL device 
functions (here 

 we start traverse AST from `MarkDevice` function, code of `MarkDevice` is here 
).
To get a correct set of SYCL device functions in produced module we added a 
check for all declarations inside the CodeGen on  `sycl_device` attribute 
existence - so it will ignore declarations without `sycl_device` attribute if 
we are compiling for SYCL device (code is here 
).
 But with this check it's possible situation when function was parsed and 
ignored by the CodeGen before we added `sycl_device' attribute to it so we 
added yet another parsing action inside the clang::ParseAST to generate code 
for all SYCL device functions from the special array (code is here 
).

> 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?


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:286
+help.
+  }];
+}

Anastasia wrote:
> keryell wrote:
> > aaron.ballman wrote:
> > > I'm still not entirely certain how I would know what to mark and how. 
> > > From the description, it sounds like whoever authors `parallel_for` needs 
> > > to do this marking, or it somehow happens automatically?
> > > 
> > > (I'll do another editorial pass once I understand the intended behavior a 
> > > bit better -- I expect there will be a few more wording issues to 
> > > address.)
> > In normal SYCL it happens automatically.
> > In the compiler unit-tests it is done manually to exercise the framework.
> > I am the one who suggested that in some other contexts, it could be used 
> > manually for some special purpose like using some weird hardware, but I do 
> > not want to derail the main review with this.
> > In normal SYCL it happens automatically.
> > In the compiler unit-tests it is done manually to exercise the framework.
> > 
> 
> 
> I think if they are not to be exposed to the user they should have no 
> spelling. There are plenty of other ways to test this. For example AST dump.
> 
> 
> > I am the one who suggested that in some other contexts, it could be used 
> > manually for some special purpose like using some weird hardware, but I do 
> > not want to derail the main review with this.
> 
> If you are suggesting to expose this feature then you are starting some sort 
> of a language extensions and its use should be documented in some way. I am 
> not sure about this but I think we will end up with some sort of a language 
> extension for SYCL anyways because as it stands now it's not aligned with the 
> general concept of C/C++ language design. So perhaps it's not entirely 
> unreasonable to expose this.
Generally the `sycl_device` attribute will be added automatically by the 
compiler. But as @bader mentioned before:
> 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.
I think It would be better to re-use this attribute in implementation of 
"device function marker" macro from SYCL spec than implement additional logic 
in the compiler to handle this macro. So I saved possibility to add this 
attribute in code.






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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



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

aaron.ballman wrote:
> This comment confuses me -- the file *is* a C++ file and the preceding RUN 
> line treats it as such, doesn't it?
I've tried to say that we compile regular C++ file without SYCL mode enabled. 
Looks like it can confuse since files with SYCL code have cpp extension... I 
will rewrite this comment.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1467279 , @aaron.ballman 
wrote:

>




> I'm still wondering what the actual semantics are for the attribute. Right 
> now, these are being parsed and ignored -- are there future plans here?

Yes, we are going to teach the compiler differ SYCL device code from host code 
and ignore functions without device attributes when SYCL device mode is 
enabled. I've described some possible implementation details in this comment 
.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1470318 , @keryell wrote:

> It would be better to rename `clang/test/SemaSYCL/device-attrubutes.cpp` to 
> `clang/test/SemaSYCL/device-attributes.cpp`


It's already renamed in the latest patch.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

I tried to reuse OpenCL kernel attribute with "__kernel" keyword in our current 
SYCL implementation. PR with this try is here  - 
https://github.com/intel/llvm/pull/97
Now It looks feasible but with a couple notes:
From SYCL specification "SYCL is designed to be as close to standard C++ as 
possible. Standard C++ compiler can compile the SYCL programs and they will run 
correctly on host CPU." So SYCL doesn't provide non-standard `kernel` keyword 
which is provided by OpenCL. Due this fact it's not possible to add `kernel` 
keyword as in OpenCL, it will prevent compilation of following valid SYCL code:

  int foo(int kernel) { return ++kernel; } // If "kernel" will be a keyword 
like in OpenCL, here will be a error
  …
  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);
});
  }
  ...

So I added only `__kernel` keyword for SYCL because in C++ identifiers which 
start with `__` are reserved for compiler internals.
Next note:
In our current implementation actually not quite that function which is marked 
with `sycl_kernel` (or `__kernel`, whatever) will be real OpenCL kernel in 
produced module. In SYCL all shared between host and device memory objects 
(buffers/images, these objects map to OpenCL buffers and images) can be 
accessed through special `accessor` classes. SYCL also has special mechanism 
for passing kernel arguments from host to device, if in OpenCL you need to do 
`clSetKernelArg`, in SYCL all kernel arguments are captures/fields of 
lambda/functor which is passed to `parallel_for` (See code snippet above, here 
one kernel argument - accessor `A` ). To map to OpenCL setting kernel arguments 
mechanism we added generation of some "kernel wrapper" function inside the 
compiler. "Kernel wrapper" function contains body of SYCL kernel function, 
receives OpenCL like parameters and additionally does some manipulation to 
initialize captured lambda fields with this parameters. In some pseudo code 
"kernel wrapper" looks like this:

  // SYCL kernel is defined in SYCL headers
  __kernel someSYCLKernel(lambda) {
lambda();
  }
  // Kernel wrapper
  __kernel wrapper(global int* a) {
lambda; // Actually lambda declaration doesn't have a name in AST
// Let lambda has one captured field - accessor A. We need to init it with 
global pointer from arguments:
lambda.A.__init(a);
// Body of SYCL kernel from SYCL headers:
{
  lambda();
}
  }

And actually kernel wrapper is presented in result module and passed to OpenCL 
backend.
As I said, kernel wrapper is generated by the compiler inside the Sema and 
OpenCLKernel attribute manually added to it, no matter which attribute was 
added to "SYCL kernel" from SYCL headers.
So, while we are generating this wrapper I see only one profit to use OpenCL 
kernel attribute in SYCL kernels - don't add new attribute to clang (but we 
need to add `__kernel` keyword to SYCL).
I thought about idea - don't generate kernel wrapper but looks like it will not 
work with OpenCL since we can't pass OpenCL `cl_mem` arguments inside any 
structures (including accessors and lambdas) to the kernel.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



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

Anastasia wrote:
> Undocumented -> SYCLKernelDocs
Oh, Thank you for that!



Comment at: clang/lib/Parse/ParseAST.cpp:171
 
+  if (S.getLangOpts().SYCLIsDevice) {
+for (Decl *D : S.SyclDeviceFuncs()) {

Anastasia wrote:
> Do you also need to prevent generation of non-device functions somehow?
I think It's already prevented by change to CodeGenModule.cpp in this patch. 
CodeGen just ignores declarations without SYCL device attribute now.



Comment at: clang/lib/Sema/SemaSYCL.cpp:23
+
+  bool VisitCallExpr(CallExpr *e) {
+if (FunctionDecl *Callee = e->getDirectCallee()) {

Anastasia wrote:
> Anastasia wrote:
> > bader wrote:
> > > Anastasia wrote:
> > > > This is probably not something we can change at this point but I wish 
> > > > we could avoid complexities like this. :(
> > > > 
> > > > I think this is also preventing traditional linking of translation 
> > > > units. That is somewhat unfortunate.
> > > > 
> > > > It is good direction however to keep this logic in a separate dedicated 
> > > > compilation unit.
> > > > 
> > > > I would suggest to document it a bit more including any current 
> > > > limitations/assumption that you can mark under FIXME i.e. does your 
> > > > code handle lambdas yet, what if lambdas are used in function 
> > > > parameters, etc...
> > > > I think this is also preventing traditional linking of translation 
> > > > units.
> > > 
> > > Could you elaborate more on this topic, please?
> > > What do you mean by "traditional linking of translation units" and what 
> > > exactly "is preventing" it?
> > > Do you compare with the linking of regular C++ code (i.e. which do not 
> > > split into host and device code)?
> > > If so, SYCL is different from this model and more similar to CUDA/OpenMP 
> > > models, which also skip "linking" of irrelevant part (e.g. host code is 
> > > not linked by the device compiler).
> > > Mariya added Justin (@jlebar) and Alexey (@ABataev), who work on 
> > > single-source programming models to make them aware and provide feedback 
> > > if any.
> > Yes indeed, I mean linking of modules in C/C++ even though it doesn't 
> > necessarily mean linking of object files. So you don't plan to support 
> > `SYCL_EXTERNAL` in clang?
> > 
> > In CUDA the functions executed on device are annotated manually using 
> > `__device__` hence separate translation units can specify external device 
> > function... although I don't know if CUDA implementation in clang support 
> > this.
> > 
> > I guess OpenMP is allowed to fall back to run on host?
> Ping!
> 
> 
> 
> > I would suggest to document it a bit more including any current 
> > limitations/assumption that you can mark under FIXME i.e. does your code 
> > handle lambdas yet, what if lambdas are used in function parameters, etc...
> 
> 
Oh, sorry, I missed this comment when I updated patch last time.
Could you please advise in which form I can document it?



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5520
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - 
they

Anastasia wrote:
> May be this should go into a helper function as it seems to be now a bigger 
> chunk of code that is repeated?
> 
> Although, I am not very familiar with this code. You can try to get someone 
> to review who has contributed to this more recently.
I think this chunk of code seems big because of big repeated comment.



Comment at: clang/test/CodeGenSYCL/device-functions.cpp:24
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar

Anastasia wrote:
> I can't see where the SPIR calling convention is currently set for SYCL?
If I understand correct it's set automatically on AST level because we use 
SPIR-based triple for device code. Only in case of C++ methods clang doesn't 
set SPIR calling convention. We did a modification in our codebase to get SPIR 
calling convention for C++ methods too (available [[ 
https://github.com/intel/llvm/blob/c13cb8df84418cb5d682f3bbee89090ebb0d00be/clang/lib/AST/ASTContext.cpp#L10015
 | here ]] ) 



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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/CodeGenSYCL/device-functions.cpp:24
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar

Anastasia wrote:
> Fznamznon wrote:
> > Anastasia wrote:
> > > I can't see where the SPIR calling convention is currently set for SYCL?
> > If I understand correct it's set automatically on AST level because we use 
> > SPIR-based triple for device code. Only in case of C++ methods clang 
> > doesn't set SPIR calling convention. We did a modification in our codebase 
> > to get SPIR calling convention for C++ methods too (available [[ 
> > https://github.com/intel/llvm/blob/c13cb8df84418cb5d682f3bbee89090ebb0d00be/clang/lib/AST/ASTContext.cpp#L10015
> >  | here ]] ) 
> > 
> Ok and what happens if some other target is used - not SPIR?
There will be no SPIR calling convention for device functions.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 203785.
Fznamznon added a comment.

Applied comments from @Anastasia

- Added link to documentation for `sycl_device` attribute
- Removed redundant comment from test

@Anastasia, do you have additional comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -125,6 +125,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () operator. spir_func calling convention
+// is missed for C++ methods.
+// CHECK: define internal void @"_ZZ4mainENK3$_0clEv"(%class.anon* %this)
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5517,14 +5517,30 @@
 Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) {
   InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, true,
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - they
+// have deferred instantination. We need bodies of these functions
+// so we are checking for SY

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@aaron.ballman , please let me know if you have additional 
comments/suggestions. If not, could you please accept this revision?


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Ping.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon 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:
> 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 ]] .




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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 205663.
Fznamznon added a comment.

Appled part of comments from @aaron.ballman:

- Fixed grammar and code style in all places except sycl_kernel docs
- Added a lit test which checks that sycl_device attribute implicitly added to 
proper declarations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp
  clang/test/SemaSYCL/device-code-outlining.cpp

Index: clang/test/SemaSYCL/device-code-outlining.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-code-outlining.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -ast-dump %s | FileCheck %s
+
+template 
+T bar(T arg);
+// CHECK: FunctionTemplateDecl {{.*}} bar
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+// CHECK: FunctionDecl {{.*}} foo
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+// CHECK: FunctionTemplateDecl {{.*}} kernel_single_task
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void host_foo() {
+  int b = 0;
+}
+// CHECK: FunctionDecl {{.*}} host_foo
+// CHECK-NOT: SYCLDeviceAttr
+// CHECK: FunctionDecl {{.*}} main
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  host_foo();
+  return 0;
+}
Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -125,6 +125,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}f

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:263-264
+entry point to device code i.e. will be called by host in run time.
+Here is a code example of the SYCL program, which demonstrates the need for
+this attribute:
+.. code-block:: c++

aaron.ballman wrote:
> This doesn't really demonstrate the need for the attribute -- the attribute 
> is never shown in the code example. I'd prefer an example that shows when and 
> how a user would write this attribute.
I see. I will update documentation in the next version. 


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 205813.
Fznamznon added a comment.

Updated `sycl_kernel` attribute documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp
  clang/test/SemaSYCL/device-code-outlining.cpp

Index: clang/test/SemaSYCL/device-code-outlining.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-code-outlining.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -ast-dump %s | FileCheck %s
+
+template 
+T bar(T arg);
+// CHECK: FunctionTemplateDecl {{.*}} bar
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+// CHECK: FunctionDecl {{.*}} foo
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+// CHECK: FunctionTemplateDecl {{.*}} kernel_single_task
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void host_foo() {
+  int b = 0;
+}
+// CHECK: FunctionDecl {{.*}} host_foo
+// CHECK-NOT: SYCLDeviceAttr
+// CHECK: FunctionDecl {{.*}} main
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  host_foo();
+  return 0;
+}
Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -125,6 +125,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () ope

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 205831.
Fznamznon added a comment.

Fixed a couple coding style issues, renamed markDevice function with 
markSYCLDevice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp
  clang/test/SemaSYCL/device-code-outlining.cpp

Index: clang/test/SemaSYCL/device-code-outlining.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-code-outlining.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -ast-dump %s | FileCheck %s
+
+template 
+T bar(T arg);
+// CHECK: FunctionTemplateDecl {{.*}} bar
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+// CHECK: FunctionDecl {{.*}} foo
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+// CHECK: FunctionTemplateDecl {{.*}} kernel_single_task
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void host_foo() {
+  int b = 0;
+}
+// CHECK: FunctionDecl {{.*}} host_foo
+// CHECK-NOT: SYCLDeviceAttr
+// CHECK: FunctionDecl {{.*}} main
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  host_foo();
+  return 0;
+}
Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute__((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -125,6 +125,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+/

[PATCH] D63710: [SYCL] Re-use OpenCL sampler in SYCL device mode

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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

sampler_t type name is replaced with __ocl_sampler_t to avoid
potential collisions with user types.
Selectively enabled a few OpenCL diagnostics for sampler type as some
diagnostics trigger on SYCL use cases.

For instance, OpenCL kernel in SYCL mode initializes lambda captures with
the kernel argument values. This initialization code for sampler emits
errors because OpenCL disallows sampler on the right hand side of the
binary operators - these are supposed to be used only by built-in
functions.

Another potential issue is the lambda object itself -
captured sampler is a member of the lambda object and OpenCL doesn't
allow composite types with samplers. SPIR-V produced from SYCL should be
okay as lambda object can be removed by standard LLVM transformation
passes.

Patch by Alexey Bader 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63710

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenSYCL/ocl_sampler.cpp
  clang/test/SemaSYCL/ocl_sampler.cpp

Index: clang/test/SemaSYCL/ocl_sampler.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/ocl_sampler.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -fsyntax-only -verify %s
+
+#define CLK_ADDRESS_CLAMP_TO_EDGE   2
+#define CLK_NORMALIZED_COORDS_TRUE  1
+#define CLK_FILTER_NEAREST  0x10
+#define CLK_FILTER_LINEAR   0x20
+
+__ocl_sampler_t global_nonconst_smp = 0; // expected-error {{global sampler requires a const or constant address space qualifier}}
+const __ocl_sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+
+void foo(__ocl_sampler_t argsmp) {
+  __ocl_sampler_t smp1 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  __ocl_sampler_t smp2 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST;
+  &smp1; //expected-error{{invalid argument type '__ocl_sampler_t' (aka 'sampler_t') to unary expression}}
+  *smp2; //expected-error{{invalid argument type '__ocl_sampler_t' (aka 'sampler_t') to unary expression}}
+
+  const __ocl_sampler_t const_smp6 = 0x1LL; // expected-error{{sampler_t initialization requires 32-bit integer, not 'long long'}}
+
+  __ocl_sampler_t sa[] = {argsmp, glb_smp}; // expected-error {{array of '__ocl_sampler_t' (aka 'sampler_t') type is invalid in OpenCL}}
+}
+
+void foo_smplr_ptr(__ocl_sampler_t*); // expected-error{{pointer to type '__ocl_sampler_t' (aka 'sampler_t') is invalid in OpenCL}}
Index: clang/test/CodeGenSYCL/ocl_sampler.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/ocl_sampler.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %opencl.sampler_t = type opaque
+
+void foo(__ocl_sampler_t s) {}
+// CHECK: define spir_func void @{{.*}}foo{{.*}}(%opencl.sampler_t addrspace(2)*
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2320,7 +2320,7 @@
   // OpenCL v2.0 s6.12.5 - Arrays of blocks are not supported.
   // OpenCL v2.0 s6.16.13.1 - Arrays of pipe type are not supported.
   // OpenCL v2.0 s6.9.b - Arrays of image/sampler type are not supported.
-  if (getLangOpts().OpenCL) {
+  if (getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 const QualType ArrType = Context.getBaseElementType(T);
 if (ArrType->isBlockPointerType() || ArrType->isPipeType() ||
 ArrType->isSamplerT() || ArrType->isImageType()) {
@@ -4407,7 +4407,7 @@
   // OpenCL v2.0 s6.9b - Pointer to image/sampler cannot be used.
   // OpenCL v2.0 s6.13.16.1 - Pointer to pipe cannot be used.
   // OpenCL v2.0 s6.12.5 - Pointers to Blocks are not allowed.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL || LangOpts.SYCLIsDevice) {
 if (T->isImageType() || T->isSamplerT() || T->isPipeType() ||
 T->isBlockPointerType()) {
   S.Diag(D.getIdentifierLoc(), diag::err_opencl_pointer_to_type) << T;
@@ -4605,7 +4605,7 @@
 }
   }
 
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL || LangOpts.SYCLIsDevice) {
 // OpenCL v2.0 s6.12.5 - A block cannot be the return value of a
 // function.
 if (T->isBlockPointerType() || T->isImageType() || T->isSamplerT() ||
@@ -4617,7 +4617,9 @@
 // OpenCL doesn't support variadic functions and blocks
 // (s6.9.e and s6.12.5 OpenCL v2.0) except 

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 206861.
Fznamznon added a comment.

Added warning diagnostic for `sycl_kernel` attribute.

Now if the `sycl_kernel` attribute applied to a function which doesn't meet 
requirements for OpenCL kernel generation, attribute will be ignored and 
diagnostic will be emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp
  clang/test/SemaSYCL/device-code-outlining.cpp

Index: clang/test/SemaSYCL/device-code-outlining.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-code-outlining.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -ast-dump %s | FileCheck %s
+
+template 
+T bar(T arg);
+// CHECK: FunctionTemplateDecl {{.*}} bar
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+// CHECK: FunctionDecl {{.*}} foo
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+// CHECK: FunctionTemplateDecl {{.*}} kernel_single_task
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void host_foo() {
+  int b = 0;
+}
+// CHECK: FunctionDecl {{.*}} host_foo
+// CHECK-NOT: SYCLDeviceAttr
+// CHECK: FunctionDecl {{.*}} main
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  host_foo();
+  return 0;
+}
Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+
+// Only template functions
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Both first two template parameters must be a typenames
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// No

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 206873.
Fznamznon added a comment.

Minor fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp
  clang/test/SemaSYCL/device-code-outlining.cpp

Index: clang/test/SemaSYCL/device-code-outlining.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-code-outlining.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++11 -fsycl-is-device -ast-dump %s | FileCheck %s
+
+template 
+T bar(T arg);
+// CHECK: FunctionTemplateDecl {{.*}} bar
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+// CHECK: FunctionDecl {{.*}} foo
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+// CHECK: FunctionTemplateDecl {{.*}} kernel_single_task
+// CHECK: SYCLDeviceAttr {{.*}} Implicit
+
+void host_foo() {
+  int b = 0;
+}
+// CHECK: FunctionDecl {{.*}} host_foo
+// CHECK-NOT: SYCLDeviceAttr
+// CHECK: FunctionDecl {{.*}} main
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  host_foo();
+  return 0;
+}
Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+
+// Only template functions
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Both first two template parameters must be a typenames
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+template 
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to template funtions with special prototype, please refer 'sycl_kernel' attribute documentation}}
+
+// No diagnosticts
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Ind

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 200513.
Fznamznon added a comment.
Herald added a subscriber: mgorny.

Added semantics for new attributes

- Added semantics for new attributes. Now complier can separate SYCL

device code from host code using new arrtributes.

- Removed spelling for sycl_device attribute and its documentation because

it can be added only implicitly by the compiler for now

- Removed docs for sycl_kernel attribute because this attribute is not

presented in SYCL spec and not for public use - it's some implemetation detail.
It will be used in SYCL headers implemetation to help compiler find device code
entry point in single source file. So I think no need to add documentation for
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling regular C++ file without SYCL mode enabled.
+// There should be warnings.
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () operator. spir_func calling convention
+// is missed for C++ methods.
+// CHECK: define internal void @"_ZZ4mainENK3$_0clEv"(%class.anon* %this)
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5517,14 +5517,30 @@
 Function, [

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 200658.
Fznamznon added a comment.

Minor fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling regular C++ file without SYCL mode enabled.
+// There should be warnings.
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () operator. spir_func calling convention
+// is missed for C++ methods.
+// CHECK: define internal void @"_ZZ4mainENK3$_0clEv"(%class.anon* %this)
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5517,14 +5517,30 @@
 Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) {
   InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, true,
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - they
+// have deferred instantination. We need bodies of these functions
+// so we are checking for SYCL kernel attribute after instantination.
+if (getLangOpts().SYCLIsDevice &&
+CurFD->ha

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



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

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?



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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling regular C++ file without SYCL mode enabled.

Anastasia wrote:
> Another confusion I have at the moment even though it doesn't belong to this 
> patch - isn't SYCL based on C++11?
Sorry for confusion. The C++ features used in SYCL are a subset of the C++11 
standard features.
I will add -std=c++11 key to run line to avoid such confusion in future.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 201641.
Fznamznon added a comment.

Applied comments from @Anastasia

- Added documentation for sycl_kernel function
- Added comments to Sema.h
- Added -std=c++11 to test run lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[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}}
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling regular C++ file without SYCL mode enabled.
+// There should be warnings.
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -124,6 +124,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () operator. spir_func calling convention
+// is missed for C++ methods.
+// CHECK: define internal void @"_ZZ4mainENK3$_0clEv"(%class.anon* %this)
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5517,14 +5517,30 @@
 Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) {
   InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, true,
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - they
+// have deferred instantination.

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 266066.
Fznamznon added a comment.
Herald added a subscriber: sstefan1.

Re-implemented diagnostic itself, now only usages of declarations
with unsupported types are diagnosed.
Generalized approach between OpenMP and SYCL.


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/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_PPC
-// expected-error@+4 {{host requires 1

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

The tests are failing because calling function with unsupported type in 
arguments/return value is diagnosed as well, i.e. :

  double math(float f, double d, long double ld) { ... } // `ld` is not used 
inside the `math` function
  #pragma omp target map(r)
{ r += math(f, d, ld); } // error: 'math' requires 128 bit size 'long 
double' type support, but device 'nvptx64-nvidia-cuda' does not support it

Should we diagnose calls to such functions even if those arguments/return value 
aren't used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 266877.
Fznamznon marked 2 inline comments as done.
Fznamznon added a comment.

Applied comments from Johannes.
Fixed failing tests.


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_PPC
-// expected-error@+4 {{host requires

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/Sema.cpp:1727
+
+  QualType Ty = D->getType();
+  auto CheckType = [&](QualType Ty) {

jdoerfert wrote:
> Nit: Move below `CheckType` to avoid shadowing and confusion with the arg 
> there. 
Done, thanks



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:21
   char c;
   T() : a(12), f(15) {}
   T &operator+(T &b) { f += b.a; return *this;}

jdoerfert wrote:
> Why is this not diagnosed? I mean we cannot assign 15 on the device, can we? 
> Or does it work because it is a constant (and we basically just memcpy 
> something)?
> 
> If it's the latter, do we have a test in the negative version that makes sure 
> `T(int i) : a(i), f(i) {}` is flagged?
Unfortunately, nor this case neither `T(int i) : a(i), f(i) {}` is not 
diagnosed. This happens because `DiagnoseUseOfDecl` call seems missing for 
member initializers, not because there is memcpy. So, for example, such case is 
diagnosed:
```
struct B {
  __float128 a;
};
#pragma omp declare target
void foo() {
  B var = {1}; // error: 'a' requires 128 bit size '__float128' type support, 
but device 'nvptx64-unknown-unknown' does not support it
}
```
`DiagnoseUseOfDecl` function is called in so many cases and I guess it is meant 
to be called on each usage of each declaration, that is why I think the correct 
fix is add call to `DiagnoseUseOfDecl` somewhere near building of member 
initializers . This change even doesn't break my local `check-clang` LIT tests 
run, but I'm not really sure that such change is in scope of this patch, 
because `DiagnoseUseOfDecl` contains a lot of other diagnostics as well.



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:81
-// CHECK: define weak void 
@__omp_offloading_{{.+}}foo{{.+}}_l75([[BIGTYPE:.+]]*
-// CHECK: store [[BIGTYPE]] 
{{0xL3FFF|0xM3FF0}}, 
[[BIGTYPE]]* %

jdoerfert wrote:
> Just checking, we verify in the other test this would result in an error, 
> right?
Yes, I added such test case in `nvptx_unsupported_type_messages.cpp` .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D77918: [OpenMP] Avoid crash in preparation for diagnose of unsupported type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D77918#2060446 , @jdoerfert wrote:

> I think this will be fixed by D74387 , we 
> should include the tests somewhere though.


D74387  includes a test case with expression 
involving unsupported types outside of a function (at the end of in 
`nvptx_unsupported_type_messages.cpp`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77918



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 266942.
Fznamznon marked an inline comment as done and an inline comment as not done.
Fznamznon added a comment.

Included test cases from Johannes.


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_PPC
-// expected-error@+4

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#2063423 , @ABataev wrote:

> Seems to me, this patch crashes 
> `llvm-project/openmp/libomptarget/test/mapping/declare_mapper_api.cpp`.


It seems this patch caused asking size of dependent type, AST context doesn't 
seem expecting it. I'll provide follow up fix shortly. Sorry for inconvenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: cfe-commits, sstefan1, Anastasia, ebevhan, guansong, 
yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Fznamznon added reviewers: ABataev, bader.

Do not ask size of type if it is dependent. ASTContext doesn't seem expecting
this.


Repository:
  rG LLVM Github Monorepo

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] D74387: [OpenMP][SYCL] Improve diagnosing of unsupported types usage

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Fix is here D80829  .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

I don't have rights to merge. Could someone submit it, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80829



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.
Herald added a subscriber: yaxunl.

In D74387#1950593 , @jdoerfert wrote:

> This is needed for OpenMP as well. Does it make sense to include it in this 
> patch or in another one?


I thought OpenMP already has diagnostics for unsupported types (at least 
looking into this commit 
https://github.com/llvm/llvm-project/commit/123ad1969171d0b22d0c5d0ec23468586c4d8fa7).
 Am I wrong?
The diagnostic which I'm implementing here is stricter than existing OpenMP 
diagnostic, the main goal is do not emit unsupported type at all. Does OpenMP 
need such restriction as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#1965634 , @jdoerfert wrote:

> In D74387#1964483 , @Fznamznon wrote:
>
> > In D74387#1950593 , @jdoerfert 
> > wrote:
> >
> > > This is needed for OpenMP as well. Does it make sense to include it in 
> > > this patch or in another one?
> >
> >
> > I thought OpenMP already has diagnostics for unsupported types (at least 
> > looking into this commit 
> > https://github.com/llvm/llvm-project/commit/123ad1969171d0b22d0c5d0ec23468586c4d8fa7).
> >  Am I wrong?
> >  The diagnostic which I'm implementing here is stricter than existing 
> > OpenMP diagnostic, the main goal is do not emit unsupported type at all. 
> > Does OpenMP need such restriction as well?
>
>
> OpenMP handling needs to be reverted/redone:
>
> 1. If no aux triple is available it just crashes.
> 2. If the unavailable type is not used in one of the pattern matched 
> expressions it crashes (usually during instruction selection but not always). 
> Try a call with long double arguments for example.
>
>   I'm not sure this patch fits the bill but what I was thinking we need is 
> roughly: If you have a expression with operands or function definition with 
> return/argument types which are not supported on the target, mark the 
> definition as unavailable with the type note you have. We should especially 
> allow members to have unavailable types if the member is not accessed. Memcpy 
> like operations (=mapping) are OK though. I think this should be the same for 
> OpenMP and Sycl (and HIP, and ...).


Why we should allow members to have unavailable types if the member is not 
accessed? I don't think that we always can do it, especially for SYCL. Even if 
the member is not accessed directly, the whole struct with unavailable type 
inside will get into resulting LLVM IR module anyway, this can be a problem, I 
guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#1967386 , @jdoerfert wrote:

> In D74387#1967289 , @Fznamznon wrote:
>
> > In D74387#1965634 , @jdoerfert 
> > wrote:
> >
> > > In D74387#1964483 , @Fznamznon 
> > > wrote:
> > >
> > > > In D74387#1950593 , @jdoerfert 
> > > > wrote:
> > > >
> > > > > This is needed for OpenMP as well. Does it make sense to include it 
> > > > > in this patch or in another one?
> > > >
> > > >
> > > > I thought OpenMP already has diagnostics for unsupported types (at 
> > > > least looking into this commit 
> > > > https://github.com/llvm/llvm-project/commit/123ad1969171d0b22d0c5d0ec23468586c4d8fa7).
> > > >  Am I wrong?
> > > >  The diagnostic which I'm implementing here is stricter than existing 
> > > > OpenMP diagnostic, the main goal is do not emit unsupported type at 
> > > > all. Does OpenMP need such restriction as well?
> > >
> > >
> > > OpenMP handling needs to be reverted/redone:
> > >
> > > 1. If no aux triple is available it just crashes.
> > > 2. If the unavailable type is not used in one of the pattern matched 
> > > expressions it crashes (usually during instruction selection but not 
> > > always). Try a call with long double arguments for example.
> > >
> > >   I'm not sure this patch fits the bill but what I was thinking we need 
> > > is roughly: If you have a expression with operands or function definition 
> > > with return/argument types which are not supported on the target, mark 
> > > the definition as unavailable with the type note you have. We should 
> > > especially allow members to have unavailable types if the member is not 
> > > accessed. Memcpy like operations (=mapping) are OK though. I think this 
> > > should be the same for OpenMP and Sycl (and HIP, and ...).
> >
> >
> > Why we should allow members to have unavailable types if the member is not 
> > accessed? I don't think that we always can do it, especially for SYCL. Even 
> > if the member is not accessed directly, the whole struct with unavailable 
> > type inside will get into resulting LLVM IR module anyway, this can be a 
> > problem, I guess.
>
>
> On the host you know how large the type is so you can replace it in the 
> device module with a placeholder of the appropriate size. You want to do this 
> (in OpenMP for sure) because things you map might have constitutes you don't 
> want to access on the device but you can also not (easily) split out of your 
> mapped type.


Okay, I see. Am I right that OpenMP already has such thing implemented, but 
only for functions return types? I suppose, for SYCL, we might need to replace 
unsupported type in device module everywhere...
BTW, one more question, we also have a diagnostic which is emitted on attempt 
to declare a variable with unsupported type inside the device code for this 
__float128 type and other ones (https://github.com/intel/llvm/pull/1465/files). 
Does OpenMP (and probably HIP, CUDA etc) need such diagnostic as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#1970374 , @jdoerfert wrote:

> In D74387#1969891 , @Fznamznon wrote:
>
> > In D74387#1967386 , @jdoerfert 
> > wrote:
> >
> > > In D74387#1967289 , @Fznamznon 
> > > wrote:
> > >
> > > > In D74387#1965634 , @jdoerfert 
> > > > wrote:
> > > >
> > > > > In D74387#1964483 , 
> > > > > @Fznamznon wrote:
> > > > >
> > > > > > In D74387#1950593 , 
> > > > > > @jdoerfert wrote:
> > > > > >
> > > > > > > This is needed for OpenMP as well. Does it make sense to include 
> > > > > > > it in this patch or in another one?
> > > > > >
> > > > > >
> > > > > > I thought OpenMP already has diagnostics for unsupported types (at 
> > > > > > least looking into this commit 
> > > > > > https://github.com/llvm/llvm-project/commit/123ad1969171d0b22d0c5d0ec23468586c4d8fa7).
> > > > > >  Am I wrong?
> > > > > >  The diagnostic which I'm implementing here is stricter than 
> > > > > > existing OpenMP diagnostic, the main goal is do not emit 
> > > > > > unsupported type at all. Does OpenMP need such restriction as well?
> > > > >
> > > > >
> > > > > OpenMP handling needs to be reverted/redone:
> > > > >
> > > > > 1. If no aux triple is available it just crashes.
> > > > > 2. If the unavailable type is not used in one of the pattern matched 
> > > > > expressions it crashes (usually during instruction selection but not 
> > > > > always). Try a call with long double arguments for example.
> > > > >
> > > > >   I'm not sure this patch fits the bill but what I was thinking we 
> > > > > need is roughly: If you have a expression with operands or function 
> > > > > definition with return/argument types which are not supported on the 
> > > > > target, mark the definition as unavailable with the type note you 
> > > > > have. We should especially allow members to have unavailable types if 
> > > > > the member is not accessed. Memcpy like operations (=mapping) are OK 
> > > > > though. I think this should be the same for OpenMP and Sycl (and HIP, 
> > > > > and ...).
> > > >
> > > >
> > > > Why we should allow members to have unavailable types if the member is 
> > > > not accessed? I don't think that we always can do it, especially for 
> > > > SYCL. Even if the member is not accessed directly, the whole struct 
> > > > with unavailable type inside will get into resulting LLVM IR module 
> > > > anyway, this can be a problem, I guess.
> > >
> > >
> > > On the host you know how large the type is so you can replace it in the 
> > > device module with a placeholder of the appropriate size. You want to do 
> > > this (in OpenMP for sure) because things you map might have constitutes 
> > > you don't want to access on the device but you can also not (easily) 
> > > split out of your mapped type.
> >
> >
> > Okay, I see. Am I right that OpenMP already has such thing implemented, but 
> > only for functions return types? I suppose, for SYCL, we might need to 
> > replace unsupported type in device module everywhere...
> >  BTW, one more question, we also have a diagnostic which is emitted on 
> > attempt to declare a variable with unsupported type inside the device code 
> > for this __float128 type and other ones 
> > (https://github.com/intel/llvm/pull/1465/files). Does OpenMP (and probably 
> > HIP, CUDA etc) need such diagnostic as well?
>
>
> I'm not sure we want this and I'm not sure why you would. To me, it seems 
> user hostile to disallow unsupported types categorically. We also know from 
> our codes that people have unsupported types in structs that they would 
> rather not refactor. Given that there is not really a need for this anyway, 
> why should we make them? Arguably you cannot "use" unsupported types but an 
> error like that makes sense to people. So as long as you don't use the 
> unsupported type as an operand in an expression you should be fine.
>
> We have some detection for this in clang for OpenMP but it is not sufficient. 
> We also should generalize this (IMHO) and stop duplicating logic between 
> HIP/CUDA/OpenMP/SYCL/... That said, we cannot error out because the types are 
> present but only if they are used. I would hope you would reconsider and do 
> the same. Arguably, mapping/declaring a unsupported type explicitly could be 
> diagnosed (with a warning) but as part of a struct I would advice against.
>
> Maybe I just don't understand. Could you elaborate why you think sycl has to 
> forbid them categorically?


Roughly speaking, SYCL is a wrapper over OpenCL. SYCL device compiler should be 
able to produce device code module in a form acceptable by OpenCL backends. For 
this purpose we use SPIR-V intermediate language 
(https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html). 

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#1974981 , @jdoerfert wrote:

> As I mentioned before. As long as the type is not "used" you can treat it as 
> a sequence of bytes just as well. So we can lower `__float128` to `char [16]` 
> with the right alignment. SPIRV will never see unsupported types and the code 
> works because we never access it as `float128` anyway. WDYT?


Yes, it can work for SYCL without additional diagnostics if it is possible to 
replace `__float128` with `char [16]` everywhere (including struct definitions 
and so on) in the resulting LLVM IR module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D77918: [OpenMP] Avoid crash in preparation for diagnose of unsupported type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1831
  "OpenMP device compilation mode is expected.");
+  // TODO: Do not check outside of functions for now as the targetDiag below
+  //   requires one.

Please ignore my comment if it says nonsense. It actually can since I don't 
know a lot about OpenMP but,
looking into `targetDiag`, I would say that underlying function 
(`diagIfOpenMPDeviceCode` in case of OpenMP device mode) should check that we 
actually have current function because it does `getCurFunctionDecl()` call.
Otherwise any attempt to emit diagnostic in OpenMP device mode using 
`targetDiag` or `diagIfOpenMPDeviceCode` will fail outside of functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77918



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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a subscriber: ABataev.
Fznamznon added a comment.

@jdoerfert , @ABataev , if OpenMP needs same diagnostic as well, I can 
generalize it between SYCL and OpenMP.


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] Implement thread-local storage restriction

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: cfe-commits, sstefan1, Anastasia, ebevhan, yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Fznamznon added reviewers: erichkeane, bader.
Fznamznon added a subscriber: ABataev.
Fznamznon added a comment.

@jdoerfert , @ABataev , if OpenMP needs same diagnostic as well, I can 
generalize it between SYCL and OpenMP.


The SYCL spec prohibits thread local storage in device code,
so this commit ensures an error is emitted for device code and not
emitted for host code when host target supports it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81641

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -212,6 +212,11 @@
  bool ObjCPropertyAccess,
  bool AvoidPartialAvailabilityChecks,
  ObjCInterfaceDecl *ClassReceiver) {
+  if (getLangOpts().SYCLIsDevice)
+if (auto VD = dyn_cast(D))
+  if (VD->getTLSKind() != VarDecl::TLS_None)
+SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_thread_unsupported);
+
   SourceLocation Loc = Locs.front();
   if (getLangOpts().CPlusPlus && isa(D)) {
 // If there were any diagnostics suppressed by template argument deduction,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7077,7 +7077,8 @@
diag::err_thread_non_global)
 << DeclSpec::getSpecifierName(TSCS);
 else if (!Context.getTargetInfo().isTLSSupported()) {
-  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice) {
+  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  getLangOpts().SYCLIsDevice) {
 // Postpone error emission until we've collected attributes required to
 // figure out whether it's a host or device variable and whether the
 // error should be ignored.
@@ -7179,13 +7180,17 @@
   // Handle attributes prior to checking for duplicates in MergeVarDecl
   ProcessDeclAttributes(S, NewVD, D);
 
-  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice) {
+  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  getLangOpts().SYCLIsDevice) {
 if (EmitTLSUnsupportedError &&
 ((getLangOpts().CUDA && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) ||
  (getLangOpts().OpenMPIsDevice &&
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(NewVD
   Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
diag::err_thread_unsupported);
+
+if (EmitTLSUnsupportedError && getLangOpts().SYCLIsDevice)
+  SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_thread_unsupported);
 // CUDA B.2.5: "__shared__ and __constant__ variables have implied static
 // storage [duration]."
 if (SC == SC_None && S->getFnParent() != nullptr &&
___
cfe-commits mailing list
cfe-commits@lists.

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 270135.
Fznamznon added a comment.

Fixed code style.


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/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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -212,6 +212,11 @@
  bool ObjCPropertyAccess,
  bool AvoidPartialAvailabilityChecks,
  ObjCInterfaceDecl *ClassReceiver) {
+  if (getLangOpts().SYCLIsDevice)
+if (auto *VD = dyn_cast(D))
+  if (VD->getTLSKind() != VarDecl::TLS_None)
+SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_thread_unsupported);
+
   SourceLocation Loc = Locs.front();
   if (getLangOpts().CPlusPlus && isa(D)) {
 // If there were any diagnostics suppressed by template argument deduction,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7077,7 +7077,8 @@
diag::err_thread_non_global)
 << DeclSpec::getSpecifierName(TSCS);
 else if (!Context.getTargetInfo().isTLSSupported()) {
-  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice) {
+  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  getLangOpts().SYCLIsDevice) {
 // Postpone error emission until we've collected attributes required to
 // figure out whether it's a host or device variable and whether the
 // error should be ignored.
@@ -7179,13 +7180,17 @@
   // Handle attributes prior to checking for duplicates in MergeVarDecl
   ProcessDeclAttributes(S, NewVD, D);
 
-  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice) {
+  if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  getLangOpts().SYCLIsDevice) {
 if (EmitTLSUnsupportedError &&
 ((getLangOpts().CUDA && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) ||
  (getLangOpts().OpenMPIsDevice &&
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(NewVD
   Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
diag::err_thread_unsupported);
+
+if (EmitTLSUnsupportedError && getLangOpts().SYCLIsDevice)
+  SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_thread_unsupported);
 // CUDA B.2.5: "__shared__ and __constant__ variables have implied static
 // storage [duration]."
 if (SC == SC_None && S->getFnParent() != nullptr &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:216
+  if (getLangOpts().SYCLIsDevice)
+if (auto VD = dyn_cast(D))
+  if (VD->getTLSKind() != VarDecl::TLS_None)

riccibruno wrote:
> Nit: The convention is `auto *VD`.
Fixed, thanks!


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 271133.
Fznamznon marked an inline comment as done.
Fznamznon added a comment.

Generalized diagnostic between SYCL and OpenMP.


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{{called by}}
+#pragma omp targe

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D81641#2088446 , @jdoerfert wrote:

> OpenMP has the same restriction (no surprise I guess). Thanks for the ping!
>
> I think we do not emit diagnosis right now: https://godbolt.org/z/srDkXZ
>  I think we also should diagnose this the same way, though it might be beyond 
> the scope of this patch: https://godbolt.org/z/rRZFi4


Alright, now the first case that you pointed is diagnosed as well.
The second case is not diagnosed at all and I agree that it seems beyond the 
scope of this patch, because it looks like using of `#pragma omp task untied` 
doesn't trigger emission of deferred diagnostics.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

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.


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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 271327.
Fznamznon edited the summary of this revision.
Fznamznon added a comment.

Rebased.


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{{called by}}
+#pragma omp target
+  usage();
+  return 0;
+}
Index: clang/lib/Sema/S

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Okay, seems like OpenMP needs unsupported types diagnosing as well. I'm trying 
to adapt this patch for OpenMP, but it doesn't work out of the box because it 
diagnoses memcpy like operations, so with the current patch the code like this 
will cause diagnostics:

   struct T {
 char a;
 __float128 f;
 char c;
 T() : a(12), f(15) {}
  }
  
  #pragma omp declare target
  T a = T();
  #pragma omp end declare target

It happens because member initialization in the constructor is still usage of 
`f` field which is marked unavailable because of type. I'm not sure that it is 
possible to understand how the unavailable declaration is used in the place 
where diagnostic about usage of unavailable declaration is actually emitted, so 
I will probably need some other place/solution for it.

@jdoerfert , could you please help to understand how the diagnostic should work 
for OpenMP cases? Or you probably have some spec/requirements for it?
Understanding what exactly is needed will help with the implementation, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.
Herald added a reviewer: aaron.ballman.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D74387#2023200 , @jdoerfert wrote:

> In D74387#1992682 , @Fznamznon wrote:
>
> > Okay, seems like OpenMP needs unsupported types diagnosing as well. I'm 
> > trying to adapt this patch for OpenMP, but it doesn't work out of the box 
> > because it diagnoses memcpy like operations, so with the current patch the 
> > code like this will cause diagnostics:
> >
> >struct T {
> >  char a;
> >  __float128 f;
> >  char c;
> >  T() : a(12), f(15) {}
> >   }
> >  
> >   #pragma omp declare target
> >   T a = T();
> >   #pragma omp end declare target
> >
> >
> > It happens because member initialization in the constructor is still usage 
> > of `f` field which is marked unavailable because of type. I'm not sure that 
> > it is possible to understand how the unavailable declaration is used in the 
> > place where diagnostic about usage of unavailable declaration is actually 
> > emitted, so I will probably need some other place/solution for it.
> >
> > @jdoerfert , could you please help to understand how the diagnostic should 
> > work for OpenMP cases? Or you probably have some spec/requirements for it?
> >  Understanding what exactly is needed will help with the implementation, I 
> > guess.
>
>
> I missed this update, sorry.
>
> I don't think we have a spec wording for this, it is up to the 
> implementations.
>
> In the example, a diagnostic is actually fine (IMHO). You cannot assign 15 to 
> the __float128 on the device. It doesn't work. The following code however 
> should go through without diagnostic:
>
>   struct T {
>  char a;
>  __float128 f;
>  char c;
>  T() : a(12), c(15) {}
>   }
>
>
> and it should translate to
>
>   struct T {
>  char a;
>  alignas(host_float128_alignment) char[16] __unavailable_f;
>  char c;
>  T() : a(12), c(15) {}
>   }
>
>
> Do you have other questions or examples we should discuss?


I'm not sure that I've discovered all examples and problems, but I have a 
couple of ones. I started with adapting current implementation for OpenMP and 
right now I'm analyzing corresponding OpenMP test fails (i.e. 
`clang/test/OpenMP/nvptx_unsupported_type_messages.cpp` and 
`clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp`). There are a lot of 
differences between the old approach and new one, which I'm working on. The new 
diagnostic triggers more errors than the old one, so I'd like to understand in 
which concrete cases we shouldn't emit diagnostic. For example you mentioned 
that memcopy-like operations should be ok in device code.
Right now the current implementation of the diagnostic also emits errors for 
sample like this:

  struct T {
char a;
__float128 f;
char c;
  };
  
  #pragma omp declare target
  T a;
  T b = a; // The diagnostic is triggered here, because implicit copy 
constructor uses unavailable field
  #pragma omp end declare target

Should we emit errors in such case too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Hi there,

This change seems to be causing assertion failure in clang when a struct 
contains a _BitInt with length longer than 128 - 
https://godbolt.org/z/4jTrW4fcP .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86310

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

[PATCH] D122494: Do not treat use of variable from attribute arguments as ODR use

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Attribute arguments are always compile-time expressions that need to be
evaluated. So, it is not necessary to treat attribute use as ODR use.

Treatment of use from attribute aruments as ODR use had a side
effect in incorrect processing of attributes applied to nested lambdas.
That happened because ODR use from attribute applied to a lambda forced
Sema to check if used variable is correctly captured.
For the following example:

  const int I = 10;
  auto OL = [=] () {
auto IL = [&](int K) __attribute__((enable_if(I > K, "..."))) {};
  };

when capture-checking function was validating use of either variable `I` or
parameter `K` the error "variable cannot be implicitly captured in a lambda with
no capture-default specified" was emitted. That happened due to the
following order of actions during lambda parsing: first, an empty lambda scope
is pushed to `FunctionScopes` collection in `Sema`, then the attributes on
lambdas are handled, and only after that the info about inner lambda capturing
style is saved to a corresponding lambda scope. So, at the point when
attributes on inner lambda are handled, capture-checking function thinks
that it is already inside the inner lambda and it doesn't have capturing
style, so the error was emitted.

This patch makes this error go away.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122494

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/attr-on-lambda.cpp

Index: clang/test/SemaCXX/attr-on-lambda.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-on-lambda.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+constexpr int get(const int A) { return A; }
+
+struct MyStruct {
+  const int Member = get(1);
+};
+
+void foo() {
+
+  const int I = 10;
+  constexpr MyStruct S = {1};
+  auto Outer = [=]() {
+auto Inner1 = [&](int k) __attribute__((enable_if(I > k, "nope"))){};
+
+auto Inner2 = [=]() __attribute__((enable_if(S.Member, "nope"))){};
+
+auto Inner3 = [&](int k) __attribute__((enable_if(k > get(I), "nope"))){};
+  };
+}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2031,6 +2031,8 @@
 !isCapturingReferenceToHostVarInCUDADeviceLambda(*this, VD) &&
 VD->isUsableInConstantExpressions(Context))
   return NOUR_Constant;
+if (ExprEvalContexts.back().InAttributeArgsContext)
+  return NOUR_Constant;
   }
 
   // All remaining non-variable cases constitute an odr-use. For variables, we
@@ -16932,12 +16934,13 @@
   return TransformToPE(*this).TransformType(TInfo);
 }
 
-void
-Sema::PushExpressionEvaluationContext(
+void Sema::PushExpressionEvaluationContext(
 ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl,
-ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
+ExpressionEvaluationContextRecord::ExpressionKind ExprContext,
+bool InAttributeArgs) {
   ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
-LambdaContextDecl, ExprContext);
+LambdaContextDecl, ExprContext,
+InAttributeArgs);
 
   // Discarded statements and immediate contexts nested in other
   // discarded statements or immediate context are themselves
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -444,9 +444,11 @@
   // General case. Parse all available expressions.
   bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
   EnterExpressionEvaluationContext Unevaluated(
-  Actions, Uneval
-   ? Sema::ExpressionEvaluationContext::Unevaluated
-   : Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  Actions,
+  Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
+ : Sema::ExpressionEvaluationContext::ConstantEvaluated,
+  nullptr, Sema::ExpressionEvaluationContextRecord::EK_Other, true,
+  /* InAttributeArgs = */ true);
 
   CommaLocsTy CommaLocs;
   ExprVector ParsedExprs;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1311,15 +1311,19 @@
 bool InDiscardedStatement;
 bool InImmediateFunctionContext;
 
+bool InAttributeArgsContext;
+
 ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
 

[PATCH] D118837: Methods visited for a special class must have an identifier.

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.
Herald added a project: All.

Was this one submitted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118837

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

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> I misread what was done with addrspace_cast, that new operator only allows 
> conversions that are otherwise also allowed. Based on that, this change does 
> actually align with what was done for OpenCL mode, it does not restrict 
> anything that is allowed in OpenCL mode. It does make sense, then. A slightly 
> more verbose commit message might have helped though :)

Right, the intention of this change is to re-use OpenCL mode logic. There is no 
connection between addrspace_cast operator and this change. In fact, there is 
no such operator in SYCL.

> Even better, some comments in the code explaining the "why" would have helped.

I was under impression that the change is small and therefore easy to 
understand. Would some comment like "SYCL re-uses OpenCL mode diagnostics to 
emit errors in case the cast happens between disjoint address spaces" help?


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] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Fznamznon added a reviewer: bader.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118935

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGenSYCL/address-space-conversions.cpp
  clang/test/SemaSYCL/address-space-conversions.cpp


Index: clang/test/SemaSYCL/address-space-conversions.cpp
===
--- clang/test/SemaSYCL/address-space-conversions.cpp
+++ clang/test/SemaSYCL/address-space-conversions.cpp
@@ -55,6 +55,9 @@
   baz(NoAS);   // expected-error {{no matching 
function for call to 'baz'}}
   __attribute__((opencl_local)) int *l = NoAS; // expected-error {{cannot 
initialize a variable of type '__local int *' with an lvalue of type 'int *'}}
 
+  // Explicit casts between disjoint address spaces are disallowed
+  GLOB = (__attribute__((opencl_global)) int *)PRIV; // expected-error 
{{C-style cast from '__private int *' to '__global int *' converts between 
mismatching address spaces}}
+
   (void)static_cast(GLOB);
   (void)static_cast(GLOB);
   int *i = GLOB;
Index: clang/test/CodeGenSYCL/address-space-conversions.cpp
===
--- clang/test/CodeGenSYCL/address-space-conversions.cpp
+++ clang/test/CodeGenSYCL/address-space-conversions.cpp
@@ -35,7 +35,7 @@
   __attribute__((opencl_global_host)) int *GLOBHOST;
 
   // Explicit conversions
-  // From names address spaces to default address space
+  // From named address spaces to default address space
   // CHECK-DAG: [[GLOB_LOAD:%[a-zA-Z0-9]+]] = load i32 addrspace(1)*, i32 
addrspace(1)* addrspace(4)* [[GLOB]].ascast
   // CHECK-DAG: [[GLOB_CAST:%[a-zA-Z0-9]+]] = addrspacecast i32 addrspace(1)* 
[[GLOB_LOAD]] to i32 addrspace(4)*
   // CHECK-DAG: store i32 addrspace(4)* [[GLOB_CAST]], i32 addrspace(4)* 
addrspace(4)* [[NoAS]].ascast
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2545,7 +2545,7 @@
 static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
  QualType DestType, bool CStyle,
  unsigned &msg, CastKind &Kind) {
-  if (!Self.getLangOpts().OpenCL)
+  if (!Self.getLangOpts().OpenCL && !Self.getLangOpts().SYCLIsDevice)
 // FIXME: As compiler doesn't have any information about overlapping addr
 // spaces at the moment we have to be permissive here.
 return TC_NotApplicable;


Index: clang/test/SemaSYCL/address-space-conversions.cpp
===
--- clang/test/SemaSYCL/address-space-conversions.cpp
+++ clang/test/SemaSYCL/address-space-conversions.cpp
@@ -55,6 +55,9 @@
   baz(NoAS);   // expected-error {{no matching function for call to 'baz'}}
   __attribute__((opencl_local)) int *l = NoAS; // expected-error {{cannot initialize a variable of type '__local int *' with an lvalue of type 'int *'}}
 
+  // Explicit casts between disjoint address spaces are disallowed
+  GLOB = (__attribute__((opencl_global)) int *)PRIV; // expected-error {{C-style cast from '__private int *' to '__global int *' converts between mismatching address spaces}}
+
   (void)static_cast(GLOB);
   (void)static_cast(GLOB);
   int *i = GLOB;
Index: clang/test/CodeGenSYCL/address-space-conversions.cpp
===
--- clang/test/CodeGenSYCL/address-space-conversions.cpp
+++ clang/test/CodeGenSYCL/address-space-conversions.cpp
@@ -35,7 +35,7 @@
   __attribute__((opencl_global_host)) int *GLOBHOST;
 
   // Explicit conversions
-  // From names address spaces to default address space
+  // From named address spaces to default address space
   // CHECK-DAG: [[GLOB_LOAD:%[a-zA-Z0-9]+]] = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(4)* [[GLOB]].ascast
   // CHECK-DAG: [[GLOB_CAST:%[a-zA-Z0-9]+]] = addrspacecast i32 addrspace(1)* [[GLOB_LOAD]] to i32 addrspace(4)*
   // CHECK-DAG: store i32 addrspace(4)* [[GLOB_CAST]], i32 addrspace(4)* addrspace(4)* [[NoAS]].ascast
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2545,7 +2545,7 @@
 static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
  QualType DestType, bool CStyle,
  unsigned &msg, CastKind &Kind) {
-  if (!Self.getLangOpts().OpenCL)
+  if (!Self.getLangOpts().OpenCL && !Self.getLangOpts().SYCLIsDevice)
 // FIXME: As compiler doesn't have any information

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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1cee9608982a: [SYCL] Disallow explicit casts between 
mismatching address spaces (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118935

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGenSYCL/address-space-conversions.cpp
  clang/test/SemaSYCL/address-space-conversions.cpp


Index: clang/test/SemaSYCL/address-space-conversions.cpp
===
--- clang/test/SemaSYCL/address-space-conversions.cpp
+++ clang/test/SemaSYCL/address-space-conversions.cpp
@@ -55,6 +55,9 @@
   baz(NoAS);   // expected-error {{no matching 
function for call to 'baz'}}
   __attribute__((opencl_local)) int *l = NoAS; // expected-error {{cannot 
initialize a variable of type '__local int *' with an lvalue of type 'int *'}}
 
+  // Explicit casts between disjoint address spaces are disallowed
+  GLOB = (__attribute__((opencl_global)) int *)PRIV; // expected-error 
{{C-style cast from '__private int *' to '__global int *' converts between 
mismatching address spaces}}
+
   (void)static_cast(GLOB);
   (void)static_cast(GLOB);
   int *i = GLOB;
Index: clang/test/CodeGenSYCL/address-space-conversions.cpp
===
--- clang/test/CodeGenSYCL/address-space-conversions.cpp
+++ clang/test/CodeGenSYCL/address-space-conversions.cpp
@@ -35,7 +35,7 @@
   __attribute__((opencl_global_host)) int *GLOBHOST;
 
   // Explicit conversions
-  // From names address spaces to default address space
+  // From named address spaces to default address space
   // CHECK-DAG: [[GLOB_LOAD:%[a-zA-Z0-9]+]] = load i32 addrspace(1)*, i32 
addrspace(1)* addrspace(4)* [[GLOB]].ascast
   // CHECK-DAG: [[GLOB_CAST:%[a-zA-Z0-9]+]] = addrspacecast i32 addrspace(1)* 
[[GLOB_LOAD]] to i32 addrspace(4)*
   // CHECK-DAG: store i32 addrspace(4)* [[GLOB_CAST]], i32 addrspace(4)* 
addrspace(4)* [[NoAS]].ascast
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2545,7 +2545,7 @@
 static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
  QualType DestType, bool CStyle,
  unsigned &msg, CastKind &Kind) {
-  if (!Self.getLangOpts().OpenCL)
+  if (!Self.getLangOpts().OpenCL && !Self.getLangOpts().SYCLIsDevice)
 // FIXME: As compiler doesn't have any information about overlapping addr
 // spaces at the moment we have to be permissive here.
 return TC_NotApplicable;


Index: clang/test/SemaSYCL/address-space-conversions.cpp
===
--- clang/test/SemaSYCL/address-space-conversions.cpp
+++ clang/test/SemaSYCL/address-space-conversions.cpp
@@ -55,6 +55,9 @@
   baz(NoAS);   // expected-error {{no matching function for call to 'baz'}}
   __attribute__((opencl_local)) int *l = NoAS; // expected-error {{cannot initialize a variable of type '__local int *' with an lvalue of type 'int *'}}
 
+  // Explicit casts between disjoint address spaces are disallowed
+  GLOB = (__attribute__((opencl_global)) int *)PRIV; // expected-error {{C-style cast from '__private int *' to '__global int *' converts between mismatching address spaces}}
+
   (void)static_cast(GLOB);
   (void)static_cast(GLOB);
   int *i = GLOB;
Index: clang/test/CodeGenSYCL/address-space-conversions.cpp
===
--- clang/test/CodeGenSYCL/address-space-conversions.cpp
+++ clang/test/CodeGenSYCL/address-space-conversions.cpp
@@ -35,7 +35,7 @@
   __attribute__((opencl_global_host)) int *GLOBHOST;
 
   // Explicit conversions
-  // From names address spaces to default address space
+  // From named address spaces to default address space
   // CHECK-DAG: [[GLOB_LOAD:%[a-zA-Z0-9]+]] = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(4)* [[GLOB]].ascast
   // CHECK-DAG: [[GLOB_CAST:%[a-zA-Z0-9]+]] = addrspacecast i32 addrspace(1)* [[GLOB_LOAD]] to i32 addrspace(4)*
   // CHECK-DAG: store i32 addrspace(4)* [[GLOB_CAST]], i32 addrspace(4)* addrspace(4)* [[NoAS]].ascast
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2545,7 +2545,7 @@
 static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
  QualType DestType, bool CStyle,
  unsigned &msg, CastKind &Kind) {
-  if (!Self.getLangOpts().OpenCL)
+  if (!Self.getLangOpts().OpenCL && !Self.getLangOp

[PATCH] D157879: [clang] Report missing designated initializers in C++

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D157879#4655321 , @rsmith wrote:

> This change has introduced a false positive for anonymous union members:
>
>   struct A {
>   int m;
>   union { int n = 0; };
>   };
>   
>   A a = A{.m = 0};
>
> now produces a false positive warning saying that the anonymous union member 
> in `A` is uninitialized.

Thanks! I've created https://github.com/llvm/llvm-project/issues/70444 and will 
figure something to fix this asap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157879

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

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1798
+  FunctionDecl *FD =
+  isa(C) ? cast(C) : dyn_cast(D);
   auto CheckType = [&](QualType Ty) {

So, we assume that lexical context is a function and if it is not, we assume 
that the value declaration being checked is a function. Can we actually break 
this assumption?
For example, will something like this work correctly:
```
struct S {
__float128 A = 1;
int bar = 2 * A;
};
#pragma omp declare target
  S s;
#pragma omp end declare target
```



Comment at: clang/lib/Sema/Sema.cpp:1835
   }
+  if (const auto *FPTy = dyn_cast(Ty))
+CheckType(FPTy->getReturnType());

I was a bit confused by `FPTy` variable name and already started writing a 
comment that we already checked a case `FunctionProtoType` case, then I 
realized that it is a check for Function*No*Prototype :) . Can we change FPTy 
to something closer to `FunctionNoProtoType` ?



Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:42
 #ifndef _ARCH_PPC
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size '__float128' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}

So,  `boo` is diagnosed two times, at the point where it actually used and 
where it is defined, it looks a bit like a duplication isn't it? I think it 
makes sense to diagnose only usage point with note pointing to the definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds diagnosing on attempt to use zero length arrays, pointers, refs, arrays
of them and structs/classes containing all of it.
A couple of assertions were removed from SemaSYCL because the new
diagnostics are deferred and emitted later than SemaSYCL routines.
In case a struct/class with zero length array is used this emits a set
of notes pointing out how zero length array got into used struct, like
this:

  struct ContainsArr {
int A[0]; // note: field of illegal type declared here
  };
  struct Wrapper {
ContainsArr F; // note: within field of type ContainsArr declared here
// ...
  }
  
  // Device code
  Wrapper W;
  W.use(); // error: zero-length arrays are not permitted

Total deep check of each used declaration may result in double
diagnosing at the same location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114080

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/test/SemaSYCL/zero-length-arrays.cpp

Index: clang/test/SemaSYCL/zero-length-arrays.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/zero-length-arrays.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -fsyntax-only -verify %s
+//
+// This test checks if compiler reports compilation error on an attempt to use
+// a zero-length array inside device code.
+
+template 
+__attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc(); // #KernelObjCall
+}
+
+typedef float ZEROARR[0];
+
+struct Wrapper {
+  int A;
+  int BadArray[0]; // expected-note 3{{field of illegal type 'int[0]' declared here}}
+};
+
+struct WrapperOfWrapper { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  Wrapper F;  // expected-note 2{{within field of type 'Wrapper' declared here}}
+  ZEROARR *Ptr;   //expected-note 5{{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+};
+
+template  struct InnerTemplated {
+  double Array[Size]; // expected-note 8{{field of illegal type 'double[0]' declared here}}
+};
+
+template  struct Templated {
+  unsigned A;
+  Ty Arr[Size];
+  InnerTemplated Array[Size + 1]; // expected-note 8{{within field of type 'InnerTemplated<0U>[1]' declared here}}
+};
+
+struct KernelSt {
+  int A;
+  int BadArray[0]; // expected-note {{field of illegal type 'int[0]' declared here}}
+  void operator()() const {}
+};
+
+WrapperOfWrapper offendingFoo() {
+  // expected-note@+1 {{called by 'offendingFoo'}}
+  return WrapperOfWrapper{};
+}
+
+template 
+void templatedContext() {
+  Templated Var;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@#KernelObjCall {{called by 'kernel([=] {
+// expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+(void)Var; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+  kernel([Var] {
+  });
+}
+
+void foo(const unsigned X) {
+  int Arr[0];  // expected-note 2{{declared here}}
+  ZEROARR TypeDef; // expected-note {{declared here}}
+  ZEROARR *Ptr;// expected-note {{declared here}}
+   // expected-error@#KernelObjCall 3{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=]() {
+(void)Arr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+(void)TypeDef; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+// expected-note@+1 {{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+(void)Ptr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{field of illegal type 'int[0]' declared here}}
+  kernel([Arr] { // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  WrapperOfWrapper St;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=] 

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@jdoerfert , @yaxunl , please let me know if similar thing would be useful for 
OMP/HIP/CUDA as well. I can generalize it by using `targetDiag`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D114080#3137430 , @Naghasan wrote:

> Why the need for this restriction ?

Zero length arrays are not supported by pure C++. It is an extension supported 
by clang. They are also not allowed in OpenCL and SPIR-V, and I even though I 
cannot grep an explicit restriction for them in SYCL 2020 spec (I guess that is 
just because it is an extension and not pure C++) It doesn't make sense to 
allow them here since memory allocation in device code is prohibited in SYCL by 
the spec. This diagnostic prevents further not always user friendly errors 
coming from SPIR-V emitters or from backends like OpenCL that explicitly 
disallow them on early stage and in user-friendly form. I believe other GPU 
programming models also may not allow them for the same reason - restriction on 
memory allocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> I agree, but having a field defined is different from it being used. It is 
> actually not uncommon to see SYCL functor passed with many unused fields 
> (Eigen is good example).

Are you saying that it is better to diagnose just used fields?
I believe it is safer to diagnose everything, because some backends or SPIR-V 
emitters may not expect that and crash or throw non-user friendly errors, 
because usually it is up to front-end to emit errors when some unsupported 
feature appears in device code (even unused).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 390346.
Fznamznon marked an inline comment as done.
Fznamznon added a comment.

Apply suggestion from @bader


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

Files:
  clang/lib/Sema/SemaSYCL.cpp


Index: clang/lib/Sema/SemaSYCL.cpp
===
--- clang/lib/Sema/SemaSYCL.cpp
+++ clang/lib/Sema/SemaSYCL.cpp
@@ -122,8 +122,8 @@
   NeedToEmitNotes = false;
 }
 
-// In case pointer/array/reference type is met get pointeetype, then 
proceed
-// with that type.
+// In case pointer/array/reference type is met get pointee type, then
+// proceed with that type.
 while (NextTy->isAnyPointerType() || NextTy->isArrayType() ||
NextTy->isReferenceType()) {
   if (NextTy->isArrayType())


Index: clang/lib/Sema/SemaSYCL.cpp
===
--- clang/lib/Sema/SemaSYCL.cpp
+++ clang/lib/Sema/SemaSYCL.cpp
@@ -122,8 +122,8 @@
   NeedToEmitNotes = false;
 }
 
-// In case pointer/array/reference type is met get pointeetype, then proceed
-// with that type.
+// In case pointer/array/reference type is met get pointee type, then
+// proceed with that type.
 while (NextTy->isAnyPointerType() || NextTy->isArrayType() ||
NextTy->isReferenceType()) {
   if (NextTy->isArrayType())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon marked an inline comment as not done.
Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaSYCL.cpp:68-75
+bool ErrorFound = false;
+if (isZeroSizedArray(*this, TypeToCheck)) {
+  SYCLDiagIfDeviceCode(UsedAt, diag::err_sycl_zero_array_size);
+  ErrorFound = true;
+}
+// Checks for other types can also be done here.
+if (ErrorFound) {

bader wrote:
> 
I'm not sure about this suggestion. I've made this place this way so It is easy 
to extend it to diagnose other types (for example variable length arrays). Is 
it ok if I leave it as is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D114080#3158323 , @erichkeane 
wrote:

> I note that this is missing a test, otherwise I don't see any issues with it 
> from my end.

There is a test SemaSYCL/zero-length-arrays.cpp. For some reason after I 
updated this revision Phabricator doesn't show the full diff, but it shows both 
commits. The first commit with main changes and the test is available here - 
https://reviews.llvm.org/D114080?id=387904 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 390376.
Fznamznon added a comment.

An attempt to fix review page


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/test/SemaSYCL/zero-length-arrays.cpp

Index: clang/test/SemaSYCL/zero-length-arrays.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/zero-length-arrays.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -fsyntax-only -verify %s
+//
+// This test checks if compiler reports compilation error on an attempt to use
+// a zero-length array inside device code.
+
+template 
+__attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc(); // #KernelObjCall
+}
+
+typedef float ZEROARR[0];
+
+struct Wrapper {
+  int A;
+  int BadArray[0]; // expected-note 3{{field of illegal type 'int[0]' declared here}}
+};
+
+struct WrapperOfWrapper { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  Wrapper F;  // expected-note 2{{within field of type 'Wrapper' declared here}}
+  ZEROARR *Ptr;   //expected-note 5{{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+};
+
+template  struct InnerTemplated {
+  double Array[Size]; // expected-note 8{{field of illegal type 'double[0]' declared here}}
+};
+
+template  struct Templated {
+  unsigned A;
+  Ty Arr[Size];
+  InnerTemplated Array[Size + 1]; // expected-note 8{{within field of type 'InnerTemplated<0U>[1]' declared here}}
+};
+
+struct KernelSt {
+  int A;
+  int BadArray[0]; // expected-note {{field of illegal type 'int[0]' declared here}}
+  void operator()() const {}
+};
+
+WrapperOfWrapper offendingFoo() {
+  // expected-note@+1 {{called by 'offendingFoo'}}
+  return WrapperOfWrapper{};
+}
+
+template 
+void templatedContext() {
+  Templated Var;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@#KernelObjCall {{called by 'kernel([=] {
+// expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+(void)Var; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+  kernel([Var] {
+  });
+}
+
+void foo(const unsigned X) {
+  int Arr[0];  // expected-note 2{{declared here}}
+  ZEROARR TypeDef; // expected-note {{declared here}}
+  ZEROARR *Ptr;// expected-note {{declared here}}
+   // expected-error@#KernelObjCall 3{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=]() {
+(void)Arr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+(void)TypeDef; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+// expected-note@+1 {{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+(void)Ptr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{field of illegal type 'int[0]' declared here}}
+  kernel([Arr] { // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  WrapperOfWrapper St;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel([=] {
+// expected-note@+1 {{within field of type 'WrapperOfWrapper' declared here}}
+(void)St.F.BadArray; // expected-error 4{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'WrapperOfWrapper' declared here}}
+  kernel([St] { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+
+  Templated<1, int> OK;
+  Templated<1 - 1, double> Weirdo;
+  Templated<0, float> Zero;
+  // expected-error@#KernelObjCall 4{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D114080#3158368 , @Fznamznon wrote:

> In D114080#3158323 , @erichkeane 
> wrote:
>
>> I note that this is missing a test, otherwise I don't see any issues with it 
>> from my end.
>
> There is a test SemaSYCL/zero-length-arrays.cpp. For some reason after I 
> updated this revision Phabricator doesn't show the full diff, but it shows 
> both commits. The first commit with main changes and the test is available 
> here - https://reviews.llvm.org/D114080?id=387904 .

It seems I was able to fix that. Now the link to this revision shows the full 
diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D142534: Fix emission of consteval constructor of derived type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For simple derived type ConstantEmitter returns a struct of the same
size but different type which is then stored field-by-field into memory
via pointer to derived type. In case base type has more fields than derived,
the incorrect GEP is emitted. So, just cast pointer to derived type to
appropriate type with enough fields.

Fixes #60166


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142534

Files:
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace Issue60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.Issue60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace Issue60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
 EnsureDest(E->getType());
 
 if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-  CGF.EmitAggregateStore(Result, Dest.getAddress(),
+  Address StoreDest = Dest.getAddress();
+  // The emitted value is guaranteed to have the same size as the
+  // destination but can have a different type. Just do a bitcast in this
+  // case to avoid incorrect GEPs.
+  if (Result->getType() != StoreDest.getType())
+StoreDest =
+CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+  CGF.EmitAggregateStore(Result, StoreDest,
  E->getType().isVolatileQualified());
   return;
 }


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace Issue60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.Issue60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace Issue60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
 EnsureDest(E->getType());
 
 if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-  CGF.EmitAggregateStore(Result, Dest.getAddress(),
+  Address StoreDest = Dest.getAddress();
+  // The emitted value is guaranteed to have the same size as the
+  // destination but can have a different type. Just do a bitcast in this
+  // case to avoid incorrect GEPs.
+  if (Result->getType() != StoreDest.getType())
+StoreDest =
+CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+  CGF.EmitAggregateStore(Result, StoreDest,
  E->getType().isVolatileQualified());
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142534: Fix emission of consteval constructor of derived type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Unit tests fail seems unrelated. I'm seeing the same test fail for a commit 
that is already in main 
https://github.com/llvm/llvm-project/commit/f1f583347d00aad378eb0128e72d3d2e8be5174b
 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142534

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

[PATCH] D142534: Fix emission of consteval constructor of derived type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D142534#4079958 , @shafik wrote:

> I think having a link to the github issue in the summary allows for the issue 
> be closed automatically when you commit. Is this correct @aaron.ballman I 
> have been doing this for a while and they get closed when I commit but maybe 
> there is another mechanism involved.

This works at least for GitHub pull requests, see 
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
 . We've been using this in a downstream project. I was hoping it works in the 
same way for commits, so added issue ID to a commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142534

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Ensure it is at least 8 bits.

Fixes #59801


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142550

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/vector-bool.cpp


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,9 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name 
''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name 
''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(8)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,9 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name ''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name ''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(8)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaCXX/vector-bool.cpp:95
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(8)));
+  static_assert(sizeof(FourBools) == 1);

This is not four bools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaCXX/vector-bool.cpp:95
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(8)));
+  static_assert(sizeof(FourBools) == 1);

erichkeane wrote:
> tbaeder wrote:
> > Fznamznon wrote:
> > > This is not four bools.
> > Should be 4 instead of 8, shouldn't it?
> Can you add another couple of 'oddball' sizes and make sure they work out 
> right?  I THINK the alignment logic gets it right, but I'd like tests to 
> confirm.
Yes, will fix in a moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 492125.
Fznamznon added a comment.

Adjust the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

Files:
  clang/test/SemaCXX/vector-bool.cpp


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -92,7 +92,11 @@
 }
 
 void Sizeof() {
-  using FourBools = bool __attribute__((ext_vector_type(8)));
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using ABunchOfBools = bool __attribute__((ext_vector_type(28)));
   static_assert(sizeof(FourBools) == 1);
   static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(ABunchOfBools) == 4);
 }


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -92,7 +92,11 @@
 }
 
 void Sizeof() {
-  using FourBools = bool __attribute__((ext_vector_type(8)));
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using ABunchOfBools = bool __attribute__((ext_vector_type(28)));
   static_assert(sizeof(FourBools) == 1);
   static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(ABunchOfBools) == 4);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaCXX/vector-bool.cpp:95
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(8)));
+  static_assert(sizeof(FourBools) == 1);

Fznamznon wrote:
> erichkeane wrote:
> > tbaeder wrote:
> > > Fznamznon wrote:
> > > > This is not four bools.
> > > Should be 4 instead of 8, shouldn't it?
> > Can you add another couple of 'oddball' sizes and make sure they work out 
> > right?  I THINK the alignment logic gets it right, but I'd like tests to 
> > confirm.
> Yes, will fix in a moment.
Okay, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 492151.
Fznamznon added a comment.

Add a case with vector of length 33


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/vector-bool.cpp


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,15 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name 
''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name 
''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,15 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name ''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name ''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaCXX/vector-bool.cpp:97
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using ABunchOfBools = bool __attribute__((ext_vector_type(28)));
   static_assert(sizeof(FourBools) == 1);

erichkeane wrote:
> How about 33?  Does it grow appropriately with alignment?
Its sizeof evaluates to 8. Looking at the code in ASTContext that seems to be 
appropriate result since 8 is a next power of 2 after 4.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 492362.
Fznamznon added a comment.

Add more sizes to test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/vector-bool.cpp


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,25 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name 
''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name 
''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  using SixtyFiveBools = bool __attribute__((ext_vector_type(65)));
+  using Bool129 = bool __attribute__((ext_vector_type(129)));
+  using Bool150 = bool __attribute__((ext_vector_type(150)));
+  using Bool195 = bool __attribute__((ext_vector_type(195)));
+  using Bool257 = bool __attribute__((ext_vector_type(257)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+  static_assert(sizeof(SixtyFiveBools) == 16);
+  static_assert(sizeof(Bool129) == 32);
+  static_assert(sizeof(Bool150) == 32);
+  static_assert(sizeof(Bool195) == 32);
+  static_assert(sizeof(Bool257) == 64);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,25 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name ''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name ''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  using SixtyFiveBools = bool __attribute__((ext_vector_type(65)));
+  using Bool129 = bool __attribute__((ext_vector_type(129)));
+  using Bool150 = bool __attribute__((ext_vector_type(150)));
+  using Bool195 = bool __attribute__((ext_vector_type(195)));
+  using Bool257 = bool __attribute__((ext_vector_type(257)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+  static_assert(sizeof(SixtyFiveBools) == 16);
+  static_assert(sizeof(Bool129) == 32);
+  static_assert(sizeof(Bool150) == 32);
+  static_assert(sizeof(Bool195) == 32);
+  static_assert(sizeof(Bool257) == 64);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2002,7 +2002,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaCXX/vector-bool.cpp:97
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using ABunchOfBools = bool __attribute__((ext_vector_type(28)));
   static_assert(sizeof(FourBools) == 1);

erichkeane wrote:
> Fznamznon wrote:
> > erichkeane wrote:
> > > How about 33?  Does it grow appropriately with alignment?
> > Its sizeof evaluates to 8. Looking at the code in ASTContext that seems to 
> > be appropriate result since 8 is a next power of 2 after 4.
> Yep, that makes sense to me!  Can you do 1 more group for me?  
> 
> 65, 129, 150, 195, 257?
> 
> 
Sure, done. Seems to be working in the same way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

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

[PATCH] D142550: Fix sizeof of boolean vector

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4f4b2fe21dd: [clang] Fix sizeof of boolean vector (authored 
by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142550

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/vector-bool.cpp


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,25 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name 
''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name 
''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  using SixtyFiveBools = bool __attribute__((ext_vector_type(65)));
+  using Bool129 = bool __attribute__((ext_vector_type(129)));
+  using Bool150 = bool __attribute__((ext_vector_type(150)));
+  using Bool195 = bool __attribute__((ext_vector_type(195)));
+  using Bool257 = bool __attribute__((ext_vector_type(257)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+  static_assert(sizeof(SixtyFiveBools) == 16);
+  static_assert(sizeof(Bool129) == 32);
+  static_assert(sizeof(Bool150) == 32);
+  static_assert(sizeof(Bool195) == 32);
+  static_assert(sizeof(Bool257) == 64);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2076,7 +2076,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.


Index: clang/test/SemaCXX/vector-bool.cpp
===
--- clang/test/SemaCXX/vector-bool.cpp
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -90,3 +90,25 @@
   foo(eight_bools.w);// expected-error@90 {{illegal vector component name ''w''}}
   foo(eight_bools.wyx);  // expected-error@91 {{illegal vector component name ''wyx''}}
 }
+
+void Sizeof() {
+  using FourBools = bool __attribute__((ext_vector_type(4)));
+  using NineBools = bool __attribute__((ext_vector_type(9)));
+  using TwentyEightBools = bool __attribute__((ext_vector_type(28)));
+  using ThirtyThreeBools = bool __attribute__((ext_vector_type(33)));
+  using SixtyFiveBools = bool __attribute__((ext_vector_type(65)));
+  using Bool129 = bool __attribute__((ext_vector_type(129)));
+  using Bool150 = bool __attribute__((ext_vector_type(150)));
+  using Bool195 = bool __attribute__((ext_vector_type(195)));
+  using Bool257 = bool __attribute__((ext_vector_type(257)));
+  static_assert(sizeof(FourBools) == 1);
+  static_assert(sizeof(EightBools) == 1);
+  static_assert(sizeof(NineBools) == 2);
+  static_assert(sizeof(TwentyEightBools) == 4);
+  static_assert(sizeof(ThirtyThreeBools) == 8);
+  static_assert(sizeof(SixtyFiveBools) == 16);
+  static_assert(sizeof(Bool129) == 32);
+  static_assert(sizeof(Bool150) == 32);
+  static_assert(sizeof(Bool195) == 32);
+  static_assert(sizeof(Bool257) == 64);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2076,7 +2076,8 @@
 TypeInfo EltInfo = getTypeInfo(VT->getElementType());
 Width = VT->isExtVectorBoolType() ? VT->getNumElements()
   : EltInfo.Width * VT->getNumElements();
-// Enforce at least byte alignment.
+// Enforce at least byte size and alignment.
+Width = std::max(8, Width);
 Align = std::max(8, Width);
 
 // If the alignment is not a power of 2, round up to the next power of 2.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142534: Fix emission of consteval constructor of derived type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 492485.
Fznamznon added a comment.

Rebase and apply nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142534

Files:
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace GH60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.GH60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace GH60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
 EnsureDest(E->getType());
 
 if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-  CGF.EmitAggregateStore(Result, Dest.getAddress(),
+  Address StoreDest = Dest.getAddress();
+  // The emitted value is guaranteed to have the same size as the
+  // destination but can have a different type. Just do a bitcast in this
+  // case to avoid incorrect GEPs.
+  if (Result->getType() != StoreDest.getType())
+StoreDest =
+CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+  CGF.EmitAggregateStore(Result, StoreDest,
  E->getType().isVolatileQualified());
   return;
 }


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace GH60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.GH60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace GH60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
 EnsureDest(E->getType());
 
 if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-  CGF.EmitAggregateStore(Result, Dest.getAddress(),
+  Address StoreDest = Dest.getAddress();
+  // The emitted value is guaranteed to have the same size as the
+  // destination but can have a different type. Just do a bitcast in this
+  // case to avoid incorrect GEPs.
+  if (Result->getType() != StoreDest.getType())
+StoreDest =
+CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+  CGF.EmitAggregateStore(Result, StoreDest,
  E->getType().isVolatileQualified());
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142757: [clang][driver] Do not warn about position of `/clang:-xc` in cl mode

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

In CL mode values of `/clang:` arguments end up at the end of
arguments list which makes the warning always emitted.
Just do not emit the warning in cl mode since it was implemented to
match gcc.

Fixes #59307


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142757

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,7 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+//
+// RUN: %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
-check-prefix=CL %s
+// RUN: %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
-check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2572,10 +2572,11 @@
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
   }
 


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,7 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+//
+// RUN: %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s
+// RUN: %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2572,10 +2572,11 @@
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142757: [clang][driver] Do not warn about position of `/clang:-xc` in cl mode

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 493256.
Fznamznon added a comment.

Emit an error if `/clang:-x` passed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
-check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
-check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
+++ clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
@@ -1,12 +1,12 @@
 [
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 ]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2572,11 +2572,18 @@
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
+++ clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
@@ -1,12 +1,12 @@
 [
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF /clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules /clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF /clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimpli

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/Driver.cpp:2571
 // No driver mode exposes -x and /TC or /TP; we don't support mixing them.
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }

MaskRay wrote:
> ```
> -assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not 
> allowed");
> +if (auto *A = Args.getLastArg(options::OPT_x))
> +  Diag(diag::err_drv_unsupported_opt_with_suggestion)
> +  << A->getAsString(Args) << "/TC' or '/TP";
> ```
Thank you, the suggestion seems reasonable to me. Although, the concrete place 
doesn't.
The suggested line to change is under if (we have /TC or /TP argument), so if I 
put error emission there it won't be emitted if no /TC or /TP was passed and 
the original problem reported in 
https://github.com/llvm/llvm-project/issues/59307 won't be resolved. People 
will still be confused by the warning saying `-x c` passed after last input 
when in fact they passed `/clang:-x` before the input.

Also, the whole function doesn't return even if diagnostic is emitted, so I put 
error emission in a way to not emit a warning saying that -x is passed after 
last input together with it, because both errors emitted like this:

error: '-x c' after last input file has no effect
error: unsupported option '-x c'; did you mean '/TC' or '/TP'?

look confusing.



Comment at: clang/test/Driver/x-args.c:8
 // CHECK: '-x c++' after last input file has no effect
+//
+// RUN: %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
-check-prefix=CL %s

MaskRay wrote:
> Delete `//`
> 
Done, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 493326.
Fznamznon added a comment.

Add a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
-check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
-check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
+++ clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
@@ -1,12 +1,12 @@
 [
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 ]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2572,11 +2572,18 @@
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -57,6 +57,10 @@
 - Fix crash on invalid code when looking up a destructor in a templated class
   inside a namespace. This fixes
   `Issue 59446 `_.
+- Fix confusing warning message when `/clang:-x` is passed in clang-cl driver
+  mode and emit an error which suggests using `/TC` or `/TP` clang-cl options
+  instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,8 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
==

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 493576.
Fznamznon added a comment.

Apply suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
  clang/test/Driver/x-args.c


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,9 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck 
-check-prefix=CL %s
+// RUN: not %clang_cl /WX /clang:-E /clang:-dM %s /clang:-xc 2>&1 | FileCheck 
-check-prefix=CL %s
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
Index: clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
===
--- clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
+++ clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json
@@ -1,12 +1,12 @@
 [
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF 
/clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 {
   "directory": "DIR",
-  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
+  "command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules 
/clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules 
/clang:-fimplicit-module-maps /TP --",
   "file": ""
 },
 ]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2566,17 +2566,21 @@
 }
 if (ShowNote)
   Diag(clang::diag::note_drv_t_option_is_global);
-
-// No driver mode exposes -x and /TC or /TP; we don't support mixing them.
-assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
 
   // Warn -x after last input file has no effect
-  {
+  if (!IsCLMode()) {
 Arg *LastXArg = Args.getLastArgNoClaim(options::OPT_x);
 Arg *LastInputArg = Args.getLastArgNoClaim(options::OPT_INPUT);
-if (LastXArg && LastInputArg && LastInputArg->getIndex() < 
LastXArg->getIndex())
+if (LastXArg && LastInputArg &&
+LastInputArg->getIndex() < LastXArg->getIndex())
   Diag(clang::diag::warn_drv_unused_x) << LastXArg->getValue();
+  } else {
+// In CL mode suggest /TC or /TP since -x doesn't make sense if passed via
+// /clang:.
+if (auto *A = Args.getLastArg(options::OPT_x))
+  Diag(diag::err_drv_unsupported_opt_with_suggestion)
+  << A->getAsString(Args) << "/TC' or '/TP";
   }
 
   for (Arg *A : Args) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,10 @@
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates. This fixes
   `Issue 60344 `_.
+- Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
+  driver mode and emit an error which suggests using ``/TC`` or ``/TP``
+  ``clang-cl`` options instead. This fixes
+  `Issue 59307 `_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Driver/x-args.c
===
--- clang/test/Driver/x-args.c
+++ clang/test/Driver/x-args.c
@@ -5,3 +5,9 @@
 // RUN: %clang -fsyntax-only -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -fsyntax-only %s -xc %s -xc++ -fsyntax-only 2>&1 | FileCheck %s
 // CHECK: '-x c++' after last input file has no effect
+
+// RUN: not %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s
+

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2571
 // No driver mode exposes -x and /TC or /TP; we don't support mixing them.
 assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }

MaskRay wrote:
> Fznamznon wrote:
> > MaskRay wrote:
> > > ```
> > > -assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not 
> > > allowed");
> > > +if (auto *A = Args.getLastArg(options::OPT_x))
> > > +  Diag(diag::err_drv_unsupported_opt_with_suggestion)
> > > +  << A->getAsString(Args) << "/TC' or '/TP";
> > > ```
> > Thank you, the suggestion seems reasonable to me. Although, the concrete 
> > place doesn't.
> > The suggested line to change is under if (we have /TC or /TP argument), so 
> > if I put error emission there it won't be emitted if no /TC or /TP was 
> > passed and the original problem reported in 
> > https://github.com/llvm/llvm-project/issues/59307 won't be resolved. People 
> > will still be confused by the warning saying `-x c` passed after last input 
> > when in fact they passed `/clang:-x` before the input.
> > 
> > Also, the whole function doesn't return even if diagnostic is emitted, so I 
> > put error emission in a way to not emit a warning saying that -x is passed 
> > after last input together with it, because both errors emitted like this:
> > 
> > error: '-x c' after last input file has no effect
> > error: unsupported option '-x c'; did you mean '/TC' or '/TP'?
> > 
> > look confusing.
> You need to remove `assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP 
> is not allowed");` as otherwise `/clang:-x /TC` leads to a crash in an assert 
> build of clang (`LLVM_ENABLE_ASSERTIONS=on`).
Ok, removed the assert. Verified there is no crash in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:60
   `Issue 59446 `_.
+- Fix confusing warning message when `/clang:-x` is passed in clang-cl driver
+  mode and emit an error which suggests using `/TC` or `/TP` clang-cl options

MaskRay wrote:
> Double backtick
> 
> `LLVM_ENABLE_SPHINX=on` and run `ninja docs-clang-html` to check whether the 
> docs build is good.
Ok fixed and verified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@MaskRay , thanks for the suggestions. Applied. Please let me know that it is 
ok now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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