From e713d278c693960fcafa190dcf4a8ddf505152f6 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 8 Mar 2021 10:19:24 -0800 Subject: [PATCH 1/3] [SYCL] Allow non-evaluated globals to be used in a kernel. As requested to support SYCL-2020 specialization constants, this makes our global variable diagnostics not happen unless it is in a runtime evaluated context. --- clang/lib/Sema/SemaExpr.cpp | 13 +++++++++---- clang/test/SemaSYCL/sycl-restrict.cpp | 7 +++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index ff5efe0398bc9..9caee9969125f 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -215,17 +215,22 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, if (getLangOpts().SYCLIsDevice) { if (auto VD = dyn_cast(D)) { bool IsConst = VD->getType().isConstant(Context); - if (!IsConst && VD->getStorageClass() == SC_Static) + bool IsRuntimeEvaluated = + ExprEvalContexts.empty() || + (!ExprEvalContexts.back().isUnevaluated() && + !ExprEvalContexts.back().isConstantEvaluated()); + if (IsRuntimeEvaluated && !IsConst && VD->getStorageClass() == SC_Static) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable; // Non-const globals are allowed for SYCL explicit SIMD. - else if (!isSYCLEsimdPrivateGlobal(VD) && !IsConst && - VD->hasGlobalStorage() && !isa(VD)) + else if (IsRuntimeEvaluated && !isSYCLEsimdPrivateGlobal(VD) && + !IsConst && VD->hasGlobalStorage() && !isa(VD)) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelGlobalVariable; // Disallow const statics and globals that are not zero-initialized // or constant-initialized. - else if (IsConst && VD->hasGlobalStorage() && !VD->isConstexpr() && + else if (IsRuntimeEvaluated && IsConst && VD->hasGlobalStorage() && + !VD->isConstexpr() && !checkAllowedSYCLInitializer(VD, /*CheckValueDependent =*/true)) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelConstStaticVariable; diff --git a/clang/test/SemaSYCL/sycl-restrict.cpp b/clang/test/SemaSYCL/sycl-restrict.cpp index 933fb6bd9f605..94312d4c6f962 100644 --- a/clang/test/SemaSYCL/sycl-restrict.cpp +++ b/clang/test/SemaSYCL/sycl-restrict.cpp @@ -384,6 +384,10 @@ int moar_globals = 5; } } +template +int uses_global(){} + + int addInt(int n, int m) { return n + m; } @@ -401,6 +405,9 @@ int use2(a_type ab, a_type *abp) { if (ab.fm()) // expected-note {{called by 'use2'}} return 0; + // No error, as this is not in an evaluated context. + (void)(uses_global() + uses_global()); + return another_global; // expected-error {{SYCL kernel cannot use a non-const global variable}} return ns::glob + // expected-error {{SYCL kernel cannot use a non-const global variable}} From eb63d90f5e4d49fe121e3f0593f08d0da004c134 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 8 Mar 2021 11:54:54 -0800 Subject: [PATCH 2/3] Change how we check to use Sema directly --- clang/lib/Sema/SemaExpr.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 9caee9969125f..bebd040c3a7be 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -215,10 +215,8 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, if (getLangOpts().SYCLIsDevice) { if (auto VD = dyn_cast(D)) { bool IsConst = VD->getType().isConstant(Context); - bool IsRuntimeEvaluated = - ExprEvalContexts.empty() || - (!ExprEvalContexts.back().isUnevaluated() && - !ExprEvalContexts.back().isConstantEvaluated()); + bool IsRuntimeEvaluated = ExprEvalContexts.empty() || + isUnevaluatedContext() || isConstantEvaluated(); if (IsRuntimeEvaluated && !IsConst && VD->getStorageClass() == SC_Static) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable; From 225e57226306a2a827621f4b6ebdd49693506eb6 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 9 Mar 2021 06:40:54 -0800 Subject: [PATCH 3/3] Fix build failures... Apparently I disabled my brain for a bit. Seemingly when changing over to the Sema calls directly (instead of going through the Evaluation contexts) I completely messed up the logic. It wasn't even a difficult change, it should have been a simple 'replace this call with that', but instead I mangled it beyond belief. I blame Monday. --- clang/lib/Sema/SemaExpr.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index bebd040c3a7be..76c68a2bb17d3 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -215,8 +215,9 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, if (getLangOpts().SYCLIsDevice) { if (auto VD = dyn_cast(D)) { bool IsConst = VD->getType().isConstant(Context); - bool IsRuntimeEvaluated = ExprEvalContexts.empty() || - isUnevaluatedContext() || isConstantEvaluated(); + bool IsRuntimeEvaluated = + ExprEvalContexts.empty() || + (!isUnevaluatedContext() && !isConstantEvaluated()); if (IsRuntimeEvaluated && !IsConst && VD->getStorageClass() == SC_Static) SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) << Sema::KernelNonConstStaticDataVariable;