From e064bd52a81d3be62c89ad455a6ce65f8309f892 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 10 Mar 2020 14:47:00 -0700 Subject: [PATCH 1/2] [SYCL] Implement thread-local storage restriction, move vardecl checks The SYCL spec was recently clarified to prohibit thread local storage, so this commit ensures an error is always emitted in these cases. Additionally, the DeclRefExpr to a VarDecl were switched to delayed diagnostics. Signed-off-by: Erich Keane --- clang/lib/Sema/SemaDecl.cpp | 7 ++- clang/lib/Sema/SemaExpr.cpp | 9 +++- clang/lib/Sema/SemaSYCL.cpp | 8 --- clang/test/SemaSYCL/prohibit-thread-local.cpp | 49 +++++++++++++++++++ clang/test/SemaSYCL/tls_error.cpp | 2 + 5 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 clang/test/SemaSYCL/prohibit-thread-local.cpp diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 414c18b492816..b44fccf273f2d 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -7060,10 +7060,13 @@ NamedDecl *Sema::ActOnVariableDeclarator( // Static variables declared inside SYCL device code must be const or // constexpr - if (getLangOpts().SYCLIsDevice && SCSpec == DeclSpec::SCS_static && - !R.isConstant(Context)) + if (getLangOpts().SYCLIsDevice) { + if (SCSpec == DeclSpec::SCS_static && !R.isConstant(Context)) SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable; + else if (NewVD->getTSCSpec() == DeclSpec::TSCS_thread_local) + SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_thread_unsupported); + } switch (D.getDeclSpec().getConstexprSpecifier()) { case CSK_unspecified: diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index e7d7b889f6634..5afac8774656d 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -212,10 +212,17 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, ObjCInterfaceDecl *ClassReceiver) { if (getLangOpts().SYCLIsDevice) { if (auto VD = dyn_cast(D)) { + bool IsConst = VD->getType().isConstant(Context); + if (VD->getTLSKind() != VarDecl::TLS_None) + SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_thread_unsupported); + if (VD->getStorageClass() == SC_Static && - !VD->getType().isConstant(Context)) + !IsConst) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable; + else if (VD->hasGlobalStorage() && !isa(VD) && !IsConst) + SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) + << Sema::KernelGlobalVariable; } } diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 26cc8916fae5e..19b77c6760ec1 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -322,14 +322,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor { CheckSYCLType(E->getType(), E->getSourceRange()); if (VarDecl *VD = dyn_cast(D)) { - bool IsConst = VD->getType().getNonReferenceType().isConstQualified(); - if (!IsConst && VD->hasGlobalStorage() && !VD->isStaticLocal() && - !VD->isStaticDataMember() && !isa(VD)) { - if (VD->getTLSKind() != VarDecl::TLS_None) - SemaRef.Diag(E->getLocation(), diag::err_thread_unsupported); - SemaRef.Diag(E->getLocation(), diag::err_sycl_restrict) - << Sema::KernelGlobalVariable; - } if (!VD->isLocalVarDeclOrParm() && VD->hasGlobalStorage()) { VD->addAttr(SYCLDeviceAttr::CreateImplicit(SemaRef.Context)); SemaRef.addSyclDeviceDecl(VD); diff --git a/clang/test/SemaSYCL/prohibit-thread-local.cpp b/clang/test/SemaSYCL/prohibit-thread-local.cpp new file mode 100644 index 0000000000000..ed2987d4f96ec --- /dev/null +++ b/clang/test/SemaSYCL/prohibit-thread-local.cpp @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -fsycl-is-device -verify -fsyntax-only -std=c++17 %s + +thread_local const int prohobit_ns_scope = 0; +thread_local int prohobit_ns_scope2 = 0; +thread_local const int allow_ns_scope = 0; + +struct S { + static const thread_local int prohibit_static_member; + static thread_local int prohibit_static_member2; +}; + +struct T { + static const thread_local int allow_static_member; +}; + +void foo() { + // expected-error@+1{{thread-local storage is not supported for the current target}} + thread_local const int prohibit_local = 0; + // expected-error@+1{{thread-local storage is not supported for the current target}} + thread_local int prohibit_local2; +} + +void bar() { thread_local int allow_local; } + +void usage() { + // expected-note@+1 {{called by}} + foo(); + // expected-error@+1 {{thread-local storage is not supported for the current target}} + (void)prohobit_ns_scope; + // expected-error@+2 {{SYCL kernel cannot use a non-const global variable}} + // expected-error@+1 {{thread-local storage is not supported for the current target}} + (void)prohobit_ns_scope2; + // expected-error@+1 {{thread-local storage is not supported for the current target}} + (void)S::prohibit_static_member; + // expected-error@+2 {{SYCL kernel cannot use a non-const static data variable}} + // expected-error@+1 {{thread-local storage is not supported for the current target}} + (void)S::prohibit_static_member2; +} + +template +__attribute__((sycl_kernel)) +// expected-note@+1 2{{called by}} +void kernel_single_task(Func kernelFunc) { kernelFunc(); } + +int main() { + // expected-note@+1 2{{called by}} + kernel_single_task([](){ usage(); } ); + return 0; +} diff --git a/clang/test/SemaSYCL/tls_error.cpp b/clang/test/SemaSYCL/tls_error.cpp index 5f8e14c6d1a76..95284d3512345 100644 --- a/clang/test/SemaSYCL/tls_error.cpp +++ b/clang/test/SemaSYCL/tls_error.cpp @@ -15,10 +15,12 @@ void usage() { template __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) { + //expected-note@+1{{called by}} kernelFunc(); } int main() { + //expected-note@+1{{called by}} kernel_single_task([]() { usage(); }); return 0; } From bcbc2f18eae50153be4c803a6530ac9728f0d885 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 10 Mar 2020 15:24:35 -0700 Subject: [PATCH 2/2] Relocate IsConst checks as requested by Prem, others Also fixes up the issues found by the bots, including the lacking signed-off-by. Signed-off-by: Erich Keane --- clang/lib/Sema/SemaDecl.cpp | 4 ++-- clang/lib/Sema/SemaExpr.cpp | 5 ++--- clang/test/SemaSYCL/prohibit-thread-local.cpp | 9 +++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index b44fccf273f2d..ce5a799712001 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -7062,8 +7062,8 @@ NamedDecl *Sema::ActOnVariableDeclarator( // constexpr if (getLangOpts().SYCLIsDevice) { if (SCSpec == DeclSpec::SCS_static && !R.isConstant(Context)) - SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_sycl_restrict) - << Sema::KernelNonConstStaticDataVariable; + SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_sycl_restrict) + << Sema::KernelNonConstStaticDataVariable; else if (NewVD->getTSCSpec() == DeclSpec::TSCS_thread_local) SYCLDiagIfDeviceCode(D.getIdentifierLoc(), diag::err_thread_unsupported); } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 5afac8774656d..6897085a7c6af 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -216,11 +216,10 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, if (VD->getTLSKind() != VarDecl::TLS_None) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_thread_unsupported); - if (VD->getStorageClass() == SC_Static && - !IsConst) + if (!IsConst && VD->getStorageClass() == SC_Static) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable; - else if (VD->hasGlobalStorage() && !isa(VD) && !IsConst) + else if (!IsConst && VD->hasGlobalStorage() && !isa(VD)) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelGlobalVariable; } diff --git a/clang/test/SemaSYCL/prohibit-thread-local.cpp b/clang/test/SemaSYCL/prohibit-thread-local.cpp index ed2987d4f96ec..0bb2cf3a180e0 100644 --- a/clang/test/SemaSYCL/prohibit-thread-local.cpp +++ b/clang/test/SemaSYCL/prohibit-thread-local.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsycl-is-device -verify -fsyntax-only -std=c++17 %s +// RUN: %clang_cc1 -fsycl-is-device -verify -fsyntax-only %s thread_local const int prohobit_ns_scope = 0; thread_local int prohobit_ns_scope2 = 0; @@ -39,11 +39,12 @@ void usage() { template __attribute__((sycl_kernel)) -// expected-note@+1 2{{called by}} -void kernel_single_task(Func kernelFunc) { kernelFunc(); } +// expected-note@+2 2{{called by}} +void +kernel_single_task(Func kernelFunc) { kernelFunc(); } int main() { // expected-note@+1 2{{called by}} - kernel_single_task([](){ usage(); } ); + kernel_single_task([]() { usage(); }); return 0; }