From b3d05cd697232a86ff7babc57f714c4c6225dfde Mon Sep 17 00:00:00 2001 From: gejin Date: Tue, 18 May 2021 15:46:50 +0800 Subject: [PATCH 1/9] Allow __failed_assertion to support libstdc++-11 Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 21 +++++++++++++++++++-- sycl/include/CL/sycl/builtins.hpp | 1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index d1876a67bea6c..7d7295e775366 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -61,6 +61,9 @@ static constexpr llvm::StringLiteral InitESIMDMethodName = "__init_esimd"; static constexpr llvm::StringLiteral InitSpecConstantsBuffer = "__init_specialization_constants_buffer"; static constexpr llvm::StringLiteral FinalizeMethodName = "__finalize"; +static constexpr llvm::StringLiteral GlibcxxFailedAssertion = + "__failed_assertion"; +static constexpr llvm::StringLiteral GlibcxxConfigFile = "bits/c++config.h"; constexpr unsigned MaxKernelArgsSize = 2048; namespace { @@ -320,6 +323,19 @@ static bool isSYCLKernelBodyFunction(FunctionDecl *FD) { return FD->getOverloadedOperator() == OO_Call; } +static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, + const SourceManager &SrcMgr) { + if (!Callee) + return false; + if (Callee->getName() == GlibcxxFailedAssertion) { + if (Callee->getLocation().printToString(SrcMgr).rfind( + GlibcxxConfigFile.str()) != std::string::npos) + return true; + } + + return false; +} + // Helper function to report conflicting function attributes. // F - the function, A1 - function attribute, A2 - the attribute it conflicts // with. @@ -4121,8 +4137,9 @@ void Sema::finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller, if (Callee->hasAttr() || Callee->hasAttr()) return; - // Diagnose if this is an undefined function and it is not a builtin. - if (!Callee->isDefined() && !Callee->getBuiltinID()) { + // Diagnose if this is an undefined function and it is not a builtin. + if (!Callee->isDefined() && !Callee->getBuiltinID() && + !isSYCLUndefinedAllowed(Callee, getSourceManager())) { Diag(Loc, diag::err_sycl_restrict) << Sema::KernelCallUndefinedFunction; Diag(Callee->getLocation(), diag::note_previous_decl) << Callee; Diag(Caller->getLocation(), diag::note_called_by) << Caller; diff --git a/sycl/include/CL/sycl/builtins.hpp b/sycl/include/CL/sycl/builtins.hpp index 7780341c9af03..ff3fd3f71dcc4 100644 --- a/sycl/include/CL/sycl/builtins.hpp +++ b/sycl/include/CL/sycl/builtins.hpp @@ -1632,6 +1632,7 @@ extern SYCL_EXTERNAL double ldexp(double x, int exp); extern SYCL_EXTERNAL double hypot(double x, double y); } #ifdef __GLIBC__ +extern SYCL_EXTERNAL void __failed_assertion(); extern "C" { extern SYCL_EXTERNAL void __assert_fail(const char *expr, const char *file, unsigned int line, const char *func); From 86c9ddb575bec5812ff790740a241e60de50a247 Mon Sep 17 00:00:00 2001 From: gejin Date: Tue, 18 May 2021 16:38:02 +0800 Subject: [PATCH 2/9] fix clang-format Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 7d7295e775366..47fc777d22429 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -4137,7 +4137,7 @@ void Sema::finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller, if (Callee->hasAttr() || Callee->hasAttr()) return; - // Diagnose if this is an undefined function and it is not a builtin. + // Diagnose if this is an undefined function and it is not a builtin. if (!Callee->isDefined() && !Callee->getBuiltinID() && !isSYCLUndefinedAllowed(Callee, getSourceManager())) { Diag(Loc, diag::err_sycl_restrict) << Sema::KernelCallUndefinedFunction; From 4d73ea355df658a9ec781ccb32f54112a2a5d5be Mon Sep 17 00:00:00 2001 From: gejin Date: Thu, 20 May 2021 12:04:28 +0800 Subject: [PATCH 3/9] Use more strict check for __failed_assertion Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 12 +++++++----- sycl/include/CL/sycl/builtins.hpp | 1 - 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 47fc777d22429..b100fff37c197 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -327,11 +327,13 @@ static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, const SourceManager &SrcMgr) { if (!Callee) return false; - if (Callee->getName() == GlibcxxFailedAssertion) { - if (Callee->getLocation().printToString(SrcMgr).rfind( - GlibcxxConfigFile.str()) != std::string::npos) - return true; - } + + const Type *Ty = nullptr; + if (Callee->getName() == GlibcxxFailedAssertion && + (Callee->getNumParams() == 0) && + (Ty = Callee->getReturnType().getTypePtr()) && (Ty->isVoidType())) + return SrcMgr.getFilename(SrcMgr.getSpellingLoc(Callee->getLocation())) + .rfind(GlibcxxConfigFile) != StringRef::npos; return false; } diff --git a/sycl/include/CL/sycl/builtins.hpp b/sycl/include/CL/sycl/builtins.hpp index ff3fd3f71dcc4..7780341c9af03 100644 --- a/sycl/include/CL/sycl/builtins.hpp +++ b/sycl/include/CL/sycl/builtins.hpp @@ -1632,7 +1632,6 @@ extern SYCL_EXTERNAL double ldexp(double x, int exp); extern SYCL_EXTERNAL double hypot(double x, double y); } #ifdef __GLIBC__ -extern SYCL_EXTERNAL void __failed_assertion(); extern "C" { extern SYCL_EXTERNAL void __assert_fail(const char *expr, const char *file, unsigned int line, const char *func); From 061ed21b3872277e18a9ce166f7c77907e0fb4e6 Mon Sep 17 00:00:00 2001 From: gejin Date: Fri, 21 May 2021 14:42:59 +0800 Subject: [PATCH 4/9] check system header instead of c++config.h Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index b100fff37c197..8d7b0d5592a3f 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -63,7 +63,6 @@ static constexpr llvm::StringLiteral InitSpecConstantsBuffer = static constexpr llvm::StringLiteral FinalizeMethodName = "__finalize"; static constexpr llvm::StringLiteral GlibcxxFailedAssertion = "__failed_assertion"; -static constexpr llvm::StringLiteral GlibcxxConfigFile = "bits/c++config.h"; constexpr unsigned MaxKernelArgsSize = 2048; namespace { @@ -329,11 +328,15 @@ static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, return false; const Type *Ty = nullptr; + // libstdc++-11 introduced undefined function "void __failed_assertion()" + // which may lead to SemaSYCL check failure. However, this undefined function + // is used to trigger some compilation error when check fails in compilation + // time and will be ignored when the check succeeds. We enable this function + // to support some important std functions in SYCL device. if (Callee->getName() == GlibcxxFailedAssertion && (Callee->getNumParams() == 0) && (Ty = Callee->getReturnType().getTypePtr()) && (Ty->isVoidType())) - return SrcMgr.getFilename(SrcMgr.getSpellingLoc(Callee->getLocation())) - .rfind(GlibcxxConfigFile) != StringRef::npos; + return SrcMgr.isInSystemHeader(Callee->getLocation()); return false; } From 92ca818c1f1bd9d6169a733d6776949b83162281 Mon Sep 17 00:00:00 2001 From: gejin Date: Wed, 26 May 2021 10:10:11 +0800 Subject: [PATCH 5/9] Some minor updates Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 8d7b0d5592a3f..c3492f7788731 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -327,18 +327,15 @@ static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, if (!Callee) return false; - const Type *Ty = nullptr; - // libstdc++-11 introduced undefined function "void __failed_assertion()" + // libstdc++-11 introduced an undefined function "void __failed_assertion()" // which may lead to SemaSYCL check failure. However, this undefined function - // is used to trigger some compilation error when check fails in compilation - // time and will be ignored when the check succeeds. We enable this function - // to support some important std functions in SYCL device. - if (Callee->getName() == GlibcxxFailedAssertion && - (Callee->getNumParams() == 0) && - (Ty = Callee->getReturnType().getTypePtr()) && (Ty->isVoidType())) - return SrcMgr.isInSystemHeader(Callee->getLocation()); - - return false; + // is used to trigger some compilation error when the check fails at compile + // time and will be ignored when the check succeeds. We allow calls to this + // function to support some important std functions in SYCL device. + return (Callee->getName() == GlibcxxFailedAssertion) && + (Callee->getNumParams() == 0) && + Callee->getReturnType()->isVoidType() && + SrcMgr.isInSystemHeader(Callee->getLocation()); } // Helper function to report conflicting function attributes. @@ -4143,6 +4140,8 @@ void Sema::finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller, return; // Diagnose if this is an undefined function and it is not a builtin. + // Currently, there is an exception of "__failed_assertion" in libstdc++-11, + // this undefined function is used to trigger a compiling error. if (!Callee->isDefined() && !Callee->getBuiltinID() && !isSYCLUndefinedAllowed(Callee, getSourceManager())) { Diag(Loc, diag::err_sycl_restrict) << Sema::KernelCallUndefinedFunction; From 43b12ed0732989389948e7fa086ab92b216d093d Mon Sep 17 00:00:00 2001 From: gejin Date: Fri, 28 May 2021 11:04:52 +0800 Subject: [PATCH 6/9] Minor changes Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index c3492f7788731..ac8877d71db76 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -61,7 +61,7 @@ static constexpr llvm::StringLiteral InitESIMDMethodName = "__init_esimd"; static constexpr llvm::StringLiteral InitSpecConstantsBuffer = "__init_specialization_constants_buffer"; static constexpr llvm::StringLiteral FinalizeMethodName = "__finalize"; -static constexpr llvm::StringLiteral GlibcxxFailedAssertion = +static constexpr llvm::StringLiteral LibstdcxxFailedAssertion = "__failed_assertion"; constexpr unsigned MaxKernelArgsSize = 2048; @@ -332,8 +332,8 @@ static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, // is used to trigger some compilation error when the check fails at compile // time and will be ignored when the check succeeds. We allow calls to this // function to support some important std functions in SYCL device. - return (Callee->getName() == GlibcxxFailedAssertion) && - (Callee->getNumParams() == 0) && + return (Callee->getName() == LibstdcxxFailedAssertion) && + Callee->getNumParams() == 0 && Callee->getReturnType()->isVoidType() && SrcMgr.isInSystemHeader(Callee->getLocation()); } From f5dd626b2d5560828b0a6c8802b6c5f5aa5db335 Mon Sep 17 00:00:00 2001 From: gejin Date: Fri, 28 May 2021 11:13:00 +0800 Subject: [PATCH 7/9] Fix clang-format Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index ac8877d71db76..6a37c3404e7a2 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -333,8 +333,7 @@ static bool isSYCLUndefinedAllowed(const FunctionDecl *Callee, // time and will be ignored when the check succeeds. We allow calls to this // function to support some important std functions in SYCL device. return (Callee->getName() == LibstdcxxFailedAssertion) && - Callee->getNumParams() == 0 && - Callee->getReturnType()->isVoidType() && + Callee->getNumParams() == 0 && Callee->getReturnType()->isVoidType() && SrcMgr.isInSystemHeader(Callee->getLocation()); } From 37f4766ffdaa7690bdcc6ce5e30a161ef946ec52 Mon Sep 17 00:00:00 2001 From: gejin Date: Fri, 28 May 2021 14:14:35 +0800 Subject: [PATCH 8/9] Add lit test Signed-off-by: gejin --- clang/test/SemaSYCL/Inputs/dummy_failed_assert | 1 + clang/test/SemaSYCL/__failed_assertion_fail.cpp | 17 +++++++++++++++++ clang/test/SemaSYCL/__failed_assertion_pass.cpp | 14 ++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 clang/test/SemaSYCL/Inputs/dummy_failed_assert create mode 100644 clang/test/SemaSYCL/__failed_assertion_fail.cpp create mode 100644 clang/test/SemaSYCL/__failed_assertion_pass.cpp diff --git a/clang/test/SemaSYCL/Inputs/dummy_failed_assert b/clang/test/SemaSYCL/Inputs/dummy_failed_assert new file mode 100644 index 0000000000000..e1561f0f48dbe --- /dev/null +++ b/clang/test/SemaSYCL/Inputs/dummy_failed_assert @@ -0,0 +1 @@ +void __failed_assertion(); diff --git a/clang/test/SemaSYCL/__failed_assertion_fail.cpp b/clang/test/SemaSYCL/__failed_assertion_fail.cpp new file mode 100644 index 0000000000000..ef31801624a67 --- /dev/null +++ b/clang/test/SemaSYCL/__failed_assertion_fail.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -verify -fsyntax-only %s +// UNSUPPORTED: system-windows +// This test checks that an undefined "__failed_assertion" without SYCL_EXTERNAL which is not declared +// in a system header will lead to SYCL sema check failure. + +#include "sycl.hpp" + +void __failed_assertion(); +// expected-note@-1 {{'__failed_assertion' declared here}} + +SYCL_EXTERNAL +void call_failed_assertion() { + // expected-note@-1 {{called by 'call_failed_assertion'}} + __failed_assertion(); + // expected-error@-1 {{SYCL kernel cannot call an undefined function without SYCL_EXTERNAL attribute}} +} + diff --git a/clang/test/SemaSYCL/__failed_assertion_pass.cpp b/clang/test/SemaSYCL/__failed_assertion_pass.cpp new file mode 100644 index 0000000000000..f68ad41f14670 --- /dev/null +++ b/clang/test/SemaSYCL/__failed_assertion_pass.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -verify -fsyntax-only %s +// UNSUPPORTED: system-windows +// expected-no-diagnostics +// This test checks that an undefined "__failed_assertion" without SYCL_EXTERNAL which is declared in +// a system header won't lead to SYCL sema check failure. + +#include "sycl.hpp" +#include + +SYCL_EXTERNAL +void call_failed_assertion() { + __failed_assertion(); +} + From a8fccf0d93e9be57eb67c0de326dbe0c7d46a491 Mon Sep 17 00:00:00 2001 From: gejin Date: Wed, 2 Jun 2021 11:39:25 +0800 Subject: [PATCH 9/9] put __failed_assertion lit tests into a single one Signed-off-by: gejin --- ...sertion_fail.cpp => __failed_assertion.cpp} | 18 +++++++++++++++--- .../test/SemaSYCL/__failed_assertion_pass.cpp | 14 -------------- 2 files changed, 15 insertions(+), 17 deletions(-) rename clang/test/SemaSYCL/{__failed_assertion_fail.cpp => __failed_assertion.cpp} (55%) delete mode 100644 clang/test/SemaSYCL/__failed_assertion_pass.cpp diff --git a/clang/test/SemaSYCL/__failed_assertion_fail.cpp b/clang/test/SemaSYCL/__failed_assertion.cpp similarity index 55% rename from clang/test/SemaSYCL/__failed_assertion_fail.cpp rename to clang/test/SemaSYCL/__failed_assertion.cpp index ef31801624a67..c8985158cc02b 100644 --- a/clang/test/SemaSYCL/__failed_assertion_fail.cpp +++ b/clang/test/SemaSYCL/__failed_assertion.cpp @@ -1,17 +1,29 @@ +// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -verify -DUSR -fsyntax-only %s // RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -verify -fsyntax-only %s // UNSUPPORTED: system-windows -// This test checks that an undefined "__failed_assertion" without SYCL_EXTERNAL which is not declared -// in a system header will lead to SYCL sema check failure. +// This test checks that an undefined "__failed_assertion" without SYCL_EXTERNAL will lead to SYCL sema check +// failure if it is not declared in a system header otherwise no SYCL sema check failure will be triggered. #include "sycl.hpp" - +#ifdef USR void __failed_assertion(); // expected-note@-1 {{'__failed_assertion' declared here}} +#else +#include +#endif +#ifdef USR SYCL_EXTERNAL void call_failed_assertion() { // expected-note@-1 {{called by 'call_failed_assertion'}} __failed_assertion(); // expected-error@-1 {{SYCL kernel cannot call an undefined function without SYCL_EXTERNAL attribute}} } +#else +// expected-no-diagnostics +SYCL_EXTERNAL +void call_failed_assertion() { + __failed_assertion(); +} +#endif diff --git a/clang/test/SemaSYCL/__failed_assertion_pass.cpp b/clang/test/SemaSYCL/__failed_assertion_pass.cpp deleted file mode 100644 index f68ad41f14670..0000000000000 --- a/clang/test/SemaSYCL/__failed_assertion_pass.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -verify -fsyntax-only %s -// UNSUPPORTED: system-windows -// expected-no-diagnostics -// This test checks that an undefined "__failed_assertion" without SYCL_EXTERNAL which is declared in -// a system header won't lead to SYCL sema check failure. - -#include "sycl.hpp" -#include - -SYCL_EXTERNAL -void call_failed_assertion() { - __failed_assertion(); -} -