[SYCL][UX] Diagnostic for undefined device functions (#1026)

Brand new diagnostic introduced: "call an undefined function w/o SYCL_EXTERNAL attribute"
The diagnostic is displayed for functions definitions of which are not available when its declaration is not marked with SYCL_EXTERNAL attribute if and only if the method is found in call graph (i.e. it will be called during runtime).

Signed-off-by: Sergey Kanaev <[email protected]>

Author: s-kanaev

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10642,7 +10642,9 @@ def err_sycl_restrict : Error<
   "|allocate storage"
   "|use inline assembly"
   "|call a dllimport function"
-  "|call a variadic function}0">;
+  "|call a variadic function"
+  "|call an undefined function without SYCL_EXTERNAL attribute"
+  "}0">;
 def err_sycl_virtual_types : Error<
   "No class with a vtable can be used in a SYCL kernel or any code included in the kernel">;
 def note_sycl_used_here : Note<"used here">;

clang/include/clang/Sema/Sema.h

@@ -12298,11 +12298,13 @@ class Sema final {
     KernelAllocateStorage,
     KernelUseAssembly,
     KernelCallDllimportFunction,
-    KernelCallVariadicFunction
- };
+    KernelCallVariadicFunction,
+    KernelCallUndefinedFunction
+  };
+
   bool isKnownGoodSYCLDecl(const Decl *D);
   void ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, MangleContext &MC);
-  void MarkDevice(void);
+  void MarkDevice();
 
   /// Creates a DeviceDiagBuilder that emits the diagnostic if the current
   /// context is "used as device code".
@@ -12323,10 +12325,25 @@ class Sema final {
   ///   SYCLDiagIfDeviceCode(Loc, diag::err_thread_unsupported);
   DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
 
-  /// Checks if Callee function is a device function and emits
-  /// diagnostics if it is known that it is a device function, adds this
-  /// function to the DeviceCallGraph otherwise.
-  void checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+  /// Check whether we're allowed to call Callee from the current context.
+  ///
+  /// - If the call is never allowed in a semantically-correct program
+  ///   emits an error and returns false.
+  ///
+  /// - If the call is allowed in semantically-correct programs, but only if
+  ///   it's never codegen'ed, creates a deferred diagnostic to be emitted if
+  ///   and when the caller is codegen'ed, and returns true.
+  ///
+  /// - Otherwise, returns true without emitting any diagnostics.
+  ///
+  /// Adds Callee to DeviceCallGraph if we don't know if its caller will be
+  /// codegen'ed yet.
+  bool checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+
+  /// Emit diagnostic that can't be emitted with deferred diagnostics mechanism.
+  /// At this step we imply that all device functions are marked with
+  /// sycl_device attribute.
+  void finalizeSYCLDelayedAnalysis();
 };
 
 template <typename AttrType>

clang/lib/Driver/ToolChains/Clang.cpp

@@ -4124,6 +4124,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   } else if (IsSYCL) {
     // Ensure the default version in SYCL mode is 1.2.1
     CmdArgs.push_back("-sycl-std=1.2.1");
+    // The user had not pass SYCL version, thus we'll employ no-sycl-strict
+    // to allow address-space unqualified pointers in function params/return
+    // along with marking the same function with explicit SYCL_EXTERNAL
+    CmdArgs.push_back("-Wno-sycl-strict");
   }
 
   if (IsOpenMPDevice) {

clang/lib/Sema/Sema.cpp

@@ -974,6 +974,7 @@ void Sema::ActOnEndOfTranslationUnitFragment(TUFragmentKind Kind) {
     if (SyclIntHeader != nullptr)
       SyclIntHeader->emit(getLangOpts().SYCLIntHeader);
     MarkDevice();
+    finalizeSYCLDelayedAnalysis();
   }
 
   // Finalize analysis of OpenMP-specific constructs.

clang/lib/Sema/SemaDecl.cpp

@@ -18016,6 +18016,12 @@ Decl *Sema::getObjCDeclContext() const {
 }
 
 Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD) {
+  // Due to SYCL functions are template we check if they have appropriate
+  // attribute prior to checking if it is a template
+  if (LangOpts.SYCLIsDevice &&
+      (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>()))
+    return FunctionEmissionStatus::Emitted;
+
   // Templates are emitted when they're instantiated.
   if (FD->isDependentContext())
     return FunctionEmissionStatus::TemplateDiscarded;
@@ -18080,6 +18086,23 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD) {
       return FunctionEmissionStatus::Emitted;
   }
 
+  if (getLangOpts().SYCLIsDevice) {
+    if (!FD->hasAttr<SYCLDeviceAttr>() && !FD->hasAttr<SYCLKernelAttr>())
+      return FunctionEmissionStatus::Unknown;
+
+    // Check whether this function is externally visible -- if so, it's
+    // known-emitted.
+    //
+    // We have to check the GVA linkage of the function's *definition* -- if we
+    // only have a declaration, we don't know whether or not the function will
+    // be emitted, because (say) the definition could include "inline".
+    FunctionDecl *Def = FD->getDefinition();
+
+    if (Def &&
+        !isDiscardableGVALinkage(getASTContext().GetGVALinkageForFunction(Def)))
+      return FunctionEmissionStatus::Emitted;
+  }
+
   // Otherwise, the function is known-emitted if it's in our set of
   // known-emitted functions.
   return (DeviceKnownEmittedFns.count(FD) > 0)

clang/lib/Sema/SemaDeclCXX.cpp

@@ -14684,8 +14684,9 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
   MarkFunctionReferenced(ConstructLoc, Constructor);
   if (getLangOpts().CUDA && !CheckCUDACall(ConstructLoc, Constructor))
     return ExprError();
-  if (getLangOpts().SYCLIsDevice)
-    checkSYCLDeviceFunction(ConstructLoc, Constructor);
+  if (getLangOpts().SYCLIsDevice &&
+      !checkSYCLDeviceFunction(ConstructLoc, Constructor))
+    return ExprError();
 
   return CXXConstructExpr::Create(
       Context, DeclInitType, ConstructLoc, Constructor, Elidable,

clang/lib/Sema/SemaExpr.cpp

@@ -301,8 +301,8 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
     if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
       return true;
 
-    if (getLangOpts().SYCLIsDevice)
-      checkSYCLDeviceFunction(Loc, FD);
+    if (getLangOpts().SYCLIsDevice && !checkSYCLDeviceFunction(Loc, FD))
+      return true;
   }
 
   if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {

clang/lib/Sema/SemaSYCL.cpp

@@ -537,6 +537,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
     }
     return true;
   }
+
   Sema &SemaRef;
 };
 
@@ -1410,18 +1411,6 @@ void Sema::MarkDevice(void) {
 // SYCL device specific diagnostics implementation
 // -----------------------------------------------------------------------------
 
-// Do we know that we will eventually codegen the given function?
-static bool isKnownEmitted(Sema &S, FunctionDecl *FD) {
-  assert(FD && "Given function may not be null.");
-
-  if (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>())
-    return true;
-
-  // Otherwise, the function is known-emitted if it's in our set of
-  // known-emitted functions.
-  return S.DeviceKnownEmittedFns.count(FD) > 0;
-}
-
 Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
                                                    unsigned DiagID) {
   assert(getLangOpts().SYCLIsDevice &&
@@ -1430,29 +1419,104 @@ Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
   DeviceDiagBuilder::Kind DiagKind = [this, FD] {
     if (ConstructingOpenCLKernel || !FD)
       return DeviceDiagBuilder::K_Nop;
-    if (isKnownEmitted(*this, FD))
+    if (getEmissionStatus(FD) == Sema::FunctionEmissionStatus::Emitted)
       return DeviceDiagBuilder::K_ImmediateWithCallStack;
     return DeviceDiagBuilder::K_Deferred;
   }();
   return DeviceDiagBuilder(DiagKind, Loc, DiagID, FD, *this);
 }
 
-void Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
+bool Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
+  assert(getLangOpts().SYCLIsDevice &&
+         "Should only be called during SYCL compilation");
   assert(Callee && "Callee may not be null.");
 
   // Errors in unevaluated context don't need to be generated,
   // so we can safely skip them.
-  if (isUnevaluatedContext())
-    return;
+  if (isUnevaluatedContext() || isConstantEvaluated())
+    return true;
 
   FunctionDecl *Caller = dyn_cast<FunctionDecl>(getCurLexicalContext());
 
+  if (!Caller)
+    return true;
+
+  bool CallerKnownEmitted =
+      getEmissionStatus(Caller) == FunctionEmissionStatus::Emitted;
+
   // If the caller is known-emitted, mark the callee as known-emitted.
   // Otherwise, mark the call in our call graph so we can traverse it later.
-  if (Caller && isKnownEmitted(*this, Caller))
-    markKnownEmitted(*this, Caller, Callee, Loc, isKnownEmitted);
-  else if (Caller)
+  if (CallerKnownEmitted)
+    markKnownEmitted(*this, Caller, Callee, Loc, [](Sema &S, FunctionDecl *FD) {
+      return S.getEmissionStatus(FD) == Sema::FunctionEmissionStatus::Emitted;
+    });
+  else
     DeviceCallGraph[Caller].insert({Callee, Loc});
+
+  DeviceDiagBuilder::Kind DiagKind = DeviceDiagBuilder::K_Nop;
+
+  // TODO Set DiagKind to K_Immediate/K_Deferred to emit diagnostics for Callee
+
+  DeviceDiagBuilder(DiagKind, Loc, diag::err_sycl_restrict, Caller, *this)
+      << Sema::KernelCallUndefinedFunction;
+  DeviceDiagBuilder(DiagKind, Callee->getLocation(), diag::note_previous_decl,
+                    Caller, *this)
+      << Callee;
+
+  return DiagKind != DeviceDiagBuilder::K_Immediate &&
+         DiagKind != DeviceDiagBuilder::K_ImmediateWithCallStack;
+}
+
+static void emitCallToUndefinedFnDiag(Sema &SemaRef, const FunctionDecl *Callee,
+                                      const FunctionDecl *Caller,
+                                      const SourceLocation &Loc) {
+  // 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;
+
+  // Don't emit diagnostic for functions not called from device code
+  if (!Caller->hasAttr<SYCLDeviceAttr>() && !Caller->hasAttr<SYCLKernelAttr>())
+    return;
+
+  bool RedeclHasAttr = false;
+
+  for (const Decl *Redecl : Callee->redecls()) {
+    if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Redecl)) {
+      if ((FD->hasAttr<SYCLDeviceAttr>() &&
+           !FD->getAttr<SYCLDeviceAttr>()->isImplicit()) ||
+          FD->hasAttr<SYCLKernelAttr>()) {
+        RedeclHasAttr = true;
+        break;
+      }
+    }
+  }
+
+  // Disallow functions with neither definition nor SYCL_EXTERNAL mark
+  bool NotDefinedNoAttr = !Callee->isDefined() && !RedeclHasAttr;
+
+  if (NotDefinedNoAttr && !Callee->getBuiltinID()) {
+    SemaRef.Diag(Loc, diag::err_sycl_restrict)
+        << Sema::KernelCallUndefinedFunction;
+    SemaRef.Diag(Callee->getLocation(), diag::note_previous_decl) << Callee;
+    SemaRef.Diag(Caller->getLocation(), diag::note_called_by) << Caller;
+  }
+}
+
+void Sema::finalizeSYCLDelayedAnalysis() {
+  assert(getLangOpts().SYCLIsDevice &&
+         "Should only be called during SYCL compilation");
+
+  llvm::DenseSet<const FunctionDecl *> Checked;
+
+  for (const auto &EmittedWithLoc : DeviceKnownEmittedFns) {
+    const FunctionDecl *Caller = EmittedWithLoc.getSecond().FD;
+    const SourceLocation &Loc = EmittedWithLoc.getSecond().Loc;
+    const FunctionDecl *Callee = EmittedWithLoc.getFirst();
+
+    if (Checked.insert(Callee).second)
+      emitCallToUndefinedFnDiag(*this, Callee, Caller, Loc);
+  }
 }
 
 // -----------------------------------------------------------------------------

clang/test/CodeGenSYCL/address-space-new.cpp

@@ -10,7 +10,7 @@ struct HasX {
 
 struct Y : SpaceWaster, HasX {};
 
-void bar(HasX &hx);
+SYCL_EXTERNAL void bar(HasX &hx);
 
 void baz(Y &y) {
   bar(y);

clang/test/CodeGenSYCL/bool-vectors.cpp

@@ -11,17 +11,17 @@ using bool4 = bool __attribute__((ext_vector_type(4)));
 using bool8 = bool __attribute__((ext_vector_type(8)));
 using bool16 = bool __attribute__((ext_vector_type(16)));
 
-extern bool1 foo1();
+extern SYCL_EXTERNAL bool1 foo1();
 // CHECK-DAG: declare spir_func zeroext i1 @[[FOO1:[a-zA-Z0-9_]+]]()
-extern bool2 foo2();
+extern SYCL_EXTERNAL bool2 foo2();
 // CHECK-DAG: declare spir_func <2 x i1> @[[FOO2:[a-zA-Z0-9_]+]]()
-extern bool3 foo3();
+extern SYCL_EXTERNAL bool3 foo3();
 // CHECK-DAG: declare spir_func <3 x i1> @[[FOO3:[a-zA-Z0-9_]+]]()
-extern bool4 foo4();
+extern SYCL_EXTERNAL bool4 foo4();
 // CHECK-DAG: declare spir_func <4 x i1> @[[FOO4:[a-zA-Z0-9_]+]]()
-extern bool8 foo8();
+extern SYCL_EXTERNAL bool8 foo8();
 // CHECK-DAG: declare spir_func <8 x i1> @[[FOO8:[a-zA-Z0-9_]+]]()
-extern bool16 foo16();
+extern SYCL_EXTERNAL bool16 foo16();
 // CHECK-DAG: declare spir_func <16 x i1> @[[FOO16:[a-zA-Z0-9_]+]]()
 
 void bar (bool1 b) {};

clang/test/CodeGenSYCL/fpga_pipes.cpp

@@ -3,10 +3,10 @@
 // CHECK: %opencl.pipe_ro_t
 
 using WPipeTy = __attribute__((pipe("write_only"))) const int;
-WPipeTy WPipeCreator();
+SYCL_EXTERNAL WPipeTy WPipeCreator();
 
 using RPipeTy = __attribute__((pipe("read_only"))) const int;
-RPipeTy RPipeCreator();
+SYCL_EXTERNAL RPipeTy RPipeCreator();
 
 template <typename PipeTy>
 void foo(PipeTy Pipe) {}
@@ -24,7 +24,7 @@ template <int N>
 constexpr PipeStorageTy
     TempStorage __attribute__((io_pipe_id(N))) = {2};
 
-void boo(PipeStorageTy PipeStorage);
+SYCL_EXTERNAL void boo(PipeStorageTy PipeStorage);
 
 template <int ID>
 struct ethernet_pipe {

clang/test/CodeGenSYCL/unique-stable-name.cpp

@@ -8,7 +8,7 @@
 // CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIiEvvEUlvE23->12\00",
 // CHECK: @[[LAMBDA_IN_DEP_X:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIZZ4mainENKUlvE42->5clEvEUlvE46->16EvvEUlvE23->12\00",
 
-extern "C" void printf(const char*);
+extern "C" void printf(const char *) {}
 
 template <typename T>
 void template_param() {