Move diagnostics + don't rely on params order

Signed-off-by: Dmitry Sidorov <[email protected]>

Author: MrSidims

clang/include/clang/Basic/AttrDocs.td

@@ -2007,7 +2007,9 @@ the number of kernel parameters. The metadata contains an integer that is set
 according to the values passed through the ``accessor`` property
 ``buffer_location``. These values are mapped to the actual locations of the
 global buffers (such as DDR, QDR, etc) and applied to pointer kernel parameters.
-The default value is -1.
+Number of metadata arguments is the same as a number of kernel parameters, so
+any parameter that isn't an ``accessor`` with ``buffer_location`` property is
+annotated by '-1' in the metadata node.
   }];
 }
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10976,9 +10976,11 @@ def err_sycl_attibute_cannot_be_applied_here
             "static function or function in an anonymous namespace">;
 def err_sycl_compiletime_property_duplication : Error<
   "Can't apply %0 property twice to the same accessor">;
-def err_sycl_invalid_property_template_param : Error<
+def err_sycl_invalid_property_list_template_param : Error<
   "%0 template parameter must be a "
-  "%select{parameter pack|type|non-negative integer|property_list}1">;
+  "%select{parameter pack|type|non-negative integer}1">;
+def err_sycl_invalid_accessor_property_template_param : Error<
+  "Fifth template parameter of the accessor must be of a property_list type">;
 def warn_sycl_attibute_function_raw_ptr
     : Warning<"SYCL 1.2.1 specification does not allow %0 attribute applied "
               "to a function with a raw pointer "

clang/lib/Sema/SemaSYCL.cpp

@@ -1084,6 +1084,51 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler {
     return false;
   }
 
+  void checkPropertyListType(TemplateArgument PropList, SourceLocation Loc) {
+    if (PropList.getKind() != TemplateArgument::ArgKind::Type) {
+      SemaRef.Diag(Loc,
+                   diag::err_sycl_invalid_accessor_property_template_param);
+      return;
+    }
+    QualType PropListTy = PropList.getAsType();
+    if (!Util::isPropertyListType(PropListTy)) {
+      SemaRef.Diag(Loc,
+                   diag::err_sycl_invalid_accessor_property_template_param);
+      return;
+    }
+    const auto *PropListDecl =
+        cast<ClassTemplateSpecializationDecl>(PropListTy->getAsRecordDecl());
+    const auto TemplArg = PropListDecl->getTemplateArgs()[0];
+    if (TemplArg.getKind() != TemplateArgument::ArgKind::Pack) {
+      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_list_template_param)
+          << "property_list" << /*parameter pack*/ 0;
+      return;
+    }
+    for (TemplateArgument::pack_iterator Prop = TemplArg.pack_begin();
+         Prop != TemplArg.pack_end(); ++Prop) {
+      QualType PropTy = Prop->getAsType();
+      if (Util::isSyclBufferLocationType(PropTy))
+        checkBufferLocationType(PropTy, Loc);
+    }
+  }
+
+  void checkBufferLocationType(QualType PropTy, SourceLocation Loc) {
+    const auto *PropDecl =
+        cast<ClassTemplateSpecializationDecl>(PropTy->getAsRecordDecl());
+    const auto BufferLoc = PropDecl->getTemplateArgs()[0];
+    if (BufferLoc.getKind() != TemplateArgument::ArgKind::Integral) {
+      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_list_template_param)
+          << "buffer_location" << /*non-negative integer*/ 2;
+      return;
+    }
+    int LocationID = static_cast<int>(BufferLoc.getAsIntegral().getExtValue());
+    if (LocationID < 0) {
+      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_list_template_param)
+          << "buffer_location" << /*non-negative integer*/ 2;
+      return;
+    }
+  }
+
   void checkAccessorType(QualType Ty, SourceRange Loc) {
     assert(Util::isSyclAccessorType(Ty) &&
            "Should only be called on SYCL accessor types.");
@@ -1095,6 +1140,8 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler {
       TemplateArgument TA = TAL.get(0);
       const QualType TemplateArgTy = TA.getAsType();
 
+      if (TAL.size() > 5)
+        checkPropertyListType(TAL.get(5), Loc.getBegin());
       llvm::DenseSet<QualType> Visited;
       checkSYCLType(SemaRef, TemplateArgTy, Loc, Visited);
     }
@@ -1188,26 +1235,10 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
     if (AccTy->getTemplateArgs().size() < 6)
       return;
     const auto PropList = cast<TemplateArgument>(AccTy->getTemplateArgs()[5]);
-    if (PropList.getKind() != TemplateArgument::ArgKind::Type) {
-      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_template_param)
-          << "accessor's 5th" << /*type*/ 1;
-      return;
-    }
     QualType PropListTy = PropList.getAsType();
-    if (!Util::isPropertyListType(PropListTy)) {
-      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_template_param)
-          << "accessor's 5th" << /*property_list*/ 3;
-      return;
-    }
-
     const auto *PropListDecl =
         cast<ClassTemplateSpecializationDecl>(PropListTy->getAsRecordDecl());
     const auto TemplArg = PropListDecl->getTemplateArgs()[0];
-    if (TemplArg.getKind() != TemplateArgument::ArgKind::Pack) {
-      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_template_param)
-          << "property_list" << /*parameter pack*/ 0;
-      return;
-    }
     // Move through TemplateArgs list of a property list and search for
     // properties. If found - apply the appropriate attribute to ParmVarDecl.
     for (TemplateArgument::pack_iterator Prop = TemplArg.pack_begin();
@@ -1233,17 +1264,7 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
     const auto *PropDecl =
         cast<ClassTemplateSpecializationDecl>(PropTy->getAsRecordDecl());
     const auto BufferLoc = PropDecl->getTemplateArgs()[0];
-    if (BufferLoc.getKind() != TemplateArgument::ArgKind::Integral) {
-      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_template_param)
-          << "buffer_location" << /*non-negative integer*/ 2;
-      return;
-    }
     int LocationID = static_cast<int>(BufferLoc.getAsIntegral().getExtValue());
-    if (LocationID < 0) {
-      SemaRef.Diag(Loc, diag::err_sycl_invalid_property_template_param)
-          << "buffer_location" << /*non-negative integer*/ 2;
-      return;
-    }
     Param->addAttr(
         SYCLIntelBufferLocationAttr::CreateImplicit(Ctx, LocationID));
   }
@@ -1262,15 +1283,12 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
     // Don't do -1 here because we count on this to be the first parameter added
     // (if any).
     size_t ParamIndex = Params.size();
-    ParmVarDecl **ParamIt = InitMethod->parameters().begin();
-    if (*ParamIt) {
-      addParam(FD, (*ParamIt)->getType().getCanonicalType());
-      if (isAccessorType)
+    for (const ParmVarDecl *Param : InitMethod->parameters()) {
+      QualType ParamTy = Param->getType();
+      addParam(FD, ParamTy.getCanonicalType());
+      if (ParamTy.getTypePtr()->isPointerType() && isAccessorType)
         handleAccessorPropertyList(Params.back(), RecordDecl,
                                    FD->getLocation());
-      ++ParamIt;
-      for (; ParamIt != InitMethod->parameters().end(); ++ParamIt)
-        addParam(FD, (*ParamIt)->getType().getCanonicalType());
     }
     LastParamIndex = ParamIndex;
     return true;
@@ -1341,13 +1359,11 @@ class SyclKernelDeclCreator : public SyclKernelFieldHandler {
     // Don't do -1 here because we count on this to be the first parameter added
     // (if any).
     size_t ParamIndex = Params.size();
-    ParmVarDecl **ParamIt = InitMethod->parameters().begin();
-    if (*ParamIt) {
-      addParam(BS, (*ParamIt)->getType().getCanonicalType());
-      handleAccessorPropertyList(Params.back(), RecordDecl, BS.getBeginLoc());
-      ++ParamIt;
-      for (; ParamIt != InitMethod->parameters().end(); ++ParamIt)
-        addParam(BS, (*ParamIt)->getType().getCanonicalType());
+    for (const ParmVarDecl *Param : InitMethod->parameters()) {
+      QualType ParamTy = Param->getType();
+      addParam(BS, ParamTy.getCanonicalType());
+      if (ParamTy.getTypePtr()->isPointerType())
+        handleAccessorPropertyList(Params.back(), RecordDecl, BS.getBeginLoc());
     }
     LastParamIndex = ParamIndex;
     return true;

clang/test/SemaSYCL/buffer_location.cpp

@@ -82,7 +82,7 @@ int main() {
 #else
         //expected-error@+1{{buffer_location template parameter must be a non-negative integer}}
         accessorD.use();
-        //expected-error@+1{{accessor's 5th template parameter must be a property_list}}
+        //expected-error@+1{{Fifth template parameter of the accessor must be of a property_list type}}
         accessorE.use();
         //expected-error@+1{{Can't apply buffer_location property twice to the same accessor}}
         accessorF.use();