[SYCL] Add type checking to SYCL accessors. (#1741)

Signed-off-by: Chris Perkins <[email protected]>

Author: cperkinsintel

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10816,7 +10816,6 @@ def err_sycl_restrict : Error<
   "}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">;
 def note_sycl_recursive_function_declared_here: Note<"function implemented using recursion declared here">;
 def err_sycl_non_trivially_copy_ctor_dtor_type
     : Error<"kernel parameter has non-trivially %select{copy "

clang/lib/Sema/SemaSYCL.cpp

@@ -210,19 +210,9 @@ static bool isZeroSizedArray(QualType Ty) {
   return false;
 }
 
-static Sema::DeviceDiagBuilder
-emitDeferredDiagnosticAndNote(Sema &S, SourceRange Loc, unsigned DiagID,
-                              SourceRange UsedAtLoc) {
-  Sema::DeviceDiagBuilder builder =
-      S.SYCLDiagIfDeviceCode(Loc.getBegin(), DiagID);
-  if (UsedAtLoc.isValid())
-    S.SYCLDiagIfDeviceCode(UsedAtLoc.getBegin(), diag::note_sycl_used_here);
-  return builder;
-}
-
-static void checkSYCLVarType(Sema &S, QualType Ty, SourceRange Loc,
-                             llvm::DenseSet<QualType> Visited,
-                             SourceRange UsedAtLoc = SourceRange()) {
+static void checkSYCLType(Sema &S, QualType Ty, SourceRange Loc,
+                          llvm::DenseSet<QualType> Visited,
+                          SourceRange UsedAtLoc = SourceRange()) {
   // Not all variable types are supported inside SYCL kernels,
   // for example the quad type __float128 will cause errors in the
   // SPIR-V translation phase.
@@ -233,16 +223,21 @@ static void checkSYCLVarType(Sema &S, QualType Ty, SourceRange Loc,
   // different location than the variable declaration and we need to
   // inform the user of both, e.g. struct member usage vs declaration.
 
+  bool Emitting = false;
+
   //--- check types ---
 
   // zero length arrays
-  if (isZeroSizedArray(Ty))
-    emitDeferredDiagnosticAndNote(S, Loc, diag::err_typecheck_zero_array_size,
-                                  UsedAtLoc);
+  if (isZeroSizedArray(Ty)) {
+    S.SYCLDiagIfDeviceCode(Loc.getBegin(), diag::err_typecheck_zero_array_size);
+    Emitting = true;
+  }
 
   // variable length arrays
-  if (Ty->isVariableArrayType())
-    emitDeferredDiagnosticAndNote(S, Loc, diag::err_vla_unsupported, UsedAtLoc);
+  if (Ty->isVariableArrayType()) {
+    S.SYCLDiagIfDeviceCode(Loc.getBegin(), diag::err_vla_unsupported);
+    Emitting = true;
+  }
 
   // Sub-reference array or pointer, then proceed with that type.
   while (Ty->isAnyPointerType() || Ty->isArrayType())
@@ -253,9 +248,14 @@ static void checkSYCLVarType(Sema &S, QualType Ty, SourceRange Loc,
       Ty->isSpecificBuiltinType(BuiltinType::UInt128) ||
       Ty->isSpecificBuiltinType(BuiltinType::LongDouble) ||
       (Ty->isSpecificBuiltinType(BuiltinType::Float128) &&
-       !S.Context.getTargetInfo().hasFloat128Type()))
-    emitDeferredDiagnosticAndNote(S, Loc, diag::err_type_unsupported, UsedAtLoc)
+       !S.Context.getTargetInfo().hasFloat128Type())) {
+    S.SYCLDiagIfDeviceCode(Loc.getBegin(), diag::err_type_unsupported)
         << Ty.getUnqualifiedType().getCanonicalType();
+    Emitting = true;
+  }
+
+  if (Emitting && UsedAtLoc.isValid())
+    S.SYCLDiagIfDeviceCode(UsedAtLoc.getBegin(), diag::note_used_here);
 
   //--- now recurse ---
   // Pointers complicate recursion. Add this type to Visited.
@@ -264,16 +264,15 @@ static void checkSYCLVarType(Sema &S, QualType Ty, SourceRange Loc,
     return;
 
   if (const auto *ATy = dyn_cast<AttributedType>(Ty))
-    return checkSYCLVarType(S, ATy->getModifiedType(), Loc, Visited);
+    return checkSYCLType(S, ATy->getModifiedType(), Loc, Visited);
 
   if (const auto *RD = Ty->getAsRecordDecl()) {
     for (const auto &Field : RD->fields())
-      checkSYCLVarType(S, Field->getType(), Field->getSourceRange(), Visited,
-                       Loc);
+      checkSYCLType(S, Field->getType(), Field->getSourceRange(), Visited, Loc);
   } else if (const auto *FPTy = dyn_cast<FunctionProtoType>(Ty)) {
     for (const auto &ParamTy : FPTy->param_types())
-      checkSYCLVarType(S, ParamTy, Loc, Visited);
-    checkSYCLVarType(S, FPTy->getReturnType(), Loc, Visited);
+      checkSYCLType(S, ParamTy, Loc, Visited);
+    checkSYCLType(S, FPTy->getReturnType(), Loc, Visited);
   }
 }
 
@@ -284,7 +283,7 @@ void Sema::checkSYCLDeviceVarDecl(VarDecl *Var) {
   SourceRange Loc = Var->getLocation();
   llvm::DenseSet<QualType> Visited;
 
-  checkSYCLVarType(*this, Ty, Loc, Visited);
+  checkSYCLType(*this, Ty, Loc, Visited);
 }
 
 class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
@@ -805,6 +804,22 @@ class SyclKernelFieldChecker
   bool IsInvalid = false;
   DiagnosticsEngine &Diag;
 
+  void checkAccessorType(QualType Ty, SourceRange Loc) {
+    assert(Util::isSyclAccessorType(Ty) &&
+           "Should only be called on SYCL accessor types.");
+
+    const RecordDecl *RecD = Ty->getAsRecordDecl();
+    if (const ClassTemplateSpecializationDecl *CTSD =
+            dyn_cast<ClassTemplateSpecializationDecl>(RecD)) {
+      const TemplateArgumentList &TAL = CTSD->getTemplateArgs();
+      TemplateArgument TA = TAL.get(0);
+      const QualType TemplateArgTy = TA.getAsType();
+
+      llvm::DenseSet<QualType> Visited;
+      checkSYCLType(SemaRef, TemplateArgTy, Loc, Visited);
+    }
+  }
+
 public:
   SyclKernelFieldChecker(Sema &S)
       : SyclKernelFieldHandler(S), Diag(S.getASTContext().getDiagnostics()) {}
@@ -836,6 +851,15 @@ class SyclKernelFieldChecker
     }
   }
 
+  void handleSyclAccessorType(const CXXBaseSpecifier &BS,
+                              QualType FieldTy) final {
+    checkAccessorType(FieldTy, BS.getBeginLoc());
+  }
+
+  void handleSyclAccessorType(FieldDecl *FD, QualType FieldTy) final {
+    checkAccessorType(FieldTy, FD->getLocation());
+  }
+
   // We should be able to handle this, so we made it part of the visitor, but
   // this is 'to be implemented'.
   void handleArrayType(FieldDecl *FD, QualType FieldTy) final {
@@ -1543,7 +1567,9 @@ 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 || !FD)
+    if (ConstructingOpenCLKernel)
+      return DeviceDiagBuilder::K_ImmediateWithCallStack;
+    if (!FD)
       return DeviceDiagBuilder::K_Nop;
     if (getEmissionStatus(FD) == Sema::FunctionEmissionStatus::Emitted)
       return DeviceDiagBuilder::K_ImmediateWithCallStack;

clang/test/SemaSYCL/accessor-type-diagnostics.cpp

@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -I %S/Inputs -fsycl -triple spir64 -fsycl-is-device -verify -fsyntax-only  %s
+//
+// Ensure SYCL type restrictions are applied to accessors as well.
+
+#include <sycl.hpp>
+
+using namespace cl::sycl;
+
+template <typename name, typename Func>
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  kernelFunc();
+}
+
+//alias template
+template <typename...>
+using int128alias_t = __uint128_t;
+
+//templated return type
+template <typename T>
+T bar() { return T(); };
+
+//typedef
+typedef __float128 trickyFloatType;
+
+//struct
+struct Mesh {
+  __int128 prohib; //#struct_member
+};
+
+int main() {
+  accessor<int, 1, access::mode::read_write> ok_acc;
+  // -- accessors using prohibited types
+  accessor<__float128, 1, access::mode::read_write> f128_acc;
+  accessor<__int128, 1, access::mode::read_write> i128_acc;
+  accessor<long double, 1, access::mode::read_write> ld_acc;
+  // -- pointers, aliases, auto, typedef, decltype of prohibited type
+  accessor<__int128 *, 1, access::mode::read_write> i128Ptr_acc;
+  accessor<int128alias_t<int>, 1, access::mode::read_write> aliased_acc;
+  accessor<trickyFloatType, 1, access::mode::read_write> typedef_acc;
+  auto V = bar<__int128>();
+  accessor<decltype(V), 1, access::mode::read_write> declty_acc;
+  // -- Accessor of struct that contains a prohibited type.
+  accessor<Mesh, 1, access::mode::read_write> struct_acc;
+
+  kernel<class use_local>(
+      [=]() {
+        ok_acc.use();
+
+        // -- accessors using prohibited types
+        // expected-error@+1 {{'__float128' is not supported on this target}}
+        f128_acc.use();
+        // expected-error@+1 {{'__int128' is not supported on this target}}
+        i128_acc.use();
+        // expected-error@+1 {{'long double' is not supported on this target}}
+        ld_acc.use();
+
+        // -- pointers, aliases, auto, typedef, decltype of prohibited type
+        // expected-error@+1 {{'__int128' is not supported on this target}}
+        i128Ptr_acc.use();
+        // expected-error@+1 {{'unsigned __int128' is not supported on this target}}
+        aliased_acc.use();
+        // expected-error@+1 {{'__float128' is not supported on this target}}
+        typedef_acc.use();
+        // expected-error@+1 {{'__int128' is not supported on this target}}
+        declty_acc.use();
+
+        // -- Accessor of struct that contains a prohibited type.
+        // expected-error@#struct_member {{'__int128' is not supported on this target}}
+        // expected-note@+1 {{used here}}
+        struct_acc.use();
+      });
+}