From f3e005de5aaf355264ce946db2ccd864fc43c2ed Mon Sep 17 00:00:00 2001 From: gejin Date: Mon, 22 Mar 2021 16:00:36 +0800 Subject: [PATCH 1/4] Report Compiling error for recursion in SYCL kernel Signed-off-by: gejin --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 --- clang/lib/Sema/SemaSYCL.cpp | 3 ++- clang/test/SemaSYCL/restrict-recursion3.cpp | 2 +- clang/test/SemaSYCL/restrict-recursion4.cpp | 4 ++-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index bfafb3da5c30d..d764e0547d016 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11243,9 +11243,6 @@ def warn_sycl_implicit_decl "declaration for a kernel type name; your program may not " "be portable">, InGroup, ShowInSystemHeader, DefaultIgnore; -def warn_sycl_restrict_recursion - : Warning<"SYCL kernel cannot call a recursive function">, - InGroup, DefaultError; def err_ivdep_duplicate_arg : Error< "duplicate argument to 'ivdep'; attribute requires one or both of a safelen " "and array">; diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 2939bf4316a06..5c3e2c8afd2cd 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -370,7 +370,8 @@ class MarkDeviceFunction : public RecursiveASTVisitor { // all functions used by kernel have already been parsed and have // definitions. if (RecursiveSet.count(Callee) && !ConstexprDepth) { - SemaRef.Diag(e->getExprLoc(), diag::warn_sycl_restrict_recursion); + SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict) + << Sema::KernelCallRecursiveFunction; SemaRef.Diag(Callee->getSourceRange().getBegin(), diag::note_sycl_recursive_function_declared_here) << Sema::KernelCallRecursiveFunction; diff --git a/clang/test/SemaSYCL/restrict-recursion3.cpp b/clang/test/SemaSYCL/restrict-recursion3.cpp index 9aebe88df63f2..b66e3cd580cc3 100644 --- a/clang/test/SemaSYCL/restrict-recursion3.cpp +++ b/clang/test/SemaSYCL/restrict-recursion3.cpp @@ -34,7 +34,7 @@ template __attribute__((sycl_kernel)) void kernel_single_task2(const Func &kernelFunc) { // expected-note@+1 {{called by 'kernel_single_task2}} kernelFunc(); - // expected-warning@+1 2{{SYCL kernel cannot call a recursive function}} + // expected-error@+1 2{{SYCL kernel cannot call a recursive function}} kernel_single_task2(kernelFunc); } diff --git a/clang/test/SemaSYCL/restrict-recursion4.cpp b/clang/test/SemaSYCL/restrict-recursion4.cpp index 1fa5ec4ea0f61..ee0fdb20ce4c0 100644 --- a/clang/test/SemaSYCL/restrict-recursion4.cpp +++ b/clang/test/SemaSYCL/restrict-recursion4.cpp @@ -10,7 +10,7 @@ int fib(int n) { // expected-note@+1 2{{function implemented using recursion declared here}} void kernel2(void) { - // expected-warning@+1 {{SYCL kernel cannot call a recursive function}} + // expected-error@+1 {{SYCL kernel cannot call a recursive function}} kernel2(); } @@ -24,7 +24,7 @@ void *operator new(size_t); void usage2(myFuncDef functionPtr) { // expected-error@+1 {{SYCL kernel cannot allocate storage}} int *ip = new int; - // expected-warning@+1 {{SYCL kernel cannot call a recursive function}} + // expected-error@+1 {{SYCL kernel cannot call a recursive function}} kernel2(); } From 1d53420ff8eefb0092f4d6dfd28dc555c70faea0 Mon Sep 17 00:00:00 2001 From: gejin Date: Wed, 31 Mar 2021 15:27:52 +0800 Subject: [PATCH 2/4] Support Recursion in ESIMD kernel Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 120 +++++++++++++++++++++++++++--------- 1 file changed, 90 insertions(+), 30 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 5c3e2c8afd2cd..cd3719b92677a 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -31,6 +31,7 @@ #include #include #include +// #include using namespace clang; using namespace std::placeholders; @@ -105,6 +106,8 @@ class Util { /// \param Name the function name to be checked against. static bool isSyclFunction(const FunctionDecl *FD, StringRef Name); + // static void printSyclFunction(const FunctionDecl *FD); + /// Checks whether given clang type is a full specialization of the SYCL /// specialization constant class. static bool isSyclSpecConstantType(const QualType &Ty); @@ -365,11 +368,12 @@ class MarkDeviceFunction : public RecursiveASTVisitor { Callee = Callee->getCanonicalDecl(); assert(Callee && "Device function canonical decl must be available"); + bool RecursionAllowed = TraverseEsimdKernel && Callee->hasAttr(); // Remember that all SYCL kernel functions have deferred // instantiation as template functions. It means that // all functions used by kernel have already been parsed and have // definitions. - if (RecursiveSet.count(Callee) && !ConstexprDepth) { + if (RecursiveSet.count(Callee) && !ConstexprDepth && !RecursionAllowed) { SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict) << Sema::KernelCallRecursiveFunction; SemaRef.Diag(Callee->getSourceRange().getBegin(), @@ -466,41 +470,61 @@ class MarkDeviceFunction : public RecursiveASTVisitor { // The call graph for this translation unit. CallGraph SYCLCG; // The set of functions called by a kernel function. - llvm::SmallPtrSet KernelSet; + // llvm::SmallPtrSet KernelSet; + llvm::DenseMap> SYCLKernelInvokeMap; // The set of recursive functions identified while building the // kernel set, this is used for error diagnostics. llvm::SmallPtrSet RecursiveSet; - // Determines whether the function FD is recursive. - // CalleeNode is a function which is called either directly - // or indirectly from FD. If recursion is detected then create - // diagnostic notes on each function as the callstack is unwound. - void CollectKernelSet(FunctionDecl *CalleeNode, FunctionDecl *FD, - llvm::SmallPtrSet VisitedSet) { - // We're currently checking CalleeNode on a different - // trace through the CallGraph, we avoid infinite recursion - // by using KernelSet to keep track of this. - if (!KernelSet.insert(CalleeNode).second) - // Previously seen, stop recursion. - return; - if (CallGraphNode *N = SYCLCG.getNode(CalleeNode)) { - for (const CallGraphNode *CI : *N) { - if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { - Callee = Callee->getCanonicalDecl(); - if (VisitedSet.count(Callee)) { - // There's a stack frame to visit this Callee above - // this invocation. Do not recurse here. + + void WalkSYCLKernelCGNode(FunctionDecl *KernelNode, llvm::SmallPtrSet &VisitedSet) { + CallGraphNode *CGN = SYCLCG.getNode(KernelNode); + if (!CGN) return; + for (const CallGraphNode *CI : *CGN) { + if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + Callee = Callee->getCanonicalDecl(); + if (VisitedSet.count(Callee) == 0) { + // Util::printSyclFunction(Callee); + VisitedSet.insert(Callee); + if (IsCylicSYCLFunction(Callee)) { RecursiveSet.insert(Callee); - RecursiveSet.insert(CalleeNode); - } else { - VisitedSet.insert(Callee); - CollectKernelSet(Callee, FD, VisitedSet); - VisitedSet.erase(Callee); } + WalkSYCLKernelCGNode(Callee, VisitedSet); + } + } + } + } + + bool IsCylicSYCLFunction(FunctionDecl *SYCLFunc) { + CallGraphNode *CGN = SYCLCG.getNode(SYCLFunc); + if (!CGN) return false; + llvm::SmallPtrSet VisitedSet; + for (const CallGraphNode *CI : *CGN) { + if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + Callee = Callee->getCanonicalDecl(); + if (SYCLFunctionVisit(Callee, SYCLFunc, VisitedSet)) return true; + } + } + return false; + } + + bool SYCLFunctionVisit(FunctionDecl *FDSrc, FunctionDecl *FDDes, llvm::SmallPtrSet &VisitedSet) { + if (FDSrc == FDDes) return true; + CallGraphNode *CGN = SYCLCG.getNode(FDSrc); + if (!CGN) return false; + for (const CallGraphNode *CI : *CGN) { + if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + Callee = Callee->getCanonicalDecl(); + if (VisitedSet.count(Callee) == 0) { + VisitedSet.insert(Callee); + if (SYCLFunctionVisit(Callee, FDDes, VisitedSet)) return true; } } } + return false; } + void SetTraverseEsimdKernel(bool TEsimdK) { TraverseEsimdKernel = TEsimdK; } + bool IsTraverseEsimdKernel() const { return TraverseEsimdKernel; } // Traverses over CallGraph to collect list of attributes applied to // functions called by SYCLKernel (either directly and indirectly) which needs // to be propagated down to callers and applied to SYCL kernels. @@ -579,6 +603,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor { private: Sema &SemaRef; + bool TraverseEsimdKernel; }; class KernelBodyTransform : public TreeTransform { @@ -3278,8 +3303,13 @@ void Sema::MarkDevice(void) { for (Decl *D : syclDeviceDecls()) { if (auto SYCLKernel = dyn_cast(D)) { + // Util::printSyclFunction(SYCLKernel); llvm::SmallPtrSet VisitedSet; - Marker.CollectKernelSet(SYCLKernel, SYCLKernel, VisitedSet); + // Marker.CollectKernelSet(SYCLKernel, SYCLKernel, VisitedSet); + llvm::SmallPtrSet SYCLKernelInvokeSet; + llvm::SmallPtrSet RecurSet; + Marker.WalkSYCLKernelCGNode(SYCLKernel, SYCLKernelInvokeSet); + Marker.SYCLKernelInvokeMap.insert(std::make_pair(SYCLKernel, SYCLKernelInvokeSet)); // Let's propagate attributes from device functions to a SYCL kernels llvm::SmallVector Attrs; @@ -3395,9 +3425,24 @@ void Sema::MarkDevice(void) { } } } - for (const auto &elt : Marker.KernelSet) { - if (FunctionDecl *Def = elt->getDefinition()) - Marker.TraverseStmt(Def->getBody()); + + llvm::SmallPtrSet NoEsimdModeVisited; + llvm::SmallPtrSet EsimdModeVisited; + for (Decl *D : syclDeviceDecls()) { + if (auto SYCLKernelDec = dyn_cast(D)) { + if (FunctionDecl *SYCLKernelDef = SYCLKernelDec->getDefinition()) { + Marker.SetTraverseEsimdKernel(SYCLKernelDef->hasAttr()); + auto &VisitedSYCLFunctionSet = (SYCLKernelDef->hasAttr() ? EsimdModeVisited : NoEsimdModeVisited); + Marker.TraverseStmt(SYCLKernelDef->getBody()); + for (FunctionDecl * SYCLFunctionDec : Marker.SYCLKernelInvokeMap[SYCLKernelDec]) { + if (VisitedSYCLFunctionSet.count(SYCLFunctionDec) != 0) continue; + if (FunctionDecl *SYCLFunctionDef = SYCLFunctionDec->getDefinition()) { + Marker.TraverseStmt(SYCLFunctionDef->getBody()); + VisitedSYCLFunctionSet.insert(SYCLFunctionDec); + } + } + } + } } } @@ -4129,6 +4174,21 @@ bool Util::isSyclFunction(const FunctionDecl *FD, StringRef Name) { return matchContext(DC, Scopes); } +/* void Util::printSyclFunction(const FunctionDecl *FD) { + if (FD != nullptr && FD->isFunctionOrMethod() && FD->getIdentifier() && + !FD->getName().empty()) { + std::cout << "sycl function: " << FD->getName().str() << std::endl; + return; + } + + if (FD == nullptr) std::cout << "null FD" << std::endl; + else if (!FD->isFunctionOrMethod()) std::cout << "FD is not function or method" << std::endl; + else if (!FD->getIdentifier()) std::cout << "FD has no identifier" << std::endl; + else if (FD->getName().empty()) std::cout << "FD empty name" << std::endl; + else; + return; +}*/ + bool Util::isAccessorPropertyListType(const QualType &Ty) { const StringRef &Name = "accessor_property_list"; std::array Scopes = { From 68c5111f86964834c3ae57f3a4bb830e6305bca0 Mon Sep 17 00:00:00 2001 From: gejin Date: Thu, 1 Apr 2021 14:05:45 +0800 Subject: [PATCH 3/4] clang format Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 149 ++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 59 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index cd3719b92677a..baecac1335d02 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -31,7 +31,6 @@ #include #include #include -// #include using namespace clang; using namespace std::placeholders; @@ -106,8 +105,6 @@ class Util { /// \param Name the function name to be checked against. static bool isSyclFunction(const FunctionDecl *FD, StringRef Name); - // static void printSyclFunction(const FunctionDecl *FD); - /// Checks whether given clang type is a full specialization of the SYCL /// specialization constant class. static bool isSyclSpecConstantType(const QualType &Ty); @@ -368,7 +365,8 @@ class MarkDeviceFunction : public RecursiveASTVisitor { Callee = Callee->getCanonicalDecl(); assert(Callee && "Device function canonical decl must be available"); - bool RecursionAllowed = TraverseEsimdKernel && Callee->hasAttr(); + bool RecursionAllowed = + TraverseEsimdKernel && Callee->hasAttr(); // Remember that all SYCL kernel functions have deferred // instantiation as template functions. It means that // all functions used by kernel have already been parsed and have @@ -469,60 +467,83 @@ class MarkDeviceFunction : public RecursiveASTVisitor { // The call graph for this translation unit. CallGraph SYCLCG; - // The set of functions called by a kernel function. - // llvm::SmallPtrSet KernelSet; - llvm::DenseMap> SYCLKernelInvokeMap; - // The set of recursive functions identified while building the - // kernel set, this is used for error diagnostics. + // Record the mapping between each SYCL kernel or SYCL_EXTERNAL function and + // functions called by it. + llvm::DenseMap> + SYCLKernelInvokeMap; + // The set of recursive functions identified while going through the call + // graph for each SYCL kernel or SYCL_EXTERNAL function, this is used for + // error diagnostics. llvm::SmallPtrSet RecursiveSet; - void WalkSYCLKernelCGNode(FunctionDecl *KernelNode, llvm::SmallPtrSet &VisitedSet) { - CallGraphNode *CGN = SYCLCG.getNode(KernelNode); - if (!CGN) return; - for (const CallGraphNode *CI : *CGN) { - if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + // Traverse over call graph to find all functions directly or indirectly + // called by SYCLFunc, all functions called are recorded in VisitedSet. + // Each time a function is visited, check if it is a "cyclic" function + // which means the function will directly or indrectly call itself. + // All "cyclic" functions are recorded in RecursiveSet for later recursion + // error diagnostics. + void + WalkSYCLFunctionCG(FunctionDecl *SYCLFunc, + llvm::SmallPtrSet &VisitedSet, + llvm::SmallPtrSet CyclicCheckSet) { + CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(SYCLFunc); + if (!SYCLFuncCGN) + return; + for (const CallGraphNode *CGN : *SYCLFuncCGN) { + if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { Callee = Callee->getCanonicalDecl(); if (VisitedSet.count(Callee) == 0) { - // Util::printSyclFunction(Callee); VisitedSet.insert(Callee); - if (IsCylicSYCLFunction(Callee)) { - RecursiveSet.insert(Callee); + if (CyclicCheckSet.count(Callee) == 0) { + if (IsCyclicSYCLFunction(Callee)) { + RecursiveSet.insert(Callee); + } + CyclicCheckSet.insert(Callee); } - WalkSYCLKernelCGNode(Callee, VisitedSet); + WalkSYCLFunctionCG(Callee, VisitedSet, CyclicCheckSet); } } } } - bool IsCylicSYCLFunction(FunctionDecl *SYCLFunc) { - CallGraphNode *CGN = SYCLCG.getNode(SYCLFunc); - if (!CGN) return false; + // Traverses over call graph to find whether a SYCL function will directly + // or indirectly call itself. + bool IsCyclicSYCLFunction(FunctionDecl *SYCLFunc) { + CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(SYCLFunc); + if (!SYCLFuncCGN) + return false; llvm::SmallPtrSet VisitedSet; - for (const CallGraphNode *CI : *CGN) { - if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + for (const CallGraphNode *CGN : *SYCLFuncCGN) { + if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { Callee = Callee->getCanonicalDecl(); - if (SYCLFunctionVisit(Callee, SYCLFunc, VisitedSet)) return true; + if (IsSYCLFunctionInvoke(Callee, SYCLFunc, VisitedSet)) + return true; } } return false; } - bool SYCLFunctionVisit(FunctionDecl *FDSrc, FunctionDecl *FDDes, llvm::SmallPtrSet &VisitedSet) { - if (FDSrc == FDDes) return true; - CallGraphNode *CGN = SYCLCG.getNode(FDSrc); - if (!CGN) return false; - for (const CallGraphNode *CI : *CGN) { - if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + // Traverse over call graph to find whether FDSrc wil directly or indrectly + // call FDDes. + bool IsSYCLFunctionInvoke(FunctionDecl *FDSrc, FunctionDecl *FDDes, + llvm::SmallPtrSet &VisitedSet) { + if (FDSrc == FDDes) + return true; + CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(FDSrc); + if (!SYCLFuncCGN) + return false; + for (const CallGraphNode *CGN : *SYCLFuncCGN) { + if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { Callee = Callee->getCanonicalDecl(); if (VisitedSet.count(Callee) == 0) { VisitedSet.insert(Callee); - if (SYCLFunctionVisit(Callee, FDDes, VisitedSet)) return true; + if (IsSYCLFunctionInvoke(Callee, FDDes, VisitedSet)) + return true; } } } return false; } - void SetTraverseEsimdKernel(bool TEsimdK) { TraverseEsimdKernel = TEsimdK; } bool IsTraverseEsimdKernel() const { return TraverseEsimdKernel; } // Traverses over CallGraph to collect list of attributes applied to @@ -3301,15 +3322,15 @@ void Sema::MarkDevice(void) { } } + llvm::SmallPtrSet SYCLFunctionCyclicCheckSet; for (Decl *D : syclDeviceDecls()) { if (auto SYCLKernel = dyn_cast(D)) { - // Util::printSyclFunction(SYCLKernel); llvm::SmallPtrSet VisitedSet; - // Marker.CollectKernelSet(SYCLKernel, SYCLKernel, VisitedSet); llvm::SmallPtrSet SYCLKernelInvokeSet; - llvm::SmallPtrSet RecurSet; - Marker.WalkSYCLKernelCGNode(SYCLKernel, SYCLKernelInvokeSet); - Marker.SYCLKernelInvokeMap.insert(std::make_pair(SYCLKernel, SYCLKernelInvokeSet)); + Marker.WalkSYCLFunctionCG(SYCLKernel, SYCLKernelInvokeSet, + SYCLFunctionCyclicCheckSet); + Marker.SYCLKernelInvokeMap.insert( + std::make_pair(SYCLKernel, SYCLKernelInvokeSet)); // Let's propagate attributes from device functions to a SYCL kernels llvm::SmallVector Attrs; @@ -3426,19 +3447,44 @@ void Sema::MarkDevice(void) { } } - llvm::SmallPtrSet NoEsimdModeVisited; - llvm::SmallPtrSet EsimdModeVisited; + // Previously, we traverse all SYCL functions including kernel functions + // and functions called by kernel to do some sema check following same rules. + // But this mechanism can't meet our requirements now as different rules may + // be required for different type of SYCL kernel. For example, recursion is + // not allowed for normal SYCL kernel but recursive function with "noinline" + // attribute is permitted in SYCL ESIMD kernel. + // Now, we traverse each SYCL kernel together with all functions called by it + // directly or indirectly. Before the sema check, we will check kernel type + // to decide whether different rules are used. Currently, only normal SYCL + // kernel and SYCL ESIMD kernel are supported here. A internal flag in Marker + // is used to indicate whether current check is on SYCL ESIMD kernel and all + // functions it call directly or indirectly. + // The traverse will introduce unnecessary duplicate checks as a function can + // be called in different SYCL kernels. For example, if "kernel1" and + // "kernel2" both call function "foo" and these 2 kernels are same type, we + // firstly check "kernel1" with "foo" and when we check "kernel2", "foo" will + // be checked again but it is unnecessary as the rules are same with previous + // check. In order to avoid unnecessary checks, we use 2 set to record all + // checked functions in different mode, only the one not in set will be + // checked. + llvm::SmallPtrSet NonESIMDModeCheckSet; + llvm::SmallPtrSet ESIMDModeCheckSet; for (Decl *D : syclDeviceDecls()) { if (auto SYCLKernelDec = dyn_cast(D)) { if (FunctionDecl *SYCLKernelDef = SYCLKernelDec->getDefinition()) { Marker.SetTraverseEsimdKernel(SYCLKernelDef->hasAttr()); - auto &VisitedSYCLFunctionSet = (SYCLKernelDef->hasAttr() ? EsimdModeVisited : NoEsimdModeVisited); + auto &SYCLFunctionCheckSet = + (SYCLKernelDef->hasAttr() ? ESIMDModeCheckSet + : NonESIMDModeCheckSet); Marker.TraverseStmt(SYCLKernelDef->getBody()); - for (FunctionDecl * SYCLFunctionDec : Marker.SYCLKernelInvokeMap[SYCLKernelDec]) { - if (VisitedSYCLFunctionSet.count(SYCLFunctionDec) != 0) continue; - if (FunctionDecl *SYCLFunctionDef = SYCLFunctionDec->getDefinition()) { + for (FunctionDecl *SYCLFunctionDec : + Marker.SYCLKernelInvokeMap[SYCLKernelDec]) { + if (SYCLFunctionCheckSet.count(SYCLFunctionDec) != 0) + continue; + if (FunctionDecl *SYCLFunctionDef = + SYCLFunctionDec->getDefinition()) { Marker.TraverseStmt(SYCLFunctionDef->getBody()); - VisitedSYCLFunctionSet.insert(SYCLFunctionDec); + SYCLFunctionCheckSet.insert(SYCLFunctionDec); } } } @@ -4174,21 +4220,6 @@ bool Util::isSyclFunction(const FunctionDecl *FD, StringRef Name) { return matchContext(DC, Scopes); } -/* void Util::printSyclFunction(const FunctionDecl *FD) { - if (FD != nullptr && FD->isFunctionOrMethod() && FD->getIdentifier() && - !FD->getName().empty()) { - std::cout << "sycl function: " << FD->getName().str() << std::endl; - return; - } - - if (FD == nullptr) std::cout << "null FD" << std::endl; - else if (!FD->isFunctionOrMethod()) std::cout << "FD is not function or method" << std::endl; - else if (!FD->getIdentifier()) std::cout << "FD has no identifier" << std::endl; - else if (FD->getName().empty()) std::cout << "FD empty name" << std::endl; - else; - return; -}*/ - bool Util::isAccessorPropertyListType(const QualType &Ty) { const StringRef &Name = "accessor_property_list"; std::array Scopes = { From 4a3eb3799c81a848c1803abffaa53155fd0769c4 Mon Sep 17 00:00:00 2001 From: gejin Date: Fri, 2 Apr 2021 09:19:28 +0800 Subject: [PATCH 4/4] revert esimd kernel recursion support Signed-off-by: gejin --- clang/lib/Sema/SemaSYCL.cpp | 157 ++++++++---------------------------- 1 file changed, 33 insertions(+), 124 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index d6a138dab2a73..b2810b8cf3086 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -371,13 +371,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor { Callee = Callee->getCanonicalDecl(); assert(Callee && "Device function canonical decl must be available"); - bool RecursionAllowed = - TraverseEsimdKernel && Callee->hasAttr(); // Remember that all SYCL kernel functions have deferred // instantiation as template functions. It means that // all functions used by kernel have already been parsed and have // definitions. - if (RecursiveSet.count(Callee) && !ConstexprDepth && !RecursionAllowed) { + if (RecursiveSet.count(Callee) && !ConstexprDepth) { SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict) << Sema::KernelCallRecursiveFunction; SemaRef.Diag(Callee->getSourceRange().getBegin(), @@ -473,85 +471,42 @@ class MarkDeviceFunction : public RecursiveASTVisitor { // The call graph for this translation unit. CallGraph SYCLCG; - // Record the mapping between each SYCL kernel or SYCL_EXTERNAL function and - // functions called by it. - llvm::DenseMap> - SYCLKernelInvokeMap; - // The set of recursive functions identified while going through the call - // graph for each SYCL kernel or SYCL_EXTERNAL function, this is used for - // error diagnostics. + // The set of functions called by a kernel function. + llvm::SmallPtrSet KernelSet; + // The set of recursive functions identified while building the + // kernel set, this is used for error diagnostics. llvm::SmallPtrSet RecursiveSet; - - // Traverse over call graph to find all functions directly or indirectly - // called by SYCLFunc, all functions called are recorded in VisitedSet. - // Each time a function is visited, check if it is a "cyclic" function - // which means the function will directly or indrectly call itself. - // All "cyclic" functions are recorded in RecursiveSet for later recursion - // error diagnostics. - void - WalkSYCLFunctionCG(FunctionDecl *SYCLFunc, - llvm::SmallPtrSet &VisitedSet, - llvm::SmallPtrSet CyclicCheckSet) { - CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(SYCLFunc); - if (!SYCLFuncCGN) + // Determines whether the function FD is recursive. + // CalleeNode is a function which is called either directly + // or indirectly from FD. If recursion is detected then create + // diagnostic notes on each function as the callstack is unwound. + void CollectKernelSet(FunctionDecl *CalleeNode, FunctionDecl *FD, + llvm::SmallPtrSet VisitedSet) { + // We're currently checking CalleeNode on a different + // trace through the CallGraph, we avoid infinite recursion + // by using KernelSet to keep track of this. + if (!KernelSet.insert(CalleeNode).second) + // Previously seen, stop recursion. return; - for (const CallGraphNode *CGN : *SYCLFuncCGN) { - if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { - Callee = Callee->getCanonicalDecl(); - if (VisitedSet.count(Callee) == 0) { - VisitedSet.insert(Callee); - if (CyclicCheckSet.count(Callee) == 0) { - if (IsCyclicSYCLFunction(Callee)) { - RecursiveSet.insert(Callee); - } - CyclicCheckSet.insert(Callee); + if (CallGraphNode *N = SYCLCG.getNode(CalleeNode)) { + for (const CallGraphNode *CI : *N) { + if (FunctionDecl *Callee = dyn_cast(CI->getDecl())) { + Callee = Callee->getCanonicalDecl(); + if (VisitedSet.count(Callee)) { + // There's a stack frame to visit this Callee above + // this invocation. Do not recurse here. + RecursiveSet.insert(Callee); + RecursiveSet.insert(CalleeNode); + } else { + VisitedSet.insert(Callee); + CollectKernelSet(Callee, FD, VisitedSet); + VisitedSet.erase(Callee); } - WalkSYCLFunctionCG(Callee, VisitedSet, CyclicCheckSet); } } } } - // Traverses over call graph to find whether a SYCL function will directly - // or indirectly call itself. - bool IsCyclicSYCLFunction(FunctionDecl *SYCLFunc) { - CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(SYCLFunc); - if (!SYCLFuncCGN) - return false; - llvm::SmallPtrSet VisitedSet; - for (const CallGraphNode *CGN : *SYCLFuncCGN) { - if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { - Callee = Callee->getCanonicalDecl(); - if (IsSYCLFunctionInvoke(Callee, SYCLFunc, VisitedSet)) - return true; - } - } - return false; - } - - // Traverse over call graph to find whether FDSrc wil directly or indrectly - // call FDDes. - bool IsSYCLFunctionInvoke(FunctionDecl *FDSrc, FunctionDecl *FDDes, - llvm::SmallPtrSet &VisitedSet) { - if (FDSrc == FDDes) - return true; - CallGraphNode *SYCLFuncCGN = SYCLCG.getNode(FDSrc); - if (!SYCLFuncCGN) - return false; - for (const CallGraphNode *CGN : *SYCLFuncCGN) { - if (FunctionDecl *Callee = dyn_cast(CGN->getDecl())) { - Callee = Callee->getCanonicalDecl(); - if (VisitedSet.count(Callee) == 0) { - VisitedSet.insert(Callee); - if (IsSYCLFunctionInvoke(Callee, FDDes, VisitedSet)) - return true; - } - } - } - return false; - } - void SetTraverseEsimdKernel(bool TEsimdK) { TraverseEsimdKernel = TEsimdK; } - bool IsTraverseEsimdKernel() const { return TraverseEsimdKernel; } // Traverses over CallGraph to collect list of attributes applied to // functions called by SYCLKernel (either directly and indirectly) which needs // to be propagated down to callers and applied to SYCL kernels. @@ -655,7 +610,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor { private: Sema &SemaRef; - bool TraverseEsimdKernel; }; class KernelBodyTransform : public TreeTransform { @@ -3493,15 +3447,10 @@ void Sema::MarkDevice(void) { } } - llvm::SmallPtrSet SYCLFunctionCyclicCheckSet; for (Decl *D : syclDeviceDecls()) { if (auto SYCLKernel = dyn_cast(D)) { llvm::SmallPtrSet VisitedSet; - llvm::SmallPtrSet SYCLKernelInvokeSet; - Marker.WalkSYCLFunctionCG(SYCLKernel, SYCLKernelInvokeSet, - SYCLFunctionCyclicCheckSet); - Marker.SYCLKernelInvokeMap.insert( - std::make_pair(SYCLKernel, SYCLKernelInvokeSet)); + Marker.CollectKernelSet(SYCLKernel, SYCLKernel, VisitedSet); // Let's propagate attributes from device functions to a SYCL kernels llvm::SmallVector Attrs; @@ -3620,49 +3569,9 @@ void Sema::MarkDevice(void) { } } } - - // Previously, we traverse all SYCL functions including kernel functions - // and functions called by kernel to do some sema check following same rules. - // But this mechanism can't meet our requirements now as different rules may - // be required for different type of SYCL kernel. For example, recursion is - // not allowed for normal SYCL kernel but recursive function with "noinline" - // attribute is permitted in SYCL ESIMD kernel. - // Now, we traverse each SYCL kernel together with all functions called by it - // directly or indirectly. Before the sema check, we will check kernel type - // to decide whether different rules are used. Currently, only normal SYCL - // kernel and SYCL ESIMD kernel are supported here. A internal flag in Marker - // is used to indicate whether current check is on SYCL ESIMD kernel and all - // functions it call directly or indirectly. - // The traverse will introduce unnecessary duplicate checks as a function can - // be called in different SYCL kernels. For example, if "kernel1" and - // "kernel2" both call function "foo" and these 2 kernels are same type, we - // firstly check "kernel1" with "foo" and when we check "kernel2", "foo" will - // be checked again but it is unnecessary as the rules are same with previous - // check. In order to avoid unnecessary checks, we use 2 set to record all - // checked functions in different mode, only the one not in set will be - // checked. - llvm::SmallPtrSet NonESIMDModeCheckSet; - llvm::SmallPtrSet ESIMDModeCheckSet; - for (Decl *D : syclDeviceDecls()) { - if (auto SYCLKernelDec = dyn_cast(D)) { - if (FunctionDecl *SYCLKernelDef = SYCLKernelDec->getDefinition()) { - Marker.SetTraverseEsimdKernel(SYCLKernelDef->hasAttr()); - auto &SYCLFunctionCheckSet = - (SYCLKernelDef->hasAttr() ? ESIMDModeCheckSet - : NonESIMDModeCheckSet); - Marker.TraverseStmt(SYCLKernelDef->getBody()); - for (FunctionDecl *SYCLFunctionDec : - Marker.SYCLKernelInvokeMap[SYCLKernelDec]) { - if (SYCLFunctionCheckSet.count(SYCLFunctionDec) != 0) - continue; - if (FunctionDecl *SYCLFunctionDef = - SYCLFunctionDec->getDefinition()) { - Marker.TraverseStmt(SYCLFunctionDef->getBody()); - SYCLFunctionCheckSet.insert(SYCLFunctionDec); - } - } - } - } + for (const auto &elt : Marker.KernelSet) { + if (FunctionDecl *Def = elt->getDefinition()) + Marker.TraverseStmt(Def->getBody()); } }