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

[Alexey Bader via Phabricator via cfe-commits]

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

Applied code review suggestions.

- Split the patch into two parts. This patch contains only Sema part and LLVM 
IR generation part will be added separately. Updated commit message.
- Replace `can't` with `cannot`.


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/lib/Sema/SemaDeclAttr.cpp
  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,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 function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// First two template parameters cannot be non-type template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute cannot be a non-type template parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute cannot be a non-type template parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// 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@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6412,6 +6412,45 @@
   D->addAttr(::new (S.Context) OpenCLAccessAttr(S.Context, AL));
 }
 
+static void handleSYCLKernelAttr(Sema , Decl *D, const ParsedAttr ) {
+  // The 'sycl_kernel' attribute applies only to functions.
+  const auto *FD = cast(D);
+  const FunctionTemplateDecl *FT = FD->getDescribedFunctionTemplate();
+  assert(FT && "Function template is expected");
+
+  // Function template must have at least two template parameters.
+  const TemplateParameterList *TL = FT->getTemplateParameters();
+  if (TL->size() < 2) {
+S.Diag(FT->getLocation(), diag::warn_sycl_kernel_num_of_template_params);
+return;
+  }
+
+  // Template parameters must be typenames.
+  for (unsigned I = 0; I < 2; ++I) {
+const NamedDecl *TParam = TL->getParam(I);
+ 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

The attribute bits LGTM aside from a wording nit with the diagnostic; I have no 
opinion on the CodeGen question.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10080
+  "template parameter of a function template with the 'sycl_kernel' attribute"
+  " can't be a non-type template parameter">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

`can't` -> `cannot`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr()) {
+// SYCL kernels can be templated and not called from anywhere in the

bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > Need to check if the decl must be emitted at all.
> > > Let me check that I get it right. You suggest adding `if 
> > > (MustBeEmitted(Global))`, right?
> > > ```
> > >   if (LangOpts.SYCLIsDevice && Global->hasAttr() && 
> > > MustBeEmitted(Global)) {
> > > ...
> > > addDeferredDeclToEmit(GD);
> > > return;
> > >   }
> > > ```
> > Yes
> Okay. Making this change requires additional adjustments in the patch and I 
> have a few options.
> In this patch we do not add any logic forcing compiler to emit SYCL kernel. 
> This logic is supposed to be added by follow-up patch (currently under SYCL 
> working group review here https://github.com/intel/llvm/pull/249), which add 
> code emitting "externally visible" OpenCL kernel calling function object 
> passed to SYCL kernel function.
> 
> I can:
> 1) Temporally remove CodeGen test and add updated version back with the 
> follow-up patch
> 2) Do change making SYCL kernels "externally visible" and revert this change 
> with the follow-up patch (this is kind of current logic which emits SYCL 
> kernels unconditionally)
> 3) Merge two patches and submit them together, but I assume it will 
> significantly increase the size of the patch.
Probably, better would be to split the 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] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr()) {
+// SYCL kernels can be templated and not called from anywhere in the

ABataev wrote:
> bader wrote:
> > ABataev wrote:
> > > Need to check if the decl must be emitted at all.
> > Let me check that I get it right. You suggest adding `if 
> > (MustBeEmitted(Global))`, right?
> > ```
> >   if (LangOpts.SYCLIsDevice && Global->hasAttr() && 
> > MustBeEmitted(Global)) {
> > ...
> > addDeferredDeclToEmit(GD);
> > return;
> >   }
> > ```
> Yes
Okay. Making this change requires additional adjustments in the patch and I 
have a few options.
In this patch we do not add any logic forcing compiler to emit SYCL kernel. 
This logic is supposed to be added by follow-up patch (currently under SYCL 
working group review here https://github.com/intel/llvm/pull/249), which add 
code emitting "externally visible" OpenCL kernel calling function object passed 
to SYCL kernel function.

I can:
1) Temporally remove CodeGen test and add updated version back with the 
follow-up patch
2) Do change making SYCL kernels "externally visible" and revert this change 
with the follow-up patch (this is kind of current logic which emits SYCL 
kernels unconditionally)
3) Merge two patches and submit them together, but I assume it will 
significantly increase the size of the 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] Implement SYCL device code outlining

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr()) {
+// SYCL kernels can be templated and not called from anywhere in the

bader wrote:
> ABataev wrote:
> > Need to check if the decl must be emitted at all.
> Let me check that I get it right. You suggest adding `if 
> (MustBeEmitted(Global))`, right?
> ```
>   if (LangOpts.SYCLIsDevice && Global->hasAttr() && 
> MustBeEmitted(Global)) {
> ...
> addDeferredDeclToEmit(GD);
> return;
>   }
> ```
Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr()) {
+// SYCL kernels can be templated and not called from anywhere in the

ABataev wrote:
> Need to check if the decl must be emitted at all.
Let me check that I get it right. You suggest adding `if 
(MustBeEmitted(Global))`, right?
```
  if (LangOpts.SYCLIsDevice && Global->hasAttr() && 
MustBeEmitted(Global)) {
...
addDeferredDeclToEmit(GD);
return;
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr()) {
+// SYCL kernels can be templated and not called from anywhere in the

Need to check if the decl must be emitted at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

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

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

aaron.ballman wrote:
> bader wrote:
> > aaron.ballman wrote:
> > > This diagnostic reads a bit like you cannot do this: `template ` 
> > > when I think the actual restriction is that you cannot do this: `template 
> > > `. Is that correct? If so, I think this could be worded as 
> > > `template parameter of a function template with the 'sycl_kernel' 
> > > attribute must be a template type parameter`.
> > > 
> > > Just double-checking, but you also intend to prohibit template template 
> > > parameters? e.g., you can't do `template  typename C>`
> > > This diagnostic reads a bit like you cannot do this: template  
> > > when I think the actual restriction is that you cannot do this: template 
> > > . Is that correct?
> > 
> > Yes. That is correct.
> > 
> > >  If so, I think this could be worded as template parameter of a function 
> > > template with the 'sycl_kernel' attribute must be a template type 
> > > parameter.
> > 
> > Thanks! Applied your wording.
> > 
> > > Just double-checking, but you also intend to prohibit template template 
> > > parameters? e.g., you can't do template  typename C>
> > 
> > Currently we allow following use case: 
> > https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
> >  I assume it qualifies as "template type" and not as "template template" 
> > parameter. Right?
> > 
> > Quoting SYCL specification $6.2 Naming of kernels 
> > (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).
> > 
> > > SYCL kernels are extracted from C++ source files and stored in an 
> > > implementation- defined format. In the case of
> > > the shared-source compilation model, the kernels have to be uniquely 
> > > identified by both host and device compiler.
> > > This is required in order for the host runtime to be able to load the 
> > > kernel by using the OpenCL host runtime
> > > interface.
> > > From this requirement the following rules apply for naming the kernels:
> > > • The kernel name is a C++ typename.
> > > • The kernel needs to have a globally-visible name. In the case of a 
> > > named function object type, the name can
> > > be the typename of the function object, as long as it is 
> > > globally-visible. In the case where it isn’t, a globally visible name has 
> > > to be provided, as template parameter to the kernel invoking interface, 
> > > as described in 4.8.5.
> > > In C++11, lambdas do not have a globally-visible name, so a 
> > > globally-visible typename has to be provided
> > > in the kernel invoking interface, as described in 4.8.5.
> > > • The kernel name has to be a unique identifier in the program.
> > > 
> > 
> > We also have an extension, which lifts these restrictions/requirements when 
> > clang is used as host and device compiler. @erichkeane implemented built-in 
> > function (https://github.com/intel/llvm/pull/250) providing "unique 
> > identifier", which we use as a kernel name for lambda objects. But this is 
> > going to be a separate patch.
> > Currently we allow following use case: 
> > https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
> >  I assume it qualifies as "template type" and not as "template template" 
> > parameter. Right?
> 
> Yeah, those are template types. A template template parameter would be: 
> https://godbolt.org/z/9kwbW9
> In that example, `C` is a template template parameter and `Ty` is a template 
> type parameter. The part I'm a bit unclear on is why a template template 
> parameter should be disallowed (I believe it names a type, as opposed to a 
> non-type template parameter which names a value)?
I think Mariya implemented this restriction to avoid usages not required for 
SYCL kernel support implementation in run-time library. As it was mentioned 
before, this attribute is intended to be used by SYCL run-time library only and 
current implantation do not require `template template parameter` support.
I think that this might be useful for alternative implementations, so I updated 
the patch to restrict non-type template parameters only.



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

aaron.ballman wrote:
> bader wrote:
> > It looks like this change is not needed anymore. This check fails on my 
> > machine with the latest version of the patch.
> > 
> > @aaron.ballman, I'm not sure if this is a problem of the implementation or 
> 

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

[Alexey Bader via Phabricator via cfe-commits]

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

Applied code review comments from Aaron.

Allow template template parameters for function templates marked with 
`sycl_kernel` attribute.


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/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  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,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 function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// First two template parameters can't be non-type template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// 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@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple spir64 -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();
+}
+
+// Make sure that definitions for the types not used in SYCL kernels are not
+// emitted
+// CHECK-NOT: %struct.A
+// CHECK-NOT: @a = {{.*}} %struct.A
+struct A {
+  int x = 10;
+} a;
+
+int main() {
+  a.x = 8;
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+
+// baz is not called from the SYCL kernel, so it must not be emitted
+// CHECK-NOT: define {{.*}} @{{.*}}baz
+void baz() {}
+

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: arphaman.
aaron.ballman added a subscriber: arphaman.
aaron.ballman added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

bader wrote:
> aaron.ballman wrote:
> > This diagnostic reads a bit like you cannot do this: `template ` 
> > when I think the actual restriction is that you cannot do this: `template 
> > `. Is that correct? If so, I think this could be worded as `template 
> > parameter of a function template with the 'sycl_kernel' attribute must be a 
> > template type parameter`.
> > 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do `template  typename C>`
> > This diagnostic reads a bit like you cannot do this: template  
> > when I think the actual restriction is that you cannot do this: template 
> > . Is that correct?
> 
> Yes. That is correct.
> 
> >  If so, I think this could be worded as template parameter of a function 
> > template with the 'sycl_kernel' attribute must be a template type parameter.
> 
> Thanks! Applied your wording.
> 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do template  typename C>
> 
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?
> 
> Quoting SYCL specification $6.2 Naming of kernels 
> (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).
> 
> > SYCL kernels are extracted from C++ source files and stored in an 
> > implementation- defined format. In the case of
> > the shared-source compilation model, the kernels have to be uniquely 
> > identified by both host and device compiler.
> > This is required in order for the host runtime to be able to load the 
> > kernel by using the OpenCL host runtime
> > interface.
> > From this requirement the following rules apply for naming the kernels:
> > • The kernel name is a C++ typename.
> > • The kernel needs to have a globally-visible name. In the case of a named 
> > function object type, the name can
> > be the typename of the function object, as long as it is globally-visible. 
> > In the case where it isn’t, a globally visible name has to be provided, as 
> > template parameter to the kernel invoking interface, as described in 4.8.5.
> > In C++11, lambdas do not have a globally-visible name, so a 
> > globally-visible typename has to be provided
> > in the kernel invoking interface, as described in 4.8.5.
> > • The kernel name has to be a unique identifier in the program.
> > 
> 
> We also have an extension, which lifts these restrictions/requirements when 
> clang is used as host and device compiler. @erichkeane implemented built-in 
> function (https://github.com/intel/llvm/pull/250) providing "unique 
> identifier", which we use as a kernel name for lambda objects. But this is 
> going to be a separate patch.
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?

Yeah, those are template types. A template template parameter would be: 
https://godbolt.org/z/9kwbW9
In that example, `C` is a template template parameter and `Ty` is a template 
type parameter. The part I'm a bit unclear on is why a template template 
parameter should be disallowed (I believe it names a type, as opposed to a 
non-type template parameter which names a value)?



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

bader wrote:
> It looks like this change is not needed anymore. This check fails on my 
> machine with the latest version of the patch.
> 
> @aaron.ballman, I'm not sure if this is a problem of the implementation or 
> test issue.
> Do I understand correctly that this test validates the list of the attributes 
> which can be applied using `#pragma clang`?
> If so, removing this check seems to be okay. We need only 
> `[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.
Your understanding is correct, and I think it's a bug if you don't need to add 
an entry here for `SYCLKernel`. @arphaman, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

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

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

It looks like this change is not needed anymore. This check fails on my machine 
with the latest version of the patch.

@aaron.ballman, I'm not sure if this is a problem of the implementation or test 
issue.
Do I understand correctly that this test validates the list of the attributes 
which can be applied using `#pragma clang`?
If so, removing this check seems to be okay. We need only 
`[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 230281.
bader marked 5 inline comments as done.
bader added a comment.

Applied code review 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/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.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,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 function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// Both first two template parameters must be a template type parameter
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// 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@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
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
@@ -131,6 +131,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

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

[Alexey Bader via Phabricator via cfe-commits]

bader added a subscriber: erichkeane.
bader added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

aaron.ballman wrote:
> This diagnostic reads a bit like you cannot do this: `template ` 
> when I think the actual restriction is that you cannot do this: `template 
> `. Is that correct? If so, I think this could be worded as `template 
> parameter of a function template with the 'sycl_kernel' attribute must be a 
> template type parameter`.
> 
> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do `template  typename C>`
> This diagnostic reads a bit like you cannot do this: template  when 
> I think the actual restriction is that you cannot do this: template . 
> Is that correct?

Yes. That is correct.

>  If so, I think this could be worded as template parameter of a function 
> template with the 'sycl_kernel' attribute must be a template type parameter.

Thanks! Applied your wording.

> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do template  typename C>

Currently we allow following use case: 
https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp. 
I assume it qualifies as "template type" and not as "template template" 
parameter. Right?

Quoting SYCL specification $6.2 Naming of kernels 
(https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).

> SYCL kernels are extracted from C++ source files and stored in an 
> implementation- defined format. In the case of
> the shared-source compilation model, the kernels have to be uniquely 
> identified by both host and device compiler.
> This is required in order for the host runtime to be able to load the kernel 
> by using the OpenCL host runtime
> interface.
> From this requirement the following rules apply for naming the kernels:
> • The kernel name is a C++ typename.
> • The kernel needs to have a globally-visible name. In the case of a named 
> function object type, the name can
> be the typename of the function object, as long as it is globally-visible. In 
> the case where it isn’t, a globally visible name has to be provided, as 
> template parameter to the kernel invoking interface, as described in 4.8.5.
> In C++11, lambdas do not have a globally-visible name, so a globally-visible 
> typename has to be provided
> in the kernel invoking interface, as described in 4.8.5.
> • The kernel name has to be a unique identifier in the program.
> 

We also have an extension, which lifts these restrictions/requirements when 
clang is used as host and device compiler. @erichkeane implemented built-in 
function (https://github.com/intel/llvm/pull/250) providing "unique 
identifier", which we use as a kernel name for lambda objects. But this is 
going to be a separate 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] Implement SYCL device code outlining

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10076
+def warn_sycl_kernel_num_of_template_params : Warning<
+  "'sycl_kernel' attribute only applies to template functions with at least"
+  " two template parameters">, InGroup;

Do you mean template function or function template? A function template is a 
template used to generate functions and a template function is a function 
produced by a template. I think you probably mean "function template" here.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

This diagnostic reads a bit like you cannot do this: `template ` when 
I think the actual restriction is that you cannot do this: `template `. 
Is that correct? If so, I think this could be worded as `template parameter of 
a function template with the 'sycl_kernel' attribute must be a template type 
parameter`.

Just double-checking, but you also intend to prohibit template template 
parameters? e.g., you can't do `template  typename C>`



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10082
+def warn_sycl_kernel_num_of_function_params : Warning<
+  "template function with 'sycl_kernel' attribute must have single parameter">,
+  InGroup;

Probably "function template" here as well.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10085
+def warn_sycl_kernel_return_type : Warning<
+  "template function with 'sycl_kernel' attribute must return void type">,
+  InGroup;

Same 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

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

@aaron.ballman, @Anastasia, could you take a look at new version of the patch, 
please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaSYCL/device-attributes.cpp:35
+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}}
+

Do we have to check each diagnostic message for both attribute spellings?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 228868.
bader added a subscriber: hfinkel.
bader added a comment.

Applied two remaining comments from Aaron.

- Split diagnostics for `sycl_kernel` attribute to provide more informative 
message.
- Moved attribute target check to TableGen file. I stole a workaround for a 
function template subject emulation from @hfinkel C++ JIT compiler prototype 
(https://github.com/hfinkel/llvm-project-cxxjit/blob/cxxjit/clang/include/clang/Basic/Attr.td#L121).


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/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.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,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 function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only template functions
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to template functions with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to template functions with at least two template parameters}}
+
+// Both first two template parameters must be a typenames
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of template functions with 'sycl_kernel' attribute must be typename}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of template functions with 'sycl_kernel' attribute must be typename}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{template function with 'sycl_kernel' attribute must return void type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{template function with 'sycl_kernel' attribute must return void type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{template function with 'sycl_kernel' attribute must have single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{template function with 'sycl_kernel' attribute must have single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// 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@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
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
@@ -131,6 +131,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)
 

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

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1034
+  let Spellings = [Clang<"sycl_kernel">];
+  let Subjects = SubjectList<[Function]>;
+  let LangOpts = [SYCL];

aaron.ballman wrote:
> Shouldn't this be `FunctionTemplate` instead?
@aaron.ballman, I'm not sure.
I tried to use FunctionTemplate instead of Function, but I get following 
warning:
```
warning: 'sycl_kernel' attribute only applies to redeclarable templates
```
I investigated this a little and Sema passes Function declaration instead of 
FunctionTemplate to the function validating the attribute appertains to the 
right subject. I think it's because attributes are handled before 
FunctionTemplateDecl node is created.
Do we have an infrastructure to handle "FunctionTemplate" attributes?

I can't find any other attribute with FunctionTemplate subject to learn from...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Alexey Bader via Phabricator via cfe-commits]

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

Applied comments from Aaron.

Two comments left unresolved:

- Split diagnostic message for sycl_kernel attribute into multiple messages. 
Will do tomorrow.
- Change attribute "subject" in TableGen file from "Function" to 
"FunctionTemplate". I need guidance here as I'm not sure how to make it work.

Refactored patch to re-use CodeGen infrastructure for emitting SYCL device code.
It turned out to be quite simple change - just two one-liner changes in 
ASTContext to say that only SYCL kernels must be emitted when we compile for 
SYCL device + similar change in the CodeGen to mark symbols which must be 
emitted.

Removed `sycl_device` attribute, which was required by previous implementation 
for device code outlining. I think we still might need this attribute to mark 
"non-kernel" symbols as "device code", so the compiler will emit even though 
they are not used by SYCL kernels. Anyway it's not required for device code 
outlining and shouldn't be part of this patch.

Enhanced CodeGen test to check that host part of the code is not 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/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.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,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);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ 

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

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

Hi @aaron.ballman,

Thanks a lot for the comments and sorry for the long delay. We've been working 
on complete implementation of the SYCL 1.2.1 specification.
Now I have more time to work on contributing the implementation to LLVM project.

I re-based the patch and started applying your suggestions. 
In addition to that I'd like to investigate slightly different approach to 
outlining suggested by @ABataev at LLVM Dev. Meeting conference and utilize the 
infrastructure OpenMP compiler uses in CodeGen library to emit "device part" of 
the single source.

Thanks,
Alexey


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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1034
+  let Spellings = [Clang<"sycl_kernel">];
+  let Subjects = SubjectList<[Function]>;
+  let LangOpts = [SYCL];

Shouldn't this be `FunctionTemplate` instead?



Comment at: clang/include/clang/Basic/AttrDocs.td:260
+The ``sycl_kernel`` attribute specifies that a function will be used by the
+compiler to outline device code and to generate OpenCL kernel.
+Here is a code example of the SYCL program, which demonstrates compiler's

generate an OpenCL kernel



Comment at: clang/include/clang/Basic/AttrDocs.td:261
+compiler to outline device code and to generate OpenCL kernel.
+Here is a code example of the SYCL program, which demonstrates compiler's
+outlining job:

demonstrates the compiler's



Comment at: clang/include/clang/Basic/AttrDocs.td:278
+The lambda that is passed to the ``parallel_for`` is called a SYCL "kernel
+function". A SYCL "kernel function" defines entry point to the "device part"
+of the code. Compiler will traverse all symbols accessible from a

defines the entry point



Comment at: clang/include/clang/Basic/AttrDocs.td:279
+function". A SYCL "kernel function" defines entry point to the "device part"
+of the code. Compiler will traverse all symbols accessible from a
+"kernel function" and add them to the "device part" of the code. In this code

The compiler will



Comment at: clang/include/clang/Basic/AttrDocs.td:281
+"kernel function" and add them to the "device part" of the code. In this code
+example, compiler will add "foo" function to the "device part" of the code.
+More details about compilation of functions for device can be found in the

the compiler will add the "foo" function



Comment at: clang/include/clang/Basic/AttrDocs.td:282
+example, compiler will add "foo" function to the "device part" of the code.
+More details about compilation of functions for device can be found in the
+SYCL 1.2.1 specification Section 6.4.

More details about the compilation of functions for the device part can be found



Comment at: clang/include/clang/Basic/AttrDocs.td:284
+SYCL 1.2.1 specification Section 6.4.
+To show to the compiler entry point to the "device part" of the code SYCL
+runtime can use the ``sycl_kernel`` attribute in the following way:

of the code, the SYCL runtime



Comment at: clang/include/clang/Basic/AttrDocs.td:308
+
+The compiler will also generate OpenCL kernel using the function marked with 
the
+``sycl_kernel`` attribute.

generate an OpenCL kernel



Comment at: clang/include/clang/Basic/AttrDocs.td:313
+
+- Function template with at least two template parameters is expected. The 
compiler
+generates OpenCL kernel and uses first template parameter as unique name to the

The function must be a template with at least two type template parameters.



Comment at: clang/include/clang/Basic/AttrDocs.td:314
+- Function template with at least two template parameters is expected. The 
compiler
+generates OpenCL kernel and uses first template parameter as unique name to the
+generated OpenCL kernel. Host application uses this unique name to invoke the

generates an OpenCL kernel and uses the first template parameter as a unique 
name



Comment at: clang/include/clang/Basic/AttrDocs.td:315
+generates OpenCL kernel and uses first template parameter as unique name to the
+generated OpenCL kernel. Host application uses this unique name to invoke the
+OpenCL kernel generated for the ``sycl_kernel_function`` specialized by

The host application uses



Comment at: clang/include/clang/Basic/AttrDocs.td:318
+this name and second template parameter ``KernelType`` (which might be a 
lambda type).
+- Function must have at least one parameter. First parameter expected to be a
+function object type (named or unnamed i.e. lambda). Compiler uses function

The function must
The first parameter is required to be a



Comment at: clang/include/clang/Basic/AttrDocs.td:319
+- Function must have at least one parameter. First parameter expected to be a
+function object type (named or unnamed i.e. lambda). Compiler uses function
+object type fields to generate OpenCL kernel parameters.

The compiler uses the function object type



Comment at: clang/include/clang/Basic/AttrDocs.td:321
+object type fields to generate OpenCL kernel parameters.
+- Function must return void. Compiler re-uses body of marked function to
+generate OpenCL kernel body and OpenCL kernel must return void.

The function must return void. The compiler reuses the 

[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);

[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}}
+
+// 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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}}

keryell wrote:
> bader wrote:
> > aaron.ballman wrote:
> > > Fznamznon wrote:
> > > > aaron.ballman wrote:
> > > > > I'd like to see some more tests covering less obvious scenarios. Can 
> > > > > I add this attribute to a lambda? What about a member function? How 
> > > > > does it work with virtual functions? That sort of thing.
> > > > Actually there is no restrictions for adding this attribute to any 
> > > > function to outline device code so I just checked the simplest variant.
> > > > 
> > > > But I'm working on new patch which will put some requirements on 
> > > > function which is marked with `sycl_kernel` attribute. 
> > > > This new patch will add generation of OpenCL kernel from function 
> > > > marked with `sycl_kernel` attribute. The main idea of this approach is 
> > > > described in this [[ 
> > > > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md#lowering-of-lambda-function-objects-and-named-function-objects
> > > >  | document ]] (in this document generated kernel is called "kernel 
> > > > wrapper").
> > > > And to be able to generate OpenCL kernel using function marked with 
> > > > `sycl_kernel` attribute we put some requirements on this function, for 
> > > > example it must be a template function. You can find these requirements 
> > > > and example of proper function which can be marked with `sycl_kernel` 
> > > > in this [[ https://github.com/intel/llvm/pull/177#discussion_r290451286 
> > > > | comment ]] .
> > > > 
> > > > 
> > > > Actually there is no restrictions for adding this attribute to any 
> > > > function to outline device code so I just checked the simplest variant.
> > > 
> > > So there are no concerns about code like:
> > > ```
> > > struct Base {
> > >   __attribute__((sycl_kernel)) virtual void foo();
> > >   virtual void bar();
> > > };
> > > 
> > > struct Derived : Base {
> > >   void foo() override;
> > >   __attribute__((sycl_kernel)) void bar() override;
> > > };
> > > 
> > > void f(Base *B, Derived *D) {
> > >   // Will all of these "do the right thing"?
> > >   B->foo();
> > >   B->bar();
> > > 
> > >   D->foo();
> > >   D->bar();
> > > }
> > > ```
> > > Actually there is no restrictions for adding this attribute to any 
> > > function to outline device code so I just checked the simplest variant.
> > > But I'm working on new patch which will put some requirements on function 
> > > which is marked with sycl_kernel attribute.
> > 
> > @aaron.ballman, sorry for confusing. The  usage scenarios should have been 
> > articulated more accurately.
> > We have only four uses of this attribute in our implementation:
> > https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L538
> >  (lines 538-605).
> > All four uses are applied to member functions of `cl::sycl::handler` class 
> > and all of them have similar prototype (which is mentioned by Mariya in the 
> > previous 
> > [comment](https://github.com/intel/llvm/pull/177#discussion_r290451286):
> > 
> > ```
> > namespace cl { namespace sycl {
> > class handler {
> >   template 
> >   __attribute__((sycl_kernel)) void sycl_kernel_function(KernelType 
> > KernelFuncObj) {
> > KernelFuncObj();
> >   }
> > };
> > }}
> > ```
> > 
> > Here is the list of SYCL device compiler expectations with regard to the 
> > function marked with `sycl_kernel` attribute.
> > - Function template with at least one parameter is expected. The 
> > compiler generates OpenCL kernel and uses first template parameter as 
> > unique name to the generated OpenCL kernel. Host application uses this 
> > unique name to invoke the OpenCL kernel generated for the 
> > `sycl_kernel_function` specialized by this name and KernelType (which might 
> > be a lambda type).
> > - Function must have at least one parameter. First parameter expected 
> > to be a function object type (named or unnamed i.e. lambda). Compiler uses 
> > function object type field to generate OpenCL kernel parameters.
> > 
> > Aaron, I hope it makes more sense now.
> > 
> > We don't plan in any use cases other than in SYCL standard library 
> > implementation mentioned above.
> > If I understand you concerns correctly, you want to be sure that clang 
> > prohibits other uses of this attribute, which are not intended. Right?
> > What is the best way to do this? Add more negative tests cases and make 
> > sure that clang generate error diagnostic messages?
> > 
> > If I understand you concerns correctly, you want to be sure that clang 
> > prohibits other uses of this attribute, which are not intended. Right?
> 
> But since it is an attribute to be used by SYCL 

[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] 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 () 

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

[Ronan Keryell via Phabricator via cfe-commits]

keryell 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}}

bader wrote:
> aaron.ballman wrote:
> > Fznamznon wrote:
> > > aaron.ballman wrote:
> > > > I'd like to see some more tests covering less obvious scenarios. Can I 
> > > > add this attribute to a lambda? What about a member function? How does 
> > > > it work with virtual functions? That sort of thing.
> > > Actually there is no restrictions for adding this attribute to any 
> > > function to outline device code so I just checked the simplest variant.
> > > 
> > > But I'm working on new patch which will put some requirements on function 
> > > which is marked with `sycl_kernel` attribute. 
> > > This new patch will add generation of OpenCL kernel from function marked 
> > > with `sycl_kernel` attribute. The main idea of this approach is described 
> > > in this [[ 
> > > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md#lowering-of-lambda-function-objects-and-named-function-objects
> > >  | document ]] (in this document generated kernel is called "kernel 
> > > wrapper").
> > > And to be able to generate OpenCL kernel using function marked with 
> > > `sycl_kernel` attribute we put some requirements on this function, for 
> > > example it must be a template function. You can find these requirements 
> > > and example of proper function which can be marked with `sycl_kernel` in 
> > > this [[ https://github.com/intel/llvm/pull/177#discussion_r290451286 | 
> > > comment ]] .
> > > 
> > > 
> > > Actually there is no restrictions for adding this attribute to any 
> > > function to outline device code so I just checked the simplest variant.
> > 
> > So there are no concerns about code like:
> > ```
> > struct Base {
> >   __attribute__((sycl_kernel)) virtual void foo();
> >   virtual void bar();
> > };
> > 
> > struct Derived : Base {
> >   void foo() override;
> >   __attribute__((sycl_kernel)) void bar() override;
> > };
> > 
> > void f(Base *B, Derived *D) {
> >   // Will all of these "do the right thing"?
> >   B->foo();
> >   B->bar();
> > 
> >   D->foo();
> >   D->bar();
> > }
> > ```
> > Actually there is no restrictions for adding this attribute to any function 
> > to outline device code so I just checked the simplest variant.
> > But I'm working on new patch which will put some requirements on function 
> > which is marked with sycl_kernel attribute.
> 
> @aaron.ballman, sorry for confusing. The  usage scenarios should have been 
> articulated more accurately.
> We have only four uses of this attribute in our implementation:
> https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L538 
> (lines 538-605).
> All four uses are applied to member functions of `cl::sycl::handler` class 
> and all of them have similar prototype (which is mentioned by Mariya in the 
> previous 
> [comment](https://github.com/intel/llvm/pull/177#discussion_r290451286):
> 
> ```
> namespace cl { namespace sycl {
> class handler {
>   template 
>   __attribute__((sycl_kernel)) void sycl_kernel_function(KernelType 
> KernelFuncObj) {
> KernelFuncObj();
>   }
> };
> }}
> ```
> 
> Here is the list of SYCL device compiler expectations with regard to the 
> function marked with `sycl_kernel` attribute.
>   - Function template with at least one parameter is expected. The 
> compiler generates OpenCL kernel and uses first template parameter as unique 
> name to the generated OpenCL kernel. Host application uses this unique name 
> to invoke the OpenCL kernel generated for the `sycl_kernel_function` 
> specialized by this name and KernelType (which might be a lambda type).
>   - Function must have at least one parameter. First parameter expected 
> to be a function object type (named or unnamed i.e. lambda). Compiler uses 
> function object type field to generate OpenCL kernel parameters.
> 
> Aaron, I hope it makes more sense now.
> 
> We don't plan in any use cases other than in SYCL standard library 
> implementation mentioned above.
> If I understand you concerns correctly, you want to be sure that clang 
> prohibits other uses of this attribute, which are not intended. Right?
> What is the best way to do this? Add more negative tests cases and make sure 
> that clang generate error diagnostic messages?
> 
> If I understand you concerns correctly, you want to be sure that clang 
> prohibits other uses of this attribute, which are not intended. Right?

But since it is an attribute to be used by SYCL run-time writers, I am not sure 
there is a lot of value in over-engineering the restrictions of its use. It 
diverts brain power from the real implementation & review and might even 
prevent innovation due to some creative 

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

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



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

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

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

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

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

Aaron, I hope it makes more sense now.

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



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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}}

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

So there are no concerns about code like:
```
struct Base {
  __attribute__((sycl_kernel)) virtual void foo();
  virtual void bar();
};

struct Derived : Base {
  void foo() override;
  __attribute__((sycl_kernel)) void bar() override;
};

void f(Base *B, Derived *D) {
  // Will all of these "do the right thing"?
  B->foo();
  B->bar();

  D->foo();
  D->bar();
}
```


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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Most of the comments are about minor nits like grammar and coding conventions, 
but I did have some questions regarding what kinds of functions the sycl_kernel 
attribute gets applied to. Also, I'd like to see some additional tests that 
demonstrate the sycl device attribute is being implicitly created on the proper 
declarations as expected (can probably do that using -ast-dump and checking to 
see if the right functions have the attribute attached).




Comment at: clang/include/clang/Basic/AttrDocs.td:259
+  let Content = [{
+The ``sycl_kernel`` attribute specifies that a function is SYCL "kernel
+function". SYCL "kernel function" defines an entry point to a kernel.

is SYCL "kernel function" -> is a SYCL "kernel function"



Comment at: clang/include/clang/Basic/AttrDocs.td:260
+The ``sycl_kernel`` attribute specifies that a function is SYCL "kernel
+function". SYCL "kernel function" defines an entry point to a kernel.
+Kernel is a function which will be compiled for the device and defines some

SYCL -> A SYCL



Comment at: clang/include/clang/Basic/AttrDocs.td:261
+function". SYCL "kernel function" defines an entry point to a kernel.
+Kernel is a function which will be compiled for the device and defines some
+entry point to device code i.e. will be called by host in run time.

Kernel is a -> A kernel is a



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

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.



Comment at: clang/include/clang/Basic/AttrDocs.td:278
+  }
+The lambda that is passed to the ``parallel_for`` is called SYCL
+"kernel function".

called SYLC -> called a SYLC



Comment at: clang/include/clang/Basic/AttrDocs.td:280
+"kernel function".
+The SYCL Runtime implementation will use ``sycl_kernel`` attribute to mark this
+lambda as SYCL "kernel function". Compiler is supposed to construct a kernel

use sycl_kernel -> use the sycl_kernel



Comment at: clang/include/clang/Basic/AttrDocs.td:281
+The SYCL Runtime implementation will use ``sycl_kernel`` attribute to mark this
+lambda as SYCL "kernel function". Compiler is supposed to construct a kernel
+from "kernel function", add it to the "device part" of code and traverse all

as SYCL -> as a SYCL
Compiler is supposed to -> The compiler will



Comment at: clang/include/clang/Basic/AttrDocs.td:284
+symbols accessible from "kernel function" and add them to the "device part" of
+the code. In this code example compiler is supposed to add "foo" function to 
the
+"device part" of the code.

In this code example compiler is supposed to add "foo" function -> In this code 
example, the compiler will add the "foo" function



Comment at: clang/include/clang/Sema/Sema.h:11182
+private:
+  /// Contains Function declarations to be added to the SYCL device code.
+  /// In SYCL when we generate device code we don't know which functions we 
will

Function -> function



Comment at: clang/include/clang/Sema/Sema.h:11183
+  /// Contains Function declarations to be added to the SYCL device code.
+  /// In SYCL when we generate device code we don't know which functions we 
will
+  /// emit before we emit sycl kernels so we add device functions to this array

In SYCL, when we generate device code, we don't know 



Comment at: clang/include/clang/Sema/Sema.h:11184
+  /// In SYCL when we generate device code we don't know which functions we 
will
+  /// emit before we emit sycl kernels so we add device functions to this array
+  /// and handle it in separate way.

we emit sycl kernels, so we add device 



Comment at: clang/include/clang/Sema/Sema.h:11189
+public:
+  /// This function adds function declaration to the SYCL device code.
+  void AddSyclDeviceFunc(Decl *D) {

adds the function declaration



Comment at: clang/include/clang/Sema/Sema.h:11190
+  /// This function adds function declaration to the SYCL device code.
+  void AddSyclDeviceFunc(Decl *D) {
+D->addAttr(SYCLDeviceAttr::CreateImplicit(Context));

Should be named `addSyclDeviceFunc()` per coding standards. Similar for the 
other new functions.



Comment at: clang/include/clang/Sema/Sema.h:11194
+  }
+  /// SyclDeviceFuncs - access to SYCL device function decls.
+  SmallVector () { return 

[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

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

[Aaron Ballman via cfe-commits]

Richard Smith, but his time is sometimes limited. John McCall would also be
a great choice. Otherwise, I am happy to review when back next week.

~Aaron

On Tue, Jun 11, 2019, 4:45 PM Bader, Alexey  wrote:

> Hi Aaron,
>
>
>
> I think Anastasia is on vacation.
>
> Could you recommend someone who can be “trusted reviewer” for this change,
> please?
>
> If no, we will wait for your/Anastasia review.
>
>
>
> Thanks,
>
> Alexey
>
>
>
> *From:* Aaron Ballman 
> *Sent:* Tuesday, June 11, 2019 5:04 PM
> *To:* reviews+d60455+public+bc9d5bb3412ac...@reviews.llvm.org
> *Cc:* Podchishchaeva, Mariya ; Justin
> Lebar ; ronan-l...@keryell.fr; v.lomul...@gmail.com;
> a.bat...@hotmail.com; anastasia.stul...@arm.com; Bader, Alexey <
> alexey.ba...@intel.com>; mgo...@gentoo.org; Maslov, Oleg <
> oleg.mas...@intel.com>; Gainullin, Artur ;
> b00234...@studentmail.uws.ac.uk; bevin.hans...@ericsson.com;
> cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com;
> shen...@google.com
> *Subject:* Re: [PATCH] D60455: [SYCL] Implement SYCL device code outlining
>
>
>
> I'm out of the office until next week and won't be able to review it until
> then. If another trusted reviewer accepts, I can always do post commit
> review.
>
>
>
> ~Aaron
>
>
>
> On Tue, Jun 11, 2019, 2:43 PM Mariya Podchishchaeva via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> 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

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

[Bader, Alexey via cfe-commits]

Hi Aaron,

I think Anastasia is on vacation.
Could you recommend someone who can be “trusted reviewer” for this change, 
please?
If no, we will wait for your/Anastasia review.

Thanks,
Alexey

From: Aaron Ballman 
Sent: Tuesday, June 11, 2019 5:04 PM
To: reviews+d60455+public+bc9d5bb3412ac...@reviews.llvm.org
Cc: Podchishchaeva, Mariya ; Justin Lebar 
; ronan-l...@keryell.fr; v.lomul...@gmail.com; 
a.bat...@hotmail.com; anastasia.stul...@arm.com; Bader, Alexey 
; mgo...@gentoo.org; Maslov, Oleg 
; Gainullin, Artur ; 
b00234...@studentmail.uws.ac.uk; bevin.hans...@ericsson.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: Re: [PATCH] D60455: [SYCL] Implement SYCL device code outlining

I'm out of the office until next week and won't be able to review it until 
then. If another trusted reviewer accepts, I can always do post commit review.

~Aaron

On Tue, Jun 11, 2019, 2:43 PM Mariya Podchishchaeva via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
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

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

[Aaron Ballman via cfe-commits]

I'm out of the office until next week and won't be able to review it until
then. If another trusted reviewer accepts, I can always do post commit
review.

~Aaron

On Tue, Jun 11, 2019, 2:43 PM Mariya Podchishchaeva via Phabricator <
revi...@reviews.llvm.org> wrote:

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

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

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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia 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

Fznamznon wrote:
> 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.
Just FYI at some point we generalized SPIR calling convention to be used for 
kernels irrespective from target by default (see 
`TargetCodeGenInfo::getOpenCLKernelCallingConv`). Not sure if it might make 
sense to do for SYCL device functions too. I am not saying it belongs to this 
patch though.


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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia 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

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?


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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



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

Undocumented -> SYCLKernelDocs



Comment at: clang/include/clang/Basic/AttrDocs.td:269
+
+  using namespace cl::sycl;
+  queue Q;

The example doesn't demonstrate the use of the attribute.

It explains how it is used by the toolchain only!

May be @aaron.ballman can help here as I am not sure what the format should be.



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

Do you also need to prevent generation of non-device functions somehow?



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

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





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

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.



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

I can't see where the SPIR calling convention is currently set for SYCL?



Comment at: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp:3
+// 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

I don't think this comment is necessary.


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 

[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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D60455#1518178 , @bader wrote:

> In D60455#1513499 , @Anastasia wrote:
>
> > //**Design question**//: since you are not aware what functions are to be 
> > run on a device while parsing them, at what point do you plan to diagnose 
> > the invalid behavior from the standard C++ i.e. using function pointers in 
> > kernel code is likely to cause issues?
>
>
> We are going to use DeviceDiagBuilder and related infrastructure implemented 
> in Clang to diagnose device side code errors in OpenMP/CUDA modes.
>  More details are in the comments here:
>  
> https://clang.llvm.org/doxygen/classclang_1_1Sema_1_1DeviceDiagBuilder.html#details


Just a thought, if you parse host code first and provide the device outlining 
information to the device compilation phase would you then be able to reuse 
more parsing functionality from OpenCL?

> 
> 
>> Also do you need to outline the data structures too? For example classes 
>> used on device are not allowed to have virtual function.
> 
> Yes. This restriction is already implemented in our code base on GitHub.

Cool, is it implemented in `SemaSYCL.cpp` too?




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

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

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

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D60455#1513499 , @Anastasia wrote:

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


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

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

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




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

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

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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

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

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




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

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



You are adding an attribute that is exposed to the programmers that use clang 
to compile their code, so unless you come up with some way to reject it in the 
non-toolchain mode it has to be documented. And for clang it will become 
"hidden" SYCL dialect so absolutely not different to `__kernel`.

Another aspect to consider is that clang used `TypePrinter` in diagnostics and 
even though printing entire function signature is rare it might appear in 
diagnostics and the programmer should have a way to understand what the "alien" 
construct is. This is where clang documentation will help.



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

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

[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

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



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

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

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

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

> Also did __kernel attribute not work at the end?

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

@keryell :

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

@agozillon :

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

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

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

Let me know how I can help here.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



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

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.


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 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 &&
+