[SYCL] [FPGA] Add mutual diagnostic for num_simd_work_items attribute in conjunction with the reqd_work_group_size attribute

clang/include/clang/Basic/Attr.td

@@ -2845,6 +2845,11 @@ def ReqdWorkGroupSize : InheritableAttr {
     ArrayRef<const Expr *> dimensions() const {
       return {getXDim(), getYDim(), getZDim()};
     }
+    bool isDependent() const {
+      return (getXDim() && getXDim()->isValueDependent()) ||
+             (getYDim() && getYDim()->isValueDependent()) ||
+             (getZDim() && getZDim()->isValueDependent());
+    }
     Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
       return getXDim()->getIntegerConstantExpr(Ctx);
     }

clang/include/clang/Basic/AttrDocs.td

@@ -2418,6 +2418,26 @@ device kernel, the attribute is not ignored and it is propagated to the kernel.
     [[intel::num_simd_work_items(N)]] void operator()() const {}
   };
 
+If the`` intel::reqd_work_group_size`` or ``cl::reqd_work_group_size``
+attribute is specified on a declaration along with a
+intel::num_simd_work_items attribute, the work group size attribute
+arguments must all be evenly divisible by the argument specified in
+the ``intel::num_simd_work_items`` attribute.
+
+.. code-block:: c++
+
+  struct func {
+    [[intel::num_simd_work_items(4)]]
+    [[intel::reqd_work_group_size(64, 64, 64)]]
+    void operator()() const {}
+  };
+
+  struct bar {
+    [[intel::reqd_work_group_size(64, 64, 64)]]
+    [[intel::num_simd_work_items(4)]]
+    void operator()() const {}
+  };
+
   }];
 }
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -11212,6 +11212,9 @@ def err_sycl_invalid_accessor_property_list_template_param : Error<
   "%select{accessor_property_list|accessor_property_list pack argument|buffer_location}0 "
   "template parameter must be a "
   "%select{parameter pack|type|non-negative integer}1">;
+def err_sycl_num_kernel_wrong_reqd_wg_size : Error<
+  "%0 attribute must evenly divide the work-group size for the %1 attribute">;
+
 def warn_sycl_pass_by_value_deprecated 
     : Warning<"Passing kernel functions by value is deprecated in SYCL 2020">,
       InGroup<Sycl2020Compat>, ShowInSystemHeader;

clang/include/clang/Sema/Sema.h

@@ -13071,23 +13071,25 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
       return;
     E = ICE.get();
     int32_t ArgInt = ArgVal.getSExtValue();
-    if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems ||
-        CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
+    if (CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
       if (ArgInt <= 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI.getAttrName() << /*positive*/ 0;
+            << CI << /*positive*/ 0;
         return;
       }
     }
-    if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
+    if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim ||
+        CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems) {
       if (ArgInt < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI.getAttrName() << /*non-negative*/ 1;
+            << CI << /*non-negative*/ 1;
         return;
       }
+    }
+    if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
       if (ArgInt > 3) {
         Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
-            << CI.getAttrName() << 0 << 3 << E->getSourceRange();
+            << CI << 0 << 3 << E->getSourceRange();
         return;
       }
     }
@@ -13096,8 +13098,8 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
   D->addAttr(::new (Context) AttrType(Context, CI, E));
 }
 
-static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
-                                      Expr *E) {
+static Expr *checkWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
+                                   Expr *E) {
   assert(E && "Attribute must have an argument.");
 
   if (!E->isInstantiationDependent()) {
@@ -13142,9 +13144,9 @@ void Sema::addIntelSYCLTripleArgFunctionAttr(Decl *D,
       !ZDimExpr->isValueDependent()) {
 
     // Save ConstantExpr in semantic attribute
-    XDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, XDimExpr);
-    YDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, YDimExpr);
-    ZDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, ZDimExpr);
+    XDimExpr = checkWorkSizeAttrExpr(*this, CI, XDimExpr);
+    YDimExpr = checkWorkSizeAttrExpr(*this, CI, YDimExpr);
+    ZDimExpr = checkWorkSizeAttrExpr(*this, CI, ZDimExpr);
 
     if (!XDimExpr || !YDimExpr || !ZDimExpr)
       return;
@@ -13229,8 +13231,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
 
     if (!ArgVal) {
       Diag(E->getExprLoc(), diag::err_attribute_argument_type)
-          << A.getAttrName() << AANT_ArgumentIntegerConstant
-          << E->getSourceRange();
+          << A << AANT_ArgumentIntegerConstant << E->getSourceRange();
       return nullptr;
     }
 
@@ -13240,7 +13241,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
         A.getParsedKind() == ParsedAttr::AT_SYCLIntelFPGALoopCoalesce) {
       if (Val <= 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << A.getAttrName() << /* positive */ 0;
+            << A << /* positive */ 0;
         return nullptr;
       }
     } else if (A.getParsedKind() ==
@@ -13251,7 +13252,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
                    ParsedAttr::AT_SYCLIntelFPGASpeculatedIterations) {
       if (Val < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << A.getAttrName() << /* non-negative */ 1;
+            << A << /* non-negative */ 1;
         return nullptr;
       }
     } else {

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3095,6 +3095,21 @@ static bool checkWorkGroupSizeValues(Sema &S, Decl *D, const ParsedAttr &AL) {
   return Result;
 }
 
+static Expr *checkWorkSizeAttrExpr(Sema &S, const ParsedAttr &CI, Expr *E) {
+  assert(E && "Attribute must have an argument.");
+
+  if (!E->isInstantiationDependent()) {
+    llvm::APSInt ArgVal;
+    ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
+
+    if (ICE.isInvalid())
+      return nullptr;
+
+    E = ICE.get();
+  }
+  return E;
+}
+
 // Handles reqd_work_group_size and max_work_group_size.
 template <typename WorkGroupAttr>
 static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
@@ -3130,25 +3145,57 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
       return;
   }
 
-  if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
-    ASTContext &Ctx = S.getASTContext();
+  ASTContext &Ctx = S.getASTContext();
+
+  if (!XDimExpr->isValueDependent() && !YDimExpr->isValueDependent() &&
+      !ZDimExpr->isValueDependent()) {
     Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
     Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
     Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
-    Optional<llvm::APSInt> ExistingXDimVal = ExistingAttr->getXDimVal(Ctx);
-    Optional<llvm::APSInt> ExistingYDimVal = ExistingAttr->getYDimVal(Ctx);
-    Optional<llvm::APSInt> ExistingZDimVal = ExistingAttr->getZDimVal(Ctx);
 
-    // Compare attribute arguments value and warn for a mismatch.
-    if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
-        ExistingZDimVal != ZDimVal) {
-      S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
-      S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
+    XDimExpr = checkWorkSizeAttrExpr(S, AL, XDimExpr);
+    YDimExpr = checkWorkSizeAttrExpr(S, AL, YDimExpr);
+    ZDimExpr = checkWorkSizeAttrExpr(S, AL, ZDimExpr);
+
+    if (!XDimExpr || !YDimExpr || !ZDimExpr)
+      return;
+
+    // Skip SEMA if we're in a template, this will be diagnosed later.
+    if (S.getCurLexicalContext()->isDependentContext())
+      return;
+
+    if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
+      if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
+        int64_t NumSimdWorkItems =
+            A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
+
+        if (!(XDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
+              YDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
+              ZDimVal->getZExtValue() % NumSimdWorkItems == 0)) {
+          S.Diag(A->getLocation(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
+              << A << AL;
+          S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
+          return;
+        }
+      }
     }
-  }
 
-  if (!checkWorkGroupSizeValues(S, D, AL))
-    return;
+    if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
+      Optional<llvm::APSInt> ExistingXDimVal = ExistingAttr->getXDimVal(Ctx);
+      Optional<llvm::APSInt> ExistingYDimVal = ExistingAttr->getYDimVal(Ctx);
+      Optional<llvm::APSInt> ExistingZDimVal = ExistingAttr->getZDimVal(Ctx);
+
+      // Compare attribute arguments value and warn for a mismatch.
+      if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
+          ExistingZDimVal != ZDimVal) {
+        S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
+        S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
+      }
+    }
+
+    if (!checkWorkGroupSizeValues(S, D, AL))
+      return;
+  }
 
   S.addIntelSYCLTripleArgFunctionAttr<WorkGroupAttr>(D, AL, XDimExpr, YDimExpr,
                                                      ZDimExpr);
@@ -3197,18 +3244,51 @@ static void handleSubGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
 }
 
 // Handles num_simd_work_items.
-static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (D->isInvalidDecl())
     return;
 
-  Expr *E = A.getArgAsExpr(0);
+  Expr *E = AL.getArgAsExpr(0);
 
   if (D->getAttr<SYCLIntelNumSimdWorkItemsAttr>())
-    S.Diag(A.getLoc(), diag::warn_duplicate_attribute) << A;
+    S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
 
-  S.CheckDeprecatedSYCLAttributeSpelling(A);
+  S.CheckDeprecatedSYCLAttributeSpelling(AL);
+
+  if (!E->isValueDependent() || !E->isInstantiationDependent()) {
+    ASTContext &Ctx = S.getASTContext();
+    llvm::APSInt ArgVal;
+    ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
 
-  S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNumSimdWorkItemsAttr>(D, A, E);
+    if (ICE.isInvalid())
+      return;
+
+    E = ICE.get();
+    int64_t NumSimdWorkItems = ArgVal.getSExtValue();
+
+    if (NumSimdWorkItems == 0) {
+      S.Diag(E->getExprLoc(), diag::err_attribute_argument_is_zero)
+          << AL << E->getSourceRange();
+      return;
+    }
+
+    if (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
+      Optional<llvm::APSInt> XDimVal = A->getXDimVal(Ctx);
+      Optional<llvm::APSInt> YDimVal = A->getYDimVal(Ctx);
+      Optional<llvm::APSInt> ZDimVal = A->getZDimVal(Ctx);
+
+      if (!(XDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
+            YDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
+            ZDimVal->getZExtValue() % NumSimdWorkItems == 0)) {
+        S.Diag(AL.getLoc(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
+            << AL << A;
+        S.Diag(A->getLocation(), diag::note_conflicting_attribute);
+        return;
+      }
+    }
+  }
+
+  S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNumSimdWorkItemsAttr>(D, AL, E);
 }
 
 // Handles use_stall_enable_clusters
@@ -5966,14 +6046,13 @@ void Sema::addSYCLIntelPipeIOAttr(Decl *D, const AttributeCommonInfo &Attr,
     Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(getASTContext());
     if (!ArgVal) {
       Diag(E->getExprLoc(), diag::err_attribute_argument_type)
-          << Attr.getAttrName() << AANT_ArgumentIntegerConstant
-          << E->getSourceRange();
+          << Attr << AANT_ArgumentIntegerConstant << E->getSourceRange();
       return;
     }
     int32_t ArgInt = ArgVal->getSExtValue();
     if (ArgInt < 0) {
       Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-          << Attr.getAttrName() << /*non-negative*/ 1;
+          << Attr << /*non-negative*/ 1;
       return;
     }
   }