[SYCL] Refactor semantic checks for variable types (#1513)

The various type checks have been steadily moving out of `CheckSYCLType` which is called by most of the AST Visitor methods.  Here we finally move the last lingering type check (for VLAs) into the `CheckSYCLVarType` function and delete `CheckSYCLType` and most of its AST Visitor method callers. 

Some few of the AST Visitor methods are still used for other checks. 

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

Author: cperkinsintel

clang/lib/Sema/SemaSYCL.cpp

@@ -236,6 +236,10 @@ static void checkSYCLVarType(Sema &S, QualType Ty, SourceRange Loc,
     emitDeferredDiagnosticAndNote(S, Loc, diag::err_typecheck_zero_array_size,
                                   UsedAtLoc);
 
+  // variable length arrays
+  if (Ty->isVariableArrayType())
+    emitDeferredDiagnosticAndNote(S, Loc, diag::err_vla_unsupported, UsedAtLoc);
+
   // Sub-reference array or pointer, then proceed with that type.
   while (Ty->isAnyPointerType() || Ty->isArrayType())
     Ty = QualType{Ty->getPointeeOrArrayElementType(), 0};
@@ -284,9 +288,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
       : RecursiveASTVisitor<MarkDeviceFunction>(), SemaRef(S) {}
 
   bool VisitCallExpr(CallExpr *e) {
-    for (const auto &Arg : e->arguments())
-      CheckSYCLType(Arg->getType(), Arg->getSourceRange());
-
     if (FunctionDecl *Callee = e->getDirectCallee()) {
       Callee = Callee->getCanonicalDecl();
       assert(Callee && "Device function canonical decl must be available");
@@ -308,8 +309,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
           SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict)
               << Sema::KernelCallVirtualFunction;
 
-      CheckSYCLType(Callee->getReturnType(), Callee->getSourceRange());
-
       if (auto const *FD = dyn_cast<FunctionDecl>(Callee)) {
         // FIXME: We need check all target specified attributes for error if
         // that function with attribute can not be called from sycl kernel.  The
@@ -338,12 +337,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
     return true;
   }
 
-  bool VisitCXXConstructExpr(CXXConstructExpr *E) {
-    for (const auto &Arg : E->arguments())
-      CheckSYCLType(Arg->getType(), Arg->getSourceRange());
-    return true;
-  }
-
   bool VisitCXXTypeidExpr(CXXTypeidExpr *E) {
     SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict) << Sema::KernelRTTI;
     return true;
@@ -354,35 +347,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
     return true;
   }
 
-  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-    CheckSYCLType(TD->getUnderlyingType(), TD->getLocation());
-    return true;
-  }
-
-  bool VisitRecordDecl(RecordDecl *RD) {
-    CheckSYCLType(QualType{RD->getTypeForDecl(), 0}, RD->getLocation());
-    return true;
-  }
-
-  bool VisitParmVarDecl(VarDecl *VD) {
-    CheckSYCLType(VD->getType(), VD->getLocation());
-    return true;
-  }
-
-  bool VisitVarDecl(VarDecl *VD) {
-    CheckSYCLType(VD->getType(), VD->getLocation());
-    return true;
-  }
-
-  bool VisitDeclRefExpr(DeclRefExpr *E) {
-    Decl *D = E->getDecl();
-    if (SemaRef.isKnownGoodSYCLDecl(D))
-      return true;
-
-    CheckSYCLType(E->getType(), E->getSourceRange());
-    return true;
-  }
-
   // The call graph for this translation unit.
   CallGraph SYCLCG;
   // The set of functions called by a kernel function.
@@ -506,64 +470,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
   }
 
 private:
-  bool CheckSYCLType(QualType Ty, SourceRange Loc) {
-    llvm::DenseSet<QualType> visited;
-    return CheckSYCLType(Ty, Loc, visited);
-  }
-
-  bool CheckSYCLType(QualType Ty, SourceRange Loc,
-                     llvm::DenseSet<QualType> &Visited) {
-    if (Ty->isVariableArrayType()) {
-      SemaRef.Diag(Loc.getBegin(), diag::err_vla_unsupported);
-      return false;
-    }
-
-    while (Ty->isAnyPointerType() || Ty->isArrayType())
-      Ty = QualType{Ty->getPointeeOrArrayElementType(), 0};
-
-    // Pointers complicate recursion. Add this type to Visited.
-    // If already there, bail out.
-    if (!Visited.insert(Ty).second)
-      return true;
-
-    if (const auto *ATy = dyn_cast<AttributedType>(Ty))
-      return CheckSYCLType(ATy->getModifiedType(), Loc, Visited);
-
-    if (const auto *CRD = Ty->getAsCXXRecordDecl()) {
-      // If the class is a forward declaration - skip it, because otherwise we
-      // would query property of class with no definition, which results in
-      // clang crash.
-      if (!CRD->hasDefinition())
-        return true;
-
-      for (const auto &Field : CRD->fields()) {
-        if (!CheckSYCLType(Field->getType(), Field->getSourceRange(),
-                           Visited)) {
-          if (SemaRef.getLangOpts().SYCLIsDevice)
-            SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
-          return false;
-        }
-      }
-    } else if (const auto *RD = Ty->getAsRecordDecl()) {
-      for (const auto &Field : RD->fields()) {
-        if (!CheckSYCLType(Field->getType(), Field->getSourceRange(),
-                           Visited)) {
-          if (SemaRef.getLangOpts().SYCLIsDevice)
-            SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
-          return false;
-        }
-      }
-    } else if (const auto *FPTy = dyn_cast<FunctionProtoType>(Ty)) {
-      for (const auto &ParamTy : FPTy->param_types())
-        if (!CheckSYCLType(ParamTy, Loc, Visited))
-          return false;
-      return CheckSYCLType(FPTy->getReturnType(), Loc, Visited);
-    } else if (const auto *FTy = dyn_cast<FunctionType>(Ty)) {
-      return CheckSYCLType(FTy->getReturnType(), Loc, Visited);
-    }
-    return true;
-  }
-
   Sema &SemaRef;
 };
 

clang/test/SemaSYCL/sycl-restrict.cpp

@@ -36,7 +36,16 @@ void no_restriction(int p) {
   int index[p + 2];
 }
 void restriction(int p) {
-  int index[p + 2]; // expected-error {{variable length arrays are not supported for the current target}}
+  // This particular violation is nested under two kernels with intermediate function calls.
+  // e.g. main -> 1stkernel -> usage -> 2ndkernel -> isa_B -> restriction -> !!
+  // Because the error is in two different kernels, we are given helpful notes for the origination of the error, twice.
+  // expected-note@#call_usage {{called by 'operator()'}}
+  // expected-note@#call_kernelFunc {{called by 'kernel_single_task<fake_kernel, (lambda at}}
+  // expected-note@#call_isa_B 2{{called by 'operator()'}}
+  // expected-note@#call_rtti_kernel {{called by 'usage'}}
+  // expected-note@#rtti_kernel 2{{called by 'kernel1<kernel_name, (lambda at }}
+  // expected-note@#call_vla 2{{called by 'isa_B'}}
+  int index[p + 2]; // expected-error 2{{variable length arrays are not supported for the current target}}
 }
 } // namespace Check_VLA_Restriction
 
@@ -67,8 +76,8 @@ bool isa_B(A *a) {
   if (f1 == f2) // expected-note 2{{called by 'isa_B'}}
     return false;
 
-  Check_VLA_Restriction::restriction(7);
-  int *ip = new int; // expected-error 2{{SYCL kernel cannot allocate storage}}
+  Check_VLA_Restriction::restriction(7); //#call_vla
+  int *ip = new int;                     // expected-error 2{{SYCL kernel cannot allocate storage}}
   int i;
   int *p3 = new (&i) int;                                    // no error on placement new
   OverloadedNewDelete *x = new (struct OverloadedNewDelete); // expected-note 2{{called by 'isa_B'}}
@@ -79,7 +88,7 @@ bool isa_B(A *a) {
 
 template <typename N, typename L>
 __attribute__((sycl_kernel)) void kernel1(L l) {
-  l(); // expected-note 6{{called by 'kernel1<kernel_name, (lambda at }}
+  l(); //#rtti_kernel  // expected-note 6{{called by 'kernel1<kernel_name, (lambda at }}
 }
 } // namespace Check_RTTI_Restriction
 
@@ -189,9 +198,9 @@ void usage(myFuncDef functionPtr) {
     // expected-error@+1 {{SYCL kernel cannot use a non-const global variable}}
     b.f(); // expected-error {{SYCL kernel cannot call a virtual function}}
 
-  Check_RTTI_Restriction::kernel1<class kernel_name>([]() { // expected-note 3{{called by 'usage'}}
+  Check_RTTI_Restriction::kernel1<class kernel_name>([]() { //#call_rtti_kernel // expected-note 3{{called by 'usage'}}
     Check_RTTI_Restriction::A *a;
-    Check_RTTI_Restriction::isa_B(a); // expected-note 6{{called by 'operator()'}}
+    Check_RTTI_Restriction::isa_B(a); //#call_isa_B  // expected-note 6{{called by 'operator()'}}
   });
 
   // ======= Float128 Not Allowed in Kernel ==========
@@ -323,7 +332,7 @@ int use2(a_type ab, a_type *abp) {
 
 template <typename name, typename Func>
 __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
-  kernelFunc(); // expected-note 7{{called by 'kernel_single_task<fake_kernel, (lambda at}}
+  kernelFunc(); //#call_kernelFunc // expected-note 7{{called by 'kernel_single_task<fake_kernel, (lambda at}}
 }
 
 int main() {
@@ -340,7 +349,7 @@ int main() {
   auto notACrime = &commitInfraction;
 
   kernel_single_task<class fake_kernel>([=]() {
-    usage(&addInt); // expected-note 5{{called by 'operator()'}}
+    usage(&addInt); //#call_usage // expected-note 5{{called by 'operator()'}}
     a_type *p;
     use2(ab, p); // expected-note 2{{called by 'operator()'}}
   });