[SYCL] Diagnose __float128 type usage in device code, fixes and cleanups (#971)

The problem is that CheckSYCLCall used to get the caller incorrectly.
getCurFunctionDecl gets the wrong thing. It will get the function
containing the lambda at definition time, rather than the lambda operator().

Additional changes:
Cleaned up unnecessary calls of CheckSYCLCall function.
Fixed diagnosing of storage allocation through deferred diagnostics
system.
Renamed CheckSYCLCall with checkSYCLDeviceFunction since we don't really
check calls as CUDA/OpenMP does.
Removed unnecessary emitting of diagnostics from SemaSYCL.

Signed-off-by: Mariya Podchishchaeva <[email protected]>

Author: Fznamznon

clang/include/clang/Sema/Sema.h

@@ -12079,11 +12079,33 @@ class Sema final {
     KernelCallDllimportFunction,
     KernelCallVariadicFunction
  };
-  DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
   bool isKnownGoodSYCLDecl(const Decl *D);
   void ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, MangleContext &MC);
   void MarkDevice(void);
-  bool CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee);
+
+  /// Creates a DeviceDiagBuilder that emits the diagnostic if the current
+  /// context is "used as device code".
+  ///
+  /// - If CurLexicalContext is a kernel function or it is known that the
+  ///   function will be emitted for the device, emits the diagnostics
+  ///   immediately.
+  /// - If CurLexicalContext is a function and we are compiling
+  ///   for the device, but we don't know that this function will be codegen'ed
+  ///   for devive yet, creates a diagnostic which is emitted if and when we
+  ///   realize that the function will be codegen'ed.
+  ///
+  /// Example usage:
+  ///
+  /// Variables with thread storage duration are not allowed to be used in SYCL
+  /// device code
+  /// if (getLangOpts().SYCLIsDevice)
+  ///   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);
 };
 
 /// RAII object that enters a new expression evaluation context.

clang/lib/Sema/Sema.cpp

@@ -1610,6 +1610,9 @@ Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc, unsigned DiagID) {
   if (getLangOpts().CUDA)
     return getLangOpts().CUDAIsDevice ? CUDADiagIfDeviceCode(Loc, DiagID)
                                       : CUDADiagIfHostCode(Loc, DiagID);
+  // TODO: analyze which usages of targetDiag could be reused for SYCL.
+  // if (getLangOpts().SYCLIsDevice)
+  //   return SYCLDiagIfDeviceCode(Loc, DiagID);
   return DeviceDiagBuilder(DeviceDiagBuilder::K_Immediate, Loc, DiagID,
                            getCurFunctionDecl(), *this);
 }

clang/lib/Sema/SemaDeclCXX.cpp

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

clang/lib/Sema/SemaExpr.cpp

@@ -269,7 +269,7 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
       return true;
 
     if (getLangOpts().SYCLIsDevice)
-      CheckSYCLCall(Loc, FD);
+      checkSYCLDeviceFunction(Loc, FD);
   }
 
   if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {
@@ -15649,7 +15649,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
   if (getLangOpts().CUDA)
     CheckCUDACall(Loc, Func);
   if (getLangOpts().SYCLIsDevice)
-    CheckSYCLCall(Loc, Func);
+    checkSYCLDeviceFunction(Loc, Func);
 
   // If we need a definition, try to create one.
   if (NeedDefinition && !Func->getBody()) {
@@ -17219,15 +17219,7 @@ namespace {
     }
 
     void VisitCXXNewExpr(CXXNewExpr *E) {
-      FunctionDecl *FD = E->getOperatorNew();
-      if (FD && S.getLangOpts().SYCLIsDevice) {
-        if (FD->isReplaceableGlobalAllocationFunction())
-          S.SYCLDiagIfDeviceCode(E->getExprLoc(), diag::err_sycl_restrict)
-              << S.KernelAllocateStorage;
-        else if (FunctionDecl *Def = FD->getDefinition())
-          S.CheckSYCLCall(E->getExprLoc(), Def);
-      }
-      if (FD)
+      if (E->getOperatorNew())
         S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorNew());
       if (E->getOperatorDelete())
         S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorDelete());

clang/lib/Sema/SemaExprCXX.cpp

@@ -2171,16 +2171,15 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     if (DiagnoseUseOfDecl(OperatorNew, StartLoc))
       return ExprError();
     MarkFunctionReferenced(StartLoc, OperatorNew);
-    if (getLangOpts().SYCLIsDevice) {
-      CheckSYCLCall(StartLoc, OperatorNew);
-    }
+    if (getLangOpts().SYCLIsDevice &&
+        OperatorNew->isReplaceableGlobalAllocationFunction())
+      SYCLDiagIfDeviceCode(StartLoc, diag::err_sycl_restrict)
+          << KernelAllocateStorage;
   }
   if (OperatorDelete) {
     if (DiagnoseUseOfDecl(OperatorDelete, StartLoc))
       return ExprError();
     MarkFunctionReferenced(StartLoc, OperatorDelete);
-    if (getLangOpts().SYCLIsDevice)
-      CheckSYCLCall(StartLoc, OperatorDelete);
   }
 
   return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete,

clang/lib/Sema/SemaOverload.cpp

@@ -12916,8 +12916,6 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
                             FnDecl->getType()->castAs<FunctionProtoType>()))
         return ExprError();
 
-      if (getLangOpts().SYCLIsDevice)
-        CheckSYCLCall(OpLoc, FnDecl);
       return MaybeBindToTemporary(TheCall);
     } else {
       // We matched a built-in operator. Convert the arguments, then
@@ -13270,8 +13268,6 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
                   isa<CXXMethodDecl>(FnDecl), OpLoc, TheCall->getSourceRange(),
                   VariadicDoesNotApply);
 
-        if (getLangOpts().SYCLIsDevice)
-          CheckSYCLCall(OpLoc, FnDecl);
         ExprResult R = MaybeBindToTemporary(TheCall);
         if (R.isInvalid())
           return ExprError();
@@ -13633,8 +13629,6 @@ Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
                               Method->getType()->castAs<FunctionProtoType>()))
           return ExprError();
 
-        if (getLangOpts().SYCLIsDevice)
-          CheckSYCLCall(RLoc, FnDecl);
         return MaybeBindToTemporary(TheCall);
       } else {
         // We matched a built-in operator. Convert the arguments, then

clang/lib/Sema/SemaSYCL.cpp

@@ -359,10 +359,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
     // new operator and any user-defined overloads that
     // do not allocate storage are permitted.
     if (FunctionDecl *FD = E->getOperatorNew()) {
-      if (FD->isReplaceableGlobalAllocationFunction()) {
-        SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict)
-            << Sema::KernelAllocateStorage;
-      } else if (FunctionDecl *Def = FD->getDefinition()) {
+      if (FunctionDecl *Def = FD->getDefinition()) {
         if (!Def->hasAttr<SYCLDeviceAttr>()) {
           Def->addAttr(SYCLDeviceAttr::CreateImplicit(SemaRef.Context));
           SemaRef.addSyclDeviceDecl(Def);
@@ -529,8 +526,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
         if (!CheckSYCLType(Field->getType(), Field->getSourceRange(),
                            Visited)) {
           if (SemaRef.getLangOpts().SYCLIsDevice)
-            SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(),
-                                         diag::note_sycl_used_here);
+            SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
           return false;
         }
       }
@@ -539,8 +535,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
         if (!CheckSYCLType(Field->getType(), Field->getSourceRange(),
                            Visited)) {
           if (SemaRef.getLangOpts().SYCLIsDevice)
-            SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(),
-                                         diag::note_sycl_used_here);
+            SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
           return false;
         }
       }
@@ -1399,8 +1394,7 @@ void Sema::MarkDevice(void) {
 
 // Do we know that we will eventually codegen the given function?
 static bool isKnownEmitted(Sema &S, FunctionDecl *FD) {
-  if (!FD)
-    return true; // Seen in LIT testing
+  assert(FD && "Given function may not be null.");
 
   if (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>())
     return true;
@@ -1416,16 +1410,16 @@ 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 (ConstructingOpenCLKernel || !FD)
       return DeviceDiagBuilder::K_Nop;
-    else if (isKnownEmitted(*this, FD))
+    if (isKnownEmitted(*this, FD))
       return DeviceDiagBuilder::K_ImmediateWithCallStack;
     return DeviceDiagBuilder::K_Deferred;
   }();
   return DeviceDiagBuilder(DiagKind, Loc, DiagID, FD, *this);
 }
 
-bool Sema::CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee) {
+void Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
   assert(Callee && "Callee may not be null.");
   FunctionDecl *Caller = dyn_cast<FunctionDecl>(getCurLexicalContext());
 
@@ -1435,7 +1429,6 @@ bool Sema::CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee) {
     markKnownEmitted(*this, Caller, Callee, Loc, isKnownEmitted);
   else if (Caller)
     DeviceCallGraph[Caller].insert({Callee, Loc});
-  return true;
 }
 
 // -----------------------------------------------------------------------------

clang/lib/Sema/SemaStmtAsm.cpp

@@ -259,12 +259,11 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
   // Skip all the checks if we are compiling SYCL device code, but the function
   // is not marked to be used on device, this code won't be codegen'ed anyway.
   if (getLangOpts().SYCLIsDevice) {
-    SYCLDiagIfDeviceCode(AsmLoc, diag::err_sycl_restrict)
-        << KernelUseAssembly;
+    SYCLDiagIfDeviceCode(AsmLoc, diag::err_sycl_restrict) << KernelUseAssembly;
     return new (Context)
-      GCCAsmStmt(Context, AsmLoc, IsSimple, IsVolatile, NumOutputs,
-                 NumInputs, Names, Constraints, Exprs.data(), AsmString,
-                 NumClobbers, Clobbers, NumLabels, RParenLoc);
+        GCCAsmStmt(Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs,
+                   Names, Constraints, Exprs.data(), AsmString, NumClobbers,
+                   Clobbers, NumLabels, RParenLoc);
   }
 
   FunctionDecl *FD = dyn_cast<FunctionDecl>(getCurLexicalContext());

clang/lib/Sema/SemaType.cpp

@@ -1500,11 +1500,15 @@ static QualType ConvertDeclSpecToType(TypeProcessingState &state) {
       Result = Context.DoubleTy;
     break;
   case DeclSpec::TST_float128:
-    if (!S.Context.getTargetInfo().hasFloat128Type() && 
-        !S.getLangOpts().SYCLIsDevice &&
-        !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+    if (!S.Context.getTargetInfo().hasFloat128Type() &&
+        S.getLangOpts().SYCLIsDevice)
+      S.SYCLDiagIfDeviceCode(DS.getTypeSpecTypeLoc(),
+                             diag::err_type_unsupported)
+          << "__float128";
+    else if (!S.Context.getTargetInfo().hasFloat128Type() &&
+             !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
       S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
-        << "__float128";
+          << "__float128";
     Result = Context.Float128Ty;
     break;
   case DeclSpec::TST_bool: Result = Context.BoolTy; break; // _Bool or bool

clang/test/SemaSYCL/inline-asm.cpp

@@ -19,7 +19,7 @@ void bar() {
 #endif // LINUX_ASM
 }
 
-template <typename name, typename Func>
+template <typename Name, typename Func>
 __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
   // expected-note@+1 {{called by 'kernel_single_task<fake_kernel, (lambda}}
   kernelFunc();

clang/test/SemaSYCL/restrict-recursion3.cpp

@@ -16,6 +16,8 @@ void kernel3(void) {
 using myFuncDef = int(int,int);
 
 void usage3(myFuncDef functionPtr) {
+  // expected-error@+1 {{SYCL kernel cannot allocate storage}}
+  int *ip = new int;
   kernel3();
 }
 
@@ -26,14 +28,14 @@ int addInt(int n, int m) {
 template <typename name, typename Func>
   // expected-note@+1 2{{function implemented using recursion declared here}}
 __attribute__((sycl_kernel)) void kernel_single_task2(Func kernelFunc) {
+  // expected-note@+1 {{called by 'kernel_single_task2}}
   kernelFunc();
-  // expected-error@+1 2{{SYCL kernel cannot allocate storage}}
-  int *ip = new int;
   // expected-error@+1 2{{SYCL kernel cannot call a recursive function}}
   kernel_single_task2<name, Func>(kernelFunc);
 }
 
 int main() {
+  // expected-note@+1 {{called by 'operator()'}}
   kernel_single_task2<class fake_kernel>([]() { usage3(  &addInt ); });
   return fib(5);
 }

clang/test/SemaSYCL/restrict-recursion4.cpp

@@ -18,6 +18,8 @@ void kernel2(void) {
 using myFuncDef = int(int,int);
 
 void usage2(myFuncDef functionPtr) {
+  // expected-error@+1 {{SYCL kernel cannot allocate storage}}
+  int *ip = new int;
   // expected-error@+1 {{SYCL kernel cannot call a recursive function}}
   kernel2();
 }
@@ -28,12 +30,12 @@ int addInt(int n, int m) {
 
 template <typename name, typename Func>
 __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
-  // expected-error@+1 {{SYCL kernel cannot allocate storage}}
-  int *ip = new int;
+  // expected-note@+1 {{called by 'kernel_single_task}}
   kernelFunc();
 }
 
 int main() {
+  // expected-note@+1 {{called by 'operator()'}}
   kernel_single_task<class fake_kernel>([]() {usage2(&addInt);});
   return fib(5);
 }

clang/test/SemaSYCL/sycl-restrict.cpp

@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fsycl-is-device -Wno-return-type -verify -fsyntax-only -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsycl-is-device -fno-sycl-allow-func-ptr -Wno-return-type -verify -fsyntax-only -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsycl-is-device -DALLOW_FP=1 -fsycl-allow-func-ptr -Wno-return-type -verify -fsyntax-only -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -triple spir64 -fsycl-is-device -Wno-return-type -verify -fsyntax-only -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -triple spir64 -fsycl-is-device -fno-sycl-allow-func-ptr -Wno-return-type -verify -fsyntax-only -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -triple spir64 -fsycl-is-device -DALLOW_FP=1 -fsycl-allow-func-ptr -Wno-return-type -verify -fsyntax-only -std=c++17 %s
 
 
 namespace std {
@@ -65,6 +65,7 @@ bool isa_B(A *a) {
   // expected-error@+1 {{SYCL kernel cannot allocate storage}}
   int *ip = new int;
   int i; int *p3 = new(&i) int; // no error on placement new
+  // expected-note@+1 {{called by 'isa_B'}}
   OverloadedNewDelete *x = new( struct OverloadedNewDelete );
   auto y = new struct OverloadedNewDelete [5];
   // expected-error@+1 {{SYCL kernel cannot use rtti}}
@@ -102,6 +103,7 @@ using myFuncDef = int(int,int);
 
 void eh_ok(void)
 {
+  __float128 A;
   try {
     ;
   } catch (...) {
@@ -138,6 +140,9 @@ void usage(myFuncDef functionPtr) {
   Check_RTTI_Restriction::kernel1<class kernel_name>([]() {
   Check_RTTI_Restriction::A *a;
   Check_RTTI_Restriction::isa_B(a); });
+
+  // expected-error@+1 {{__float128 is not supported on this target}}
+  __float128 A;
 }
 
 namespace ns {
@@ -172,9 +177,12 @@ int use2 ( a_type ab, a_type *abp ) {
   // expected-note@+1 {{called by 'use2'}}
   eh_not_ok();
   Check_RTTI_Restriction:: A *a;
+  // expected-note@+1 2{{called by 'use2'}}
   Check_RTTI_Restriction:: isa_B(a);
+  // expected-note@+1 {{called by 'use2'}}
   usage(&addInt);
   Check_User_Operators::Fraction f1(3, 8), f2(1, 2), f3(10, 2);
+  // expected-note@+1 {{called by 'use2'}}
   if (f1 == f2) return false;
 }
 
@@ -183,7 +191,7 @@ __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
   kernelFunc();
   a_type ab;
   a_type *p;
-  // expected-note@+1 {{called by 'kernel_single_task'}}
+  // expected-note@+1 5{{called by 'kernel_single_task'}}
   use2(ab, p);
 }