[SYCL] Move kernel diagnostics to the cal and, enable template trail on errors (#2289)

This patch moves the diagnostics for the kernel to the point in which the
kernel is called, rather than when generating the kernel. This means that
the call location is available (so that the checkers can be taught how to print
better diagnostics in the future). The biggest impact of this, is that it
causes us to print the template instantiation 'tree' since these are diagnosing
right away.

Currently, this uses the call location to better diagnose the lambda check (see
the additional note there!), and prints a kernel-parameter-must-be-lambda-or-functor
error.

Author: Erich Keane

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10948,6 +10948,8 @@ def err_sycl_kernel_incorrectly_named : Error<
   "|needs to have a globally-visible name"
   "|name is invalid. Unscoped enum requires fixed underlying type"
   "}0">;
+def err_sycl_kernel_not_function_object
+    : Error<"kernel parameter must be a lambda or function object">;
 def err_sycl_restrict : Error<
   "SYCL kernel cannot "
 "%select{use a non-const global variable"

clang/include/clang/Sema/Sema.h

@@ -12306,6 +12306,9 @@ class Sema final {
                  bool IsMemberFunction, SourceLocation Loc, SourceRange Range,
                  VariadicCallType CallType);
 
+  void CheckSYCLKernelCall(FunctionDecl *CallerFunc, SourceRange CallLoc,
+                           ArrayRef<const Expr *> Args);
+
   bool CheckObjCString(Expr *Arg);
   ExprResult CheckOSLogFormatStringArg(Expr *Arg);
 
@@ -12727,7 +12730,7 @@ class Sema final {
   // Used to suppress diagnostics during kernel construction, since these were
   // already emitted earlier. Diagnosing during Kernel emissions also skips the
   // useful notes that shows where the kernel was called.
-  bool ConstructingOpenCLKernel = false;
+  bool DiagnosingSYCLKernel = false;
 
 public:
   void addSyclDeviceDecl(Decl *d) { SyclDeviceDecls.insert(d); }

clang/lib/Sema/SemaChecking.cpp

@@ -4558,6 +4558,9 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
   if (FD)
     diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc);
 
+  if (FD && FD->hasAttr<SYCLKernelAttr>())
+    CheckSYCLKernelCall(FD, Range, Args);
+
   // Diagnose variadic calls in SYCL.
   if (FD && FD ->isVariadic() && getLangOpts().SYCLIsDevice &&
       !isUnevaluatedContext() && !isKnownGoodSYCLDecl(FD))

clang/lib/Sema/SemaSYCL.cpp

@@ -706,7 +706,8 @@ getKernelInvocationKind(FunctionDecl *KernelCallerFunc) {
 }
 
 static const CXXRecordDecl *getKernelObjectType(FunctionDecl *Caller) {
-  return (*Caller->param_begin())->getType()->getAsCXXRecordDecl();
+  assert(Caller->getNumParams() > 0 && "Insufficient kernel parameters");
+  return Caller->getParamDecl(0)->getType()->getAsCXXRecordDecl();
 }
 
 /// Creates a kernel parameter descriptor
@@ -1205,7 +1206,6 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler {
 class SyclKernelDeclCreator : public SyclKernelFieldHandler {
   FunctionDecl *KernelDecl;
   llvm::SmallVector<ParmVarDecl *, 8> Params;
-  SyclKernelFieldChecker &ArgChecker;
   Sema::ContextRAII FuncContext;
   // Holds the last handled field's first parameter. This doesn't store an
   // iterator as push_back invalidates iterators.
@@ -1339,13 +1339,12 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
   }
 
 public:
-  SyclKernelDeclCreator(Sema &S, SyclKernelFieldChecker &ArgChecker,
-                        StringRef Name, SourceLocation Loc, bool IsInline,
-                        bool IsSIMDKernel)
+  SyclKernelDeclCreator(Sema &S, StringRef Name, SourceLocation Loc,
+                        bool IsInline, bool IsSIMDKernel)
       : SyclKernelFieldHandler(S),
         KernelDecl(createKernelDecl(S.getASTContext(), Name, Loc, IsInline,
                                     IsSIMDKernel)),
-        ArgChecker(ArgChecker), FuncContext(SemaRef, KernelDecl) {}
+        FuncContext(SemaRef, KernelDecl) {}
 
   ~SyclKernelDeclCreator() {
     ASTContext &Ctx = SemaRef.getASTContext();
@@ -1360,8 +1359,7 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
     KernelDecl->setType(FuncType);
     KernelDecl->setParams(Params);
 
-    if (ArgChecker.isValid())
-      SemaRef.addSyclDeviceDecl(KernelDecl);
+    SemaRef.addSyclDeviceDecl(KernelDecl);
   }
 
   bool handleSyclAccessorType(const CXXBaseSpecifier &BS,
@@ -2015,8 +2013,38 @@ class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler {
   using SyclKernelFieldHandler::handleSyclHalfType;
   using SyclKernelFieldHandler::handleSyclSamplerType;
 };
+
 } // namespace
 
+void Sema::CheckSYCLKernelCall(FunctionDecl *KernelFunc, SourceRange CallLoc,
+                               ArrayRef<const Expr *> Args) {
+  const CXXRecordDecl *KernelObj = getKernelObjectType(KernelFunc);
+  if (!KernelObj) {
+    Diag(Args[0]->getExprLoc(), diag::err_sycl_kernel_not_function_object);
+    KernelFunc->setInvalidDecl();
+    return;
+  }
+
+  if (KernelObj->isLambda()) {
+    for (const LambdaCapture &LC : KernelObj->captures())
+      if (LC.capturesThis() && LC.isImplicit()) {
+        Diag(LC.getLocation(), diag::err_implicit_this_capture);
+        Diag(CallLoc.getBegin(), diag::note_used_here);
+        KernelFunc->setInvalidDecl();
+      }
+  }
+
+  SyclKernelFieldChecker Checker(*this);
+
+  KernelObjVisitor Visitor{*this};
+  DiagnosingSYCLKernel = true;
+  Visitor.VisitRecordBases(KernelObj, Checker);
+  Visitor.VisitRecordFields(KernelObj, Checker);
+  DiagnosingSYCLKernel = false;
+  if (!Checker.isValid())
+    KernelFunc->setInvalidDecl();
+}
+
 // Generates the OpenCL kernel using KernelCallerFunc (kernel caller
 // function) defined is SYCL headers.
 // Generated OpenCL kernel contains the body of the kernel caller function,
@@ -2051,29 +2079,19 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc,
       constructKernelName(*this, KernelCallerFunc, MC);
   StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName
                                                        : CalculatedName);
-  if (KernelObj->isLambda()) {
-    for (const LambdaCapture &LC : KernelObj->captures())
-      if (LC.capturesThis() && LC.isImplicit())
-        Diag(LC.getLocation(), diag::err_implicit_this_capture);
-  }
-  SyclKernelFieldChecker checker(*this);
-  SyclKernelDeclCreator kernel_decl(
-      *this, checker, KernelName, KernelObj->getLocation(),
-      KernelCallerFunc->isInlined(), KernelCallerFunc->hasAttr<SYCLSimdAttr>());
+  SyclKernelDeclCreator kernel_decl(*this, KernelName, KernelObj->getLocation(),
+                                    KernelCallerFunc->isInlined(),
+                                    KernelCallerFunc->hasAttr<SYCLSimdAttr>());
   SyclKernelBodyCreator kernel_body(*this, kernel_decl, KernelObj,
                                     KernelCallerFunc);
   SyclKernelIntHeaderCreator int_header(
       *this, getSyclIntegrationHeader(), KernelObj,
       calculateKernelNameType(Context, KernelCallerFunc), KernelName,
       StableName);
 
-  ConstructingOpenCLKernel = true;
   KernelObjVisitor Visitor{*this};
-  Visitor.VisitRecordBases(KernelObj, checker, kernel_decl, kernel_body,
-                           int_header);
-  Visitor.VisitRecordFields(KernelObj, checker, kernel_decl, kernel_body,
-                            int_header);
-  ConstructingOpenCLKernel = false;
+  Visitor.VisitRecordBases(KernelObj, kernel_decl, kernel_body, int_header);
+  Visitor.VisitRecordFields(KernelObj, kernel_decl, kernel_body, int_header);
 }
 
 // This function marks all the callees of explicit SIMD kernel
@@ -2242,7 +2260,7 @@ Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
          "Should only be called during SYCL compilation");
   FunctionDecl *FD = dyn_cast<FunctionDecl>(getCurLexicalContext());
   DeviceDiagBuilder::Kind DiagKind = [this, FD] {
-    if (ConstructingOpenCLKernel)
+    if (DiagnosingSYCLKernel)
       return DeviceDiagBuilder::K_ImmediateWithCallStack;
     if (!FD)
       return DeviceDiagBuilder::K_Nop;

clang/test/SemaSYCL/kernel-not-functor.cpp

@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify %s
+
+template <typename Name, typename F>
+__attribute__((sycl_kernel)) void kernel(F kernelFunc) {
+  kernelFunc();
+}
+
+template <typename Name, typename F>
+void uses_kernel(F kernelFunc) {
+  // expected-error@+1{{kernel parameter must be a lambda or function object}}
+  kernel<Name>(kernelFunc);
+}
+
+void func() {}
+
+template <typename Name>
+void kernel_wrapper() {
+  // expected-note@+1{{in instantiation of function template specialization}}
+  uses_kernel<Name>(func);
+}
+
+void use() {
+  // expected-note@+1{{in instantiation of function template specialization}}
+  kernel_wrapper<class Foo>();
+}

clang/test/SemaSYCL/lambda_implicit_capture_this.cpp

@@ -13,6 +13,7 @@ class Class {
 };
 
 void Class::function() {
+  // expected-note@+1{{used here}}
   kernel<class kernel_wrapper>(
       [=]() {
         int acc[1] = {5};