From 80c1692f71dcb0cd6fb5ec1818a2f42b69665145 Mon Sep 17 00:00:00 2001 From: Mariya Podchishchaeva Date: Tue, 21 Dec 2021 18:54:31 +0300 Subject: [PATCH 1/2] [SYCL] Error out instead of crash when templated global var used SYCL deferred diagnostics couldn't catch the case like: ``` template class randomType { public: randomType() {} T val; }; template const randomType Var; SYCL_EXTERNAL void foo() { (void)Var; } ``` And CodeGen was crashing due to attempt to emit invalid cast. That happened because use of `Var` should be diagnosed. The diagnostic wasn't emitted because at the places where deferred diagnostic is issued for this case, templated variable doesn't yet has initializer, or it has it but current lexical context is out of device code (in this particular case it will be a translation unit). This patch follows the logic invented by `finalizeOpenMPDelayedAnalysis` and `finalizeSYCLDelayedAnalysis` routines, i.e. emit immediate diagnostic when post-parsing of AST for deferred diagnostics happens. --- clang/lib/Sema/Sema.cpp | 9 +++++++++ clang/lib/Sema/SemaDecl.cpp | 10 ++++------ clang/lib/Sema/SemaExpr.cpp | 6 ------ clang/lib/Sema/SemaSYCL.cpp | 5 ----- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 4 ---- clang/test/SemaSYCL/inline-asm.cpp | 4 ++-- clang/test/SemaSYCL/sycl-device-const-static.cpp | 14 +++++++++++++- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index ca09400f98330..c26e5657565a2 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1679,6 +1679,15 @@ class DeferredDiagnosticsEmitter } void visitUsedDecl(SourceLocation Loc, Decl *D) { + if (S.LangOpts.SYCLIsDevice && ShouldEmitRootNode) { + if (auto *VD = dyn_cast(D)) { + if (!S.checkAllowedSYCLInitializer(VD)) { + S.Diag(Loc, diag::err_sycl_restrict) + << Sema::KernelConstStaticVariable; + return; + } + } + } if (isa(D)) return; if (auto *FD = dyn_cast(D)) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index d7dc8e005720e..8dad1279523f8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -18717,16 +18717,14 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD, bool Final) { assert(FD && "Expected non-null FunctionDecl"); - // SYCL functions can be template, so we check if they have appropriate - // attribute prior to checking if it is a template. - if (LangOpts.SYCLIsDevice && - (FD->hasAttr() || FD->hasAttr())) - return FunctionEmissionStatus::Emitted; - // Templates are emitted when they're instantiated. if (FD->isDependentContext()) return FunctionEmissionStatus::TemplateDiscarded; + if (LangOpts.SYCLIsDevice && + (FD->hasAttr() || FD->hasAttr())) + return FunctionEmissionStatus::Emitted; + // Check whether this function is an externally visible definition. auto IsEmittedForExternalSymbol = [this, FD]() { // We have to check the GVA linkage of the function's *definition* -- if we diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index dd56ddab15c48..513c4a1feb3df 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -246,12 +246,6 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_esimd_global_in_sycl_context, Sema::DeviceDiagnosticReason::Sycl); - // Disallow const statics and globals that are not zero-initialized - // or constant-initialized. - else if (IsRuntimeEvaluated && IsConst && VD->hasGlobalStorage() && - !VD->isConstexpr() && !checkAllowedSYCLInitializer(VD)) - SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict) - << Sema::KernelConstStaticVariable; } else if (auto *FDecl = dyn_cast(D)) { // SYCL device function cannot be called from an ESIMD context. However, // funcitons that start with '__spirv_' or '__sycl_' are exceptions to diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 6b153adcaf34b..a753d10c86075 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -4111,11 +4111,6 @@ void Sema::finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller, const FunctionDecl *Callee, SourceLocation Loc, DeviceDiagnosticReason Reason) { - // Somehow an unspecialized template appears to be in callgraph or list of - // device functions. We don't want to emit diagnostic here. - if (Callee->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate) - return; - Callee = Callee->getMostRecentDecl(); // If the reason for the emission of this diagnostic is not SYCL-specific, diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index e08a009480a2c..0dd6dcf40d329 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5678,10 +5678,6 @@ void Sema::InstantiateVariableInitializer( if (getLangOpts().CUDA) checkAllowedCUDAInitializer(Var); - - if (getLangOpts().SYCLIsDevice && !checkAllowedSYCLInitializer(Var)) - SYCLDiagIfDeviceCode(Var->getLocation(), diag::err_sycl_restrict) - << Sema::KernelConstStaticVariable; } /// Instantiate the definition of the given variable from its diff --git a/clang/test/SemaSYCL/inline-asm.cpp b/clang/test/SemaSYCL/inline-asm.cpp index cb5b06c5119cc..8ce1a3b0719b7 100644 --- a/clang/test/SemaSYCL/inline-asm.cpp +++ b/clang/test/SemaSYCL/inline-asm.cpp @@ -22,7 +22,7 @@ static __inline unsigned int asm_func_2(unsigned int __leaf, unsigned long __d[]) { unsigned int __result; #ifdef SPIR_CHECK - //expected-error@+2 2{{invalid output constraint '=a' in asm}} + //expected-error@+2 {{invalid output constraint '=a' in asm}} __asm__("enclu" : "=a"(__result), "=b"(__d[0]), "=c"(__d[1]), "=d"(__d[2]) : "a"(__leaf), "b"(__d[0]), "c"(__d[1]), "d"(__d[2]) @@ -58,7 +58,7 @@ __attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) { #ifdef SPIR_CHECK unsigned int i = 3; unsigned long d[4]; - //expected-note@+1 2{{called by 'kernel_single_task}} + //expected-note@+1 {{called by 'kernel_single_task}} asm_func_2(i, d); #endif // SPIR_CHECK #else diff --git a/clang/test/SemaSYCL/sycl-device-const-static.cpp b/clang/test/SemaSYCL/sycl-device-const-static.cpp index f4d9bc6b3f298..f410b9415b19b 100644 --- a/clang/test/SemaSYCL/sycl-device-const-static.cpp +++ b/clang/test/SemaSYCL/sycl-device-const-static.cpp @@ -22,6 +22,16 @@ template struct U; const S s5; +template +class randomType { + public: + randomType() {} + T val; +}; + +template +const randomType Var; + void usage() { // expected-error@+1{{SYCL kernel cannot use a non-const static data variable}} static int s1; @@ -34,11 +44,13 @@ void usage() { (void)s5; // expected-error@+1{{SYCL kernel cannot use a const static or global variable that is neither zero-initialized nor constant-initialized}} (void)s6; + + //expected-error@+1{{SYCL kernel cannot use a const static or global variable that is neither zero-initialized nor constant-initialized}} + (void)Var; } template __attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) { - // expected-error@+1{{SYCL kernel cannot use a non-const static data variable}} static int z; // expected-note-re@+3{{called by 'kernel_single_task}} // expected-note-re@+2{{called by 'kernel_single_task}} From 7c4590d029eb3f607695fd3866478692fa17bb9e Mon Sep 17 00:00:00 2001 From: Mariya Podchishchaeva Date: Tue, 21 Dec 2021 22:18:34 +0300 Subject: [PATCH 2/2] Fix formatting --- clang/test/SemaSYCL/inline-asm.cpp | 4 ++-- clang/test/SemaSYCL/sycl-device-const-static.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/test/SemaSYCL/inline-asm.cpp b/clang/test/SemaSYCL/inline-asm.cpp index 8ce1a3b0719b7..a4a308fe0875a 100644 --- a/clang/test/SemaSYCL/inline-asm.cpp +++ b/clang/test/SemaSYCL/inline-asm.cpp @@ -22,7 +22,7 @@ static __inline unsigned int asm_func_2(unsigned int __leaf, unsigned long __d[]) { unsigned int __result; #ifdef SPIR_CHECK - //expected-error@+2 {{invalid output constraint '=a' in asm}} + // expected-error@+2 {{invalid output constraint '=a' in asm}} __asm__("enclu" : "=a"(__result), "=b"(__d[0]), "=c"(__d[1]), "=d"(__d[2]) : "a"(__leaf), "b"(__d[0]), "c"(__d[1]), "d"(__d[2]) @@ -58,7 +58,7 @@ __attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) { #ifdef SPIR_CHECK unsigned int i = 3; unsigned long d[4]; - //expected-note@+1 {{called by 'kernel_single_task}} + // expected-note@+1 {{called by 'kernel_single_task}} asm_func_2(i, d); #endif // SPIR_CHECK #else diff --git a/clang/test/SemaSYCL/sycl-device-const-static.cpp b/clang/test/SemaSYCL/sycl-device-const-static.cpp index f410b9415b19b..7cf081a193c2a 100644 --- a/clang/test/SemaSYCL/sycl-device-const-static.cpp +++ b/clang/test/SemaSYCL/sycl-device-const-static.cpp @@ -24,7 +24,7 @@ const S s5; template class randomType { - public: +public: randomType() {} T val; }; @@ -45,7 +45,7 @@ void usage() { // expected-error@+1{{SYCL kernel cannot use a const static or global variable that is neither zero-initialized nor constant-initialized}} (void)s6; - //expected-error@+1{{SYCL kernel cannot use a const static or global variable that is neither zero-initialized nor constant-initialized}} + // expected-error@+1{{SYCL kernel cannot use a const static or global variable that is neither zero-initialized nor constant-initialized}} (void)Var; }