From 393db1e20a369b6b7d568859ec77f69f7e659573 Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Wed, 1 Jul 2020 12:55:35 -0400 Subject: [PATCH 1/7] Error for implicit capture of this pointer inside a sycl kernel Change-Id: Icf4145a3cf3732a0ff28f0ada9880a24b97b5a0d Signed-off-by: Arvind Sudarsanam --- .../clang/Basic/DiagnosticSemaKinds.td | 3 ++ clang/lib/Sema/SemaSYCL.cpp | 10 +++++++ .../SemaSYCL/lambda_implicit_capture_this.cpp | 29 +++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 clang/test/SemaSYCL/lambda_implicit_capture_this.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 2da008862f695..74a8dd3560626 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7306,6 +7306,9 @@ let CategoryName = "Lambda Issue" in { "here">; def note_var_explicitly_captured_here : Note<"variable %0 is" "%select{| explicitly}1 captured here">; + def err_implicit_this_capture : Error< + "implicit capture of 'this' with a capture default of '=' is not allowed " + "for kernel functions in SYCL 1.2.1">; // C++14 lambda init-captures. def warn_cxx11_compat_init_capture : Warning< diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 09d5a45b306d0..a16ba1e51525d 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1652,6 +1652,16 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, constructKernelName(*this, KernelCallerFunc, MC); StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName : CalculatedName); + if (KernelLambda->isLambda()) { + llvm::DenseMap Captures; + FieldDecl *ThisCapture; + KernelLambda->getCaptureFields(Captures, ThisCapture); + for (const LambdaCapture &LC : KernelLambda->captures()) { + if (LC.capturesThis() && LC.isImplicit()) { + Diag(LC.getLocation(), diag::err_implicit_this_capture); + } + } + } SyclKernelFieldChecker checker(*this); SyclKernelDeclCreator kernel_decl( *this, checker, KernelName, KernelLambda->getLocation(), diff --git a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp new file mode 100644 index 0000000000000..c4c279d287bfd --- /dev/null +++ b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -I %S/Inputs -fsycl -fsycl-is-device -verify %s + +#include + +template +__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { + kernelFunc(); +} + +class Class { +public: + Class() : member(1) {} + void function(); + int member; +}; + +void Class::function() { + kernel( + [=]() { + int acc[1] = {5}; + acc[0] *= member; // expected-error{{implicit capture of 'this' with a capture default of '=' is not allowed for kernel functions in SYCL 1.2.1}} + }); +} + +int main(int argc, char *argv[]) { + Class c; + c.function(); +} + From d86da9466fdbe3c3ac07c31bd035b47f409c77e3 Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Thu, 2 Jul 2020 08:47:58 -0700 Subject: [PATCH 2/7] fixing a minor formatting issue Signed-off-by: Arvind Sudarsanam --- clang/test/SemaSYCL/lambda_implicit_capture_this.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp index c4c279d287bfd..4485c35194f4e 100644 --- a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp +++ b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp @@ -26,4 +26,3 @@ int main(int argc, char *argv[]) { Class c; c.function(); } - From 8ec2400b4a07116003550244a837cd0af12da804 Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Tue, 14 Jul 2020 09:16:48 -0700 Subject: [PATCH 3/7] Incorporating code review comments and test failure fix Signed-off-by: Arvind Sudarsanam --- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/Sema/SemaSYCL.cpp | 3 --- .../SemaSYCL/lambda_implicit_capture_this.cpp | 2 +- sycl/include/CL/sycl/handler.hpp | 24 +++++++++---------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 74a8dd3560626..9fa65c1fca637 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7307,8 +7307,8 @@ let CategoryName = "Lambda Issue" in { def note_var_explicitly_captured_here : Note<"variable %0 is" "%select{| explicitly}1 captured here">; def err_implicit_this_capture : Error< - "implicit capture of 'this' with a capture default of '=' is not allowed " - "for kernel functions in SYCL 1.2.1">; + "implicit capture of 'this' is not allowed for kernel functions in " + "SYCL 1.2.1">; // C++14 lambda init-captures. def warn_cxx11_compat_init_capture : Warning< diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index a16ba1e51525d..b8c02a4fe68b6 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1653,9 +1653,6 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName : CalculatedName); if (KernelLambda->isLambda()) { - llvm::DenseMap Captures; - FieldDecl *ThisCapture; - KernelLambda->getCaptureFields(Captures, ThisCapture); for (const LambdaCapture &LC : KernelLambda->captures()) { if (LC.capturesThis() && LC.isImplicit()) { Diag(LC.getLocation(), diag::err_implicit_this_capture); diff --git a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp index 4485c35194f4e..a27e076538fe7 100644 --- a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp +++ b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp @@ -18,7 +18,7 @@ void Class::function() { kernel( [=]() { int acc[1] = {5}; - acc[0] *= member; // expected-error{{implicit capture of 'this' with a capture default of '=' is not allowed for kernel functions in SYCL 1.2.1}} + acc[0] *= member; // expected-error{{implicit capture of 'this' is not allowed for kernel functions in SYCL 1.2.1}} }); } diff --git a/sycl/include/CL/sycl/handler.hpp b/sycl/include/CL/sycl/handler.hpp index 43642b75a3497..ffd2b1585ff6a 100644 --- a/sycl/include/CL/sycl/handler.hpp +++ b/sycl/include/CL/sycl/handler.hpp @@ -504,47 +504,47 @@ class __SYCL_EXPORT handler { template - detail::enable_if_t - readFromFirstAccElement(accessor Src) const { + static detail::enable_if_t + readFromFirstAccElement(accessor Src) { atomic AtomicSrc = Src; return AtomicSrc.load(); } template - detail::enable_if_t<(Dim > 0) && Mode == access::mode::atomic, T> - readFromFirstAccElement(accessor Src) const { + static detail::enable_if_t<(Dim > 0) && Mode == access::mode::atomic, T> + readFromFirstAccElement(accessor Src) { id Id = getDelinearizedIndex(Src.get_range(), 0); return Src[Id].load(); } template - detail::enable_if_t - readFromFirstAccElement(accessor Src) const { + static detail::enable_if_t + readFromFirstAccElement(accessor Src) { return *(Src.get_pointer()); } template - detail::enable_if_t - writeToFirstAccElement(accessor Dst, T V) const { + static detail::enable_if_t + writeToFirstAccElement(accessor Dst, T V) { atomic AtomicDst = Dst; AtomicDst.store(V); } template - detail::enable_if_t<(Dim > 0) && Mode == access::mode::atomic, void> - writeToFirstAccElement(accessor Dst, T V) const { + static detail::enable_if_t<(Dim > 0) && Mode == access::mode::atomic, void> + writeToFirstAccElement(accessor Dst, T V) { id Id = getDelinearizedIndex(Dst.get_range(), 0); Dst[Id].store(V); } template - detail::enable_if_t - writeToFirstAccElement(accessor Dst, T V) const { + static detail::enable_if_t + writeToFirstAccElement(accessor Dst, T V) { *(Dst.get_pointer()) = V; } From f52424eaf2cab70e80d38e7b76ce8d9977343c1d Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Tue, 14 Jul 2020 10:49:02 -0700 Subject: [PATCH 4/7] Variable name changed Signed-off-by: Arvind Sudarsanam --- clang/lib/Sema/SemaSYCL.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index b8fdcb2e03220..3c3cef063b594 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1885,8 +1885,8 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, constructKernelName(*this, KernelCallerFunc, MC); StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName : CalculatedName); - if (KernelLambda->isLambda()) { - for (const LambdaCapture &LC : KernelLambda->captures()) { + if (KernelObj->isLambda()) { + for (const LambdaCapture &LC : KernelObj->captures()) { if (LC.capturesThis() && LC.isImplicit()) { Diag(LC.getLocation(), diag::err_implicit_this_capture); } From 619cf1be75dce174dff96567d71ec317004db605 Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Mon, 20 Jul 2020 07:08:32 -0700 Subject: [PATCH 5/7] updating testcase based on comments Signed-off-by: Arvind Sudarsanam --- clang/test/SemaSYCL/lambda_implicit_capture_this.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp index a27e076538fe7..12e4b659ddaad 100644 --- a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp +++ b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp @@ -1,6 +1,4 @@ -// RUN: %clang_cc1 -I %S/Inputs -fsycl -fsycl-is-device -verify %s - -#include +// RUN: %clang_cc1 -I %S/Inputs -fsycl -fsycl-is-device -fsyntax-only -verify %s template __attribute__((sycl_kernel)) void kernel(Func kernelFunc) { From cb5840cc70fe9531cf67e8089ae72f52b921c90e Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Tue, 21 Jul 2020 10:06:46 -0700 Subject: [PATCH 6/7] updating error message Signed-off-by: Arvind Sudarsanam --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 +-- clang/test/SemaSYCL/lambda_implicit_capture_this.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b904b5a69b4f7..145b983f2c623 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7310,8 +7310,7 @@ let CategoryName = "Lambda Issue" in { def note_var_explicitly_captured_here : Note<"variable %0 is" "%select{| explicitly}1 captured here">; def err_implicit_this_capture : Error< - "implicit capture of 'this' is not allowed for kernel functions in " - "SYCL 1.2.1">; + "implicit capture of 'this' is not allowed for kernel functions">; // C++14 lambda init-captures. def warn_cxx11_compat_init_capture : Warning< diff --git a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp index 12e4b659ddaad..331eb1b64ae3e 100644 --- a/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp +++ b/clang/test/SemaSYCL/lambda_implicit_capture_this.cpp @@ -16,7 +16,7 @@ void Class::function() { kernel( [=]() { int acc[1] = {5}; - acc[0] *= member; // expected-error{{implicit capture of 'this' is not allowed for kernel functions in SYCL 1.2.1}} + acc[0] *= member; // expected-error{{implicit capture of 'this' is not allowed for kernel functions}} }); } From bec8fd038e28e923935f35091245a77d8eedd29a Mon Sep 17 00:00:00 2001 From: Arvind Sudarsanam Date: Tue, 21 Jul 2020 10:40:10 -0700 Subject: [PATCH 7/7] complying to llvm coding standards Signed-off-by: Arvind Sudarsanam --- clang/lib/Sema/SemaSYCL.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index ad1e6391ef721..ef836df159d61 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1944,11 +1944,9 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName : CalculatedName); if (KernelObj->isLambda()) { - for (const LambdaCapture &LC : KernelObj->captures()) { - if (LC.capturesThis() && LC.isImplicit()) { + for (const LambdaCapture &LC : KernelObj->captures()) + if (LC.capturesThis() && LC.isImplicit()) Diag(LC.getLocation(), diag::err_implicit_this_capture); - } - } } SyclKernelFieldChecker checker(*this); SyclKernelDeclCreator kernel_decl(