From b89e0400b4d8d5626b5db78ad73b8c31aebbde3f Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 5 Feb 2021 10:21:33 -0800
Subject: [PATCH 01/12] [SYCL] [FPGA] Add mutual diagnostic for
 num_simd_work_items attribute in conjunction with the reqd_work_group_size
 attribute
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a follow up comments on https://github.com/intel/llvm/pull/2906#discussion_r548972725

There is a rule on FPGA document that the num_simd_work_items attribute we specify must evenly divide
the work-group size we specify for the reqd_work_group_size attribute, was missed during initial
implemention of the num_simd_work_items attribute.

OpenCL spelling for num_simd_work_items attribute supports this rule.

This patch

   1. addes support for mutual diagnostic in Sema for num_simd_work_items attribute interacting with reqd_work_group_size attribute.
   2. addes tests
   3. updates documenation about the num_simd_work_items attribute with the note and exmaples.

Current implementation of SYCL attribute “num_simd_work_items” and “reqd_work_group_size” does not support this.

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/lib/Sema/SemaDeclAttr.cpp               |  52 ++++++-
 .../SemaSYCL/num_simd_work_items_device.cpp   | 130 +++++++++++++++++-
 2 files changed, 173 insertions(+), 9 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f793e55793d6e..2158950b03523 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3104,8 +3104,28 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
       return;
   }
 
+  ASTContext &Ctx = S.getASTContext();
+
+  if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
+    if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
+      Optional<llvm::APSInt> NumSimdWorkItems =
+          A->getValue()->getIntegerConstantExpr(Ctx);
+      Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
+      Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
+      Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
+
+      if (!(XDimVal->getExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
+            YDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
+            ZDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0)) {
+        S.Diag(A->getLocation(), diag::err_conflicting_sycl_function_attributes)
+            << A << AL;
+        S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
+        return;
+      }
+    }
+  }
+
   if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
-    ASTContext &Ctx = S.getASTContext();
     Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
     Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
     Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
@@ -3171,18 +3191,38 @@ 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 (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
+    ASTContext &Ctx = S.getASTContext();
+
+    if (!E->isValueDependent()) {
+      Optional<llvm::APSInt> NumSimdWorkItems = E->getIntegerConstantExpr(Ctx);
+      Optional<llvm::APSInt> XDimVal = A->getXDimVal(Ctx);
+      Optional<llvm::APSInt> YDimVal = A->getYDimVal(Ctx);
+      Optional<llvm::APSInt> ZDimVal = A->getZDimVal(Ctx);
+
+      if (!(XDimVal->getExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
+            YDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
+            ZDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0)) {
+        S.Diag(AL.getLoc(), diag::err_conflicting_sycl_function_attributes)
+            << AL << A->getSpelling();
+        S.Diag(A->getLocation(), diag::note_conflicting_attribute);
+        return;
+      }
+    }
+  }
 
-  S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNumSimdWorkItemsAttr>(D, A, E);
+  S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNumSimdWorkItemsAttr>(D, AL, E);
 }
 
 // Handles use_stall_enable_clusters
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 7531feb7add27..93539480c74c8 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -33,6 +33,68 @@ struct FuncObj {
   [[intel::num_simd_work_items(42)]] void operator()() const {}
 };
 
+#ifdef TRIGGER_ERROR
+struct TRIFuncObjBad1 {
+   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
+   [[intel::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
+   void operator()() const {}
+};
+
+struct TRIFuncObjBad2 {
+   [[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
+   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+   void operator()() const {}
+};
+
+struct TRIFuncObjBad3 {
+   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
+   [[cl::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
+   void operator()() const {}
+};
+
+struct TRIFuncObjBad4 {
+   [[cl::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
+   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+   void operator()() const {}
+};
+#endif // TRIGGER_ERROR
+
+struct TRIFuncObjGood1 {
+   [[intel::num_simd_work_items(4)]] //OK
+   [[intel::reqd_work_group_size(64, 64, 64)]]
+   void operator()() const {}
+};
+
+struct TRIFuncObjGood2 {
+   [[intel::reqd_work_group_size(64, 64, 64)]]
+   [[intel::num_simd_work_items(4)]] //OK
+   void operator()() const {}
+};
+
+struct TRIFuncObjGood3 {
+   [[intel::num_simd_work_items(4)]] //OK
+   [[cl::reqd_work_group_size(64, 64, 64)]]
+   void operator()() const {}
+};
+
+struct TRIFuncObjGood4 {
+   [[cl::reqd_work_group_size(64, 64, 64)]]
+   [[intel::num_simd_work_items(4)]] //OK
+   void operator()() const {}
+};
+
+struct TRIFuncObjGood5 {
+   [[intel::num_simd_work_items(3)]] //OK
+   [[intel::max_work_group_size(5, 5, 5)]]
+   void operator()() const {}
+};
+
+struct TRIFuncObjGood6 {
+   [[intel::max_work_group_size(5, 5, 5)]]
+   [[intel::num_simd_work_items(3)]] //OK
+   void operator()() const {}
+};
+
 int main() {
   q.submit([&](handler &h) {
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel1
@@ -54,16 +116,78 @@ int main() {
     h.single_task<class test_kernel3>(
         []() { func_do_not_ignore(); });
 
+    h.single_task<class test_kernel4>(TRIFuncObjGood1());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel4
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel5>(TRIFuncObjGood2());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel5
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel6>(TRIFuncObjGood3());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel6
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel7>(TRIFuncObjGood4());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel7
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel8>(TRIFuncObjGood5());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel8
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
+    // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+
+    h.single_task<class test_kernel9>(TRIFuncObjGood6());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel9
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
+    // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+
 #ifdef TRIGGER_ERROR
     [[intel::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}}
 
-    h.single_task<class test_kernel4>(
+    h.single_task<class test_kernel10>(
         []() [[intel::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
 
-    h.single_task<class test_kernel5>(
+    h.single_task<class test_kernel11>(
         []() [[intel::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
 
-    h.single_task<class test_kernel6>(
+    h.single_task<class test_kernel12>(TRIFuncObjBad1());
+
+    h.single_task<class test_kernel13>(TRIFuncObjBad2());
+
+    h.single_task<class test_kernel14>(TRIFuncObjBad3());
+
+    h.single_task<class test_kernel15>(TRIFuncObjBad4());
+
+    h.single_task<class test_kernel16>(
         []() [[intel::num_simd_work_items(1), intel::num_simd_work_items(2)]]{}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}}
 #endif // TRIGGER_ERROR
   });

From 4d5d9e2611a8d2a2272751e48cdbc992b9b48465 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 5 Feb 2021 11:19:33 -0800
Subject: [PATCH 02/12] Fix clang format issues

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Basic/AttrDocs.td         | 12 ++++
 .../SemaSYCL/num_simd_work_items_device.cpp   | 64 ++++++++++---------
 2 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index e51296bbab9a9..8d31f33fad829 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2418,6 +2418,18 @@ device kernel, the attribute is not ignored and it is propagated to the kernel.
     [[intel::num_simd_work_items(N)]] void operator()() const {}
   };
 
+The ``intel::num_simd_work_items`` attribute we specify must evenly divide
+the work-group size we specify for the ``intel::reqd_work_group_size`` or
+``cl::reqd_work_group_size`` attribute.
+
+.. code-block:: c++
+
+  struct TRIFuncObjGood1 {
+    [[intel::num_simd_work_items(4)]]
+    [[intel::reqd_work_group_size(64, 64, 64)]]
+    void operator()() const {}
+  };
+
   }];
 }
 
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 93539480c74c8..9cce02f6435dc 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -35,64 +35,68 @@ struct FuncObj {
 
 #ifdef TRIGGER_ERROR
 struct TRIFuncObjBad1 {
-   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
-   [[intel::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
-   void operator()() const {}
+  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
+  [[intel::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
+  void
+  operator()() const {}
 };
 
 struct TRIFuncObjBad2 {
-   [[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
-   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
-   void operator()() const {}
+  [[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
+  [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  void
+  operator()() const {}
 };
 
 struct TRIFuncObjBad3 {
-   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
-   [[cl::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
-   void operator()() const {}
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  [[cl::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
+  void
+  operator()() const {}
 };
 
 struct TRIFuncObjBad4 {
-   [[cl::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
-   [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
-   void operator()() const {}
+  [[cl::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  void
+  operator()() const {}
 };
 #endif // TRIGGER_ERROR
 
 struct TRIFuncObjGood1 {
-   [[intel::num_simd_work_items(4)]] //OK
-   [[intel::reqd_work_group_size(64, 64, 64)]]
-   void operator()() const {}
+  [[intel::num_simd_work_items(4)]]
+  [[intel::reqd_work_group_size(64, 64, 64)]] void
+  operator()() const {}
 };
 
 struct TRIFuncObjGood2 {
-   [[intel::reqd_work_group_size(64, 64, 64)]]
-   [[intel::num_simd_work_items(4)]] //OK
-   void operator()() const {}
+  [[intel::reqd_work_group_size(64, 64, 64)]]
+  [[intel::num_simd_work_items(4)]] void
+  operator()() const {}
 };
 
 struct TRIFuncObjGood3 {
-   [[intel::num_simd_work_items(4)]] //OK
-   [[cl::reqd_work_group_size(64, 64, 64)]]
-   void operator()() const {}
+  [[intel::num_simd_work_items(4)]]
+  [[cl::reqd_work_group_size(64, 64, 64)]] void
+  operator()() const {}
 };
 
 struct TRIFuncObjGood4 {
-   [[cl::reqd_work_group_size(64, 64, 64)]]
-   [[intel::num_simd_work_items(4)]] //OK
-   void operator()() const {}
+  [[cl::reqd_work_group_size(64, 64, 64)]]
+  [[intel::num_simd_work_items(4)]] void
+  operator()() const {}
 };
 
 struct TRIFuncObjGood5 {
-   [[intel::num_simd_work_items(3)]] //OK
-   [[intel::max_work_group_size(5, 5, 5)]]
-   void operator()() const {}
+  [[intel::num_simd_work_items(3)]]
+  [[intel::max_work_group_size(5, 5, 5)]] void
+  operator()() const {}
 };
 
 struct TRIFuncObjGood6 {
-   [[intel::max_work_group_size(5, 5, 5)]]
-   [[intel::num_simd_work_items(3)]] //OK
-   void operator()() const {}
+  [[intel::max_work_group_size(5, 5, 5)]]
+  [[intel::num_simd_work_items(3)]] void
+  operator()() const {}
 };
 
 int main() {

From 5fdb56b56aacfa38e288502d2041421fd5b3da15 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 5 Feb 2021 11:23:59 -0800
Subject: [PATCH 03/12] Fix clang format issues

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/test/SemaSYCL/num_simd_work_items_device.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 9cce02f6435dc..63ef008aff6b5 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -43,7 +43,7 @@ struct TRIFuncObjBad1 {
 
 struct TRIFuncObjBad2 {
   [[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
-  [[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
   void
   operator()() const {}
 };

From 86d4a31c4ede07e1a1f1b380e841f3be00414a4c Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 5 Feb 2021 11:27:13 -0800
Subject: [PATCH 04/12] update doc

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Basic/AttrDocs.td | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 8d31f33fad829..d01ee74713302 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2424,12 +2424,18 @@ the work-group size we specify for the ``intel::reqd_work_group_size`` or
 
 .. code-block:: c++
 
-  struct TRIFuncObjGood1 {
+  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 {}
+  };
+
   }];
 }
 

From b3b88ec50415eea7acf4ab6d1cb925c83c43c756 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Sun, 7 Feb 2021 21:21:59 -0800
Subject: [PATCH 05/12] Address review comments

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 .../clang/Basic/DiagnosticSemaKinds.td        |   3 +
 clang/lib/Sema/SemaDeclAttr.cpp               | 101 +++++++++++-------
 .../SemaSYCL/num_simd_work_items_device.cpp   |  91 ++++++++++++++--
 3 files changed, 148 insertions(+), 47 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d02538ef6f799..39c14167723ae 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2158950b03523..3f42aac5c0c61 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3106,38 +3106,56 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   ASTContext &Ctx = S.getASTContext();
 
-  if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
-    if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
-      Optional<llvm::APSInt> NumSimdWorkItems =
-          A->getValue()->getIntegerConstantExpr(Ctx);
-      Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
-      Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
-      Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
-
-      if (!(XDimVal->getExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
-            YDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
-            ZDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0)) {
-        S.Diag(A->getLocation(), diag::err_conflicting_sycl_function_attributes)
-            << A << AL;
-        S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
-        return;
-      }
-    }
-  }
-
-  if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
+  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);
+    if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
+      if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
+        int64_t NumSimdWorkItems =
+          A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
+
+	assert(NumSimdWorkItems && XDimVal &&
+               "Argument should be an integer constant expression");
+        assert(NumSimdWorkItems && YDimVal &&
+               "Argument should be an integer constant expression");
+        assert(NumSimdWorkItems && ZDimVal &&
+               "Argument should be an integer constant expression");
+
+        if (NumSimdWorkItems == 0)
+          return;
+
+        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 (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);
+
+      assert(XDimVal && ExistingXDimVal &&
+             "Argument should be an integer constant expression");
+      assert(YDimVal && ExistingYDimVal &&
+             "Argument should be an integer constant expression");
+      assert(ZDimVal && ExistingZDimVal &&
+             "Argument should be an integer constant expression");
+
+      // 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);
+      }
     }
   }
 
@@ -3202,20 +3220,31 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   S.CheckDeprecatedSYCLAttributeSpelling(AL);
 
-  if (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
+  if (!E->isValueDependent()) {
     ASTContext &Ctx = S.getASTContext();
+    Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(Ctx);
 
-    if (!E->isValueDependent()) {
-      Optional<llvm::APSInt> NumSimdWorkItems = E->getIntegerConstantExpr(Ctx);
+    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->getExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
-            YDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0 ||
-            ZDimVal->getZExtValue() % NumSimdWorkItems->getExtValue() == 0)) {
-        S.Diag(AL.getLoc(), diag::err_conflicting_sycl_function_attributes)
-            << AL << A->getSpelling();
+      assert(ArgVal && XDimVal &&
+             "Argument should be an integer constant expression");
+      assert(ArgVal && YDimVal &&
+             "Argument should be an integer constant expression");
+      assert(ArgVal && ZDimVal &&
+             "Argument should be an integer constant expression");
+
+      int64_t NumSimdWorkItems = ArgVal->getSExtValue();
+      if (NumSimdWorkItems == 0)
+        return;
+
+      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;
       }
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 63ef008aff6b5..9527ac41c9b00 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -35,7 +35,7 @@ struct FuncObj {
 
 #ifdef TRIGGER_ERROR
 struct TRIFuncObjBad1 {
-  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute conflicts with ''reqd_work_group_size'' attribute}}
+  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
   [[intel::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
   void
   operator()() const {}
@@ -43,13 +43,13 @@ struct TRIFuncObjBad1 {
 
 struct TRIFuncObjBad2 {
   [[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
-  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  [[intel::num_simd_work_items(3)]]        // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
   void
   operator()() const {}
 };
 
 struct TRIFuncObjBad3 {
-  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
   [[cl::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
   void
   operator()() const {}
@@ -57,7 +57,14 @@ struct TRIFuncObjBad3 {
 
 struct TRIFuncObjBad4 {
   [[cl::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
-  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute conflicts with 'reqd_work_group_size' attribute}}
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
+  void
+  operator()() const {}
+};
+
+struct TRIFuncObjBad5 {
+  [[intel::num_simd_work_items(0)]]        // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+  [[intel::reqd_work_group_size(5, 5, 5)]]
   void
   operator()() const {}
 };
@@ -99,6 +106,30 @@ struct TRIFuncObjGood6 {
   operator()() const {}
 };
 
+struct TRIFuncObjGood7 {
+  [[intel::num_simd_work_items(4)]]
+  [[intel::reqd_work_group_size(64)]] void
+  operator()() const {}
+};
+
+struct TRIFuncObjGood8 {
+  [[intel::reqd_work_group_size(64)]]
+  [[intel::num_simd_work_items(4)]] void
+  operator()() const {}
+};
+
+struct TRIFuncObjGood9 {
+  [[intel::num_simd_work_items(4)]]
+  [[intel::reqd_work_group_size(64, 64)]] void
+  operator()() const {}
+};
+
+struct TRIFuncObjGood10 {
+  [[intel::reqd_work_group_size(64, 64)]]
+  [[intel::num_simd_work_items(4)]] void
+  operator()() const {}
+};
+
 int main() {
   q.submit([&](handler &h) {
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel1
@@ -174,24 +205,62 @@ int main() {
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
 
+    h.single_task<class test_kernel10>(TRIFuncObjGood7());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel10
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel11>(TRIFuncObjGood8());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel11
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+    
+    h.single_task<class test_kernel12>(TRIFuncObjGood9());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel12
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
+    h.single_task<class test_kernel13>(TRIFuncObjGood10());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel13
+    // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
+
 #ifdef TRIGGER_ERROR
     [[intel::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}}
 
-    h.single_task<class test_kernel10>(
+    h.single_task<class test_kernel14>(
         []() [[intel::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
 
-    h.single_task<class test_kernel11>(
+    h.single_task<class test_kernel15>(
         []() [[intel::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
 
-    h.single_task<class test_kernel12>(TRIFuncObjBad1());
+    h.single_task<class test_kernel16>(TRIFuncObjBad1());
+
+    h.single_task<class test_kernel17>(TRIFuncObjBad2());
 
-    h.single_task<class test_kernel13>(TRIFuncObjBad2());
+    h.single_task<class test_kernel18>(TRIFuncObjBad3());
 
-    h.single_task<class test_kernel14>(TRIFuncObjBad3());
+    h.single_task<class test_kernel19>(TRIFuncObjBad4());
 
-    h.single_task<class test_kernel15>(TRIFuncObjBad4());
+    h.single_task<class test_kernel20>(TRIFuncObjBad5());
 
-    h.single_task<class test_kernel16>(
+    h.single_task<class test_kernel21>(
         []() [[intel::num_simd_work_items(1), intel::num_simd_work_items(2)]]{}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}}
 #endif // TRIGGER_ERROR
   });

From 3d3968b16bb6cbace721fe41833b4658680bc86d Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Sun, 7 Feb 2021 21:32:22 -0800
Subject: [PATCH 06/12] Fix calng format issues

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/lib/Sema/SemaDeclAttr.cpp                    | 4 ++--
 clang/test/SemaSYCL/num_simd_work_items_device.cpp | 7 +++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3f42aac5c0c61..2446fb9e4027c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3115,9 +3115,9 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
     if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
       if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
         int64_t NumSimdWorkItems =
-          A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
+            A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
 
-	assert(NumSimdWorkItems && XDimVal &&
+        assert(NumSimdWorkItems && XDimVal &&
                "Argument should be an integer constant expression");
         assert(NumSimdWorkItems && YDimVal &&
                "Argument should be an integer constant expression");
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 9527ac41c9b00..c1635b93521c2 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -63,9 +63,8 @@ struct TRIFuncObjBad4 {
 };
 
 struct TRIFuncObjBad5 {
-  [[intel::num_simd_work_items(0)]]        // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
-  [[intel::reqd_work_group_size(5, 5, 5)]]
-  void
+  [[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+  [[intel::reqd_work_group_size(5, 5, 5)]] void
   operator()() const {}
 };
 #endif // TRIGGER_ERROR
@@ -222,7 +221,7 @@ int main() {
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
-    
+
     h.single_task<class test_kernel12>(TRIFuncObjGood9());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel12
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}

From 664936316f6b29f07b15db31a82a8cf04715b23b Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Sun, 7 Feb 2021 21:58:59 -0800
Subject: [PATCH 07/12] Add new tests

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 .../SemaSYCL/num_simd_work_items_device.cpp   | 38 ++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index c1635b93521c2..bfa600afa7909 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -67,6 +67,34 @@ struct TRIFuncObjBad5 {
   [[intel::reqd_work_group_size(5, 5, 5)]] void
   operator()() const {}
 };
+
+struct TRIFuncObjBad6 {
+  [[intel::num_simd_work_items(3)]]  // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
+  [[intel::reqd_work_group_size(5)]] //expected-note{{conflicting attribute is here}}
+  void
+  operator()() const {}
+};
+
+struct TRIFuncObjBad7 {
+  [[intel::reqd_work_group_size(5)]] // expected-note{{conflicting attribute is here}}
+  [[intel::num_simd_work_items(3)]]  // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
+  void
+  operator()() const {}
+};
+
+struct TRIFuncObjBad8 {
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
+  [[intel::reqd_work_group_size(5, 5)]] //expected-note{{conflicting attribute is here}}
+  void
+  operator()() const {}
+};
+
+struct TRIFuncObjBad9 {
+  [[intel::reqd_work_group_size(5, 5)]] // expected-note{{conflicting attribute is here}}
+  [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
+  void
+  operator()() const {}
+};
 #endif // TRIGGER_ERROR
 
 struct TRIFuncObjGood1 {
@@ -259,7 +287,15 @@ int main() {
 
     h.single_task<class test_kernel20>(TRIFuncObjBad5());
 
-    h.single_task<class test_kernel21>(
+    h.single_task<class test_kernel21>(TRIFuncObjBad6());
+
+    h.single_task<class test_kernel22>(TRIFuncObjBad7());
+
+    h.single_task<class test_kernel13>(TRIFuncObjBad8());
+
+    h.single_task<class test_kernel24>(TRIFuncObjBad9());
+
+    h.single_task<class test_kernel25>(
         []() [[intel::num_simd_work_items(1), intel::num_simd_work_items(2)]]{}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}}
 #endif // TRIGGER_ERROR
   });

From 7f39b8c4bcac35934352016be0d09bd8daea2c54 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 12 Feb 2021 09:41:40 -0800
Subject: [PATCH 08/12] Address review comments

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Basic/Attr.td             |   5 +
 clang/include/clang/Basic/AttrDocs.td         |   8 +-
 clang/include/clang/Sema/Sema.h               |  29 ++--
 clang/lib/Sema/SemaDeclAttr.cpp               |  86 +++++-----
 .../SemaSYCL/num_simd_work_items_device.cpp   | 156 +++++++++++++++++-
 ...cl-device-num_simd_work_items-template.cpp |   6 +-
 6 files changed, 226 insertions(+), 64 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 14d78e539412f..6fc32b559ab0b 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/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);
     }
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 83013434fb253..92c0e195d8ad2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2418,9 +2418,11 @@ device kernel, the attribute is not ignored and it is propagated to the kernel.
     [[intel::num_simd_work_items(N)]] void operator()() const {}
   };
 
-The ``intel::num_simd_work_items`` attribute we specify must evenly divide
-the work-group size we specify for the ``intel::reqd_work_group_size`` or
-``cl::reqd_work_group_size`` attribute.
+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++
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index eea057e8aeda8..45df062873d6b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/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,7 +13098,7 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
   D->addAttr(::new (Context) AttrType(Context, CI, E));
 }
 
-static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
+static Expr *checkWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
                                       Expr *E) {
   assert(E && "Attribute must have an argument.");
 
@@ -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 {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 4868a5cca76b4..e2721ac8cd970 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3095,6 +3095,22 @@ 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) {
@@ -3132,27 +3148,28 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   ASTContext &Ctx = S.getASTContext();
 
-  if (!XDimExpr->isValueDependent() || !YDimExpr->isValueDependent() ||
-      !ZDimExpr->isValueDependent()) {
+  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);
 
+    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();
 
-        assert(NumSimdWorkItems && XDimVal &&
-               "Argument should be an integer constant expression");
-        assert(NumSimdWorkItems && YDimVal &&
-               "Argument should be an integer constant expression");
-        assert(NumSimdWorkItems && ZDimVal &&
-               "Argument should be an integer constant expression");
-
-        if (NumSimdWorkItems == 0)
-          return;
-
         if (!(XDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
               YDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
               ZDimVal->getZExtValue() % NumSimdWorkItems == 0)) {
@@ -3169,13 +3186,6 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
       Optional<llvm::APSInt> ExistingYDimVal = ExistingAttr->getYDimVal(Ctx);
       Optional<llvm::APSInt> ExistingZDimVal = ExistingAttr->getZDimVal(Ctx);
 
-      assert(XDimVal && ExistingXDimVal &&
-             "Argument should be an integer constant expression");
-      assert(YDimVal && ExistingYDimVal &&
-             "Argument should be an integer constant expression");
-      assert(ZDimVal && ExistingZDimVal &&
-             "Argument should be an integer constant expression");
-
       // Compare attribute arguments value and warn for a mismatch.
       if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
           ExistingZDimVal != ZDimVal) {
@@ -3183,10 +3193,10 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
         S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
       }
     }
-  }
 
-  if (!checkWorkGroupSizeValues(S, D, AL))
-    return;
+    if (!checkWorkGroupSizeValues(S, D, AL))
+      return;
+  }
 
   S.addIntelSYCLTripleArgFunctionAttr<WorkGroupAttr>(D, AL, XDimExpr, YDimExpr,
                                                      ZDimExpr);
@@ -3246,26 +3256,28 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   S.CheckDeprecatedSYCLAttributeSpelling(AL);
 
-  if (!E->isValueDependent()) {
+  if (!E->isValueDependent() || !E->isInstantiationDependent()) {
     ASTContext &Ctx = S.getASTContext();
-    Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(Ctx);
+    llvm::APSInt ArgVal;
+    ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
+
+    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);
 
-      assert(ArgVal && XDimVal &&
-             "Argument should be an integer constant expression");
-      assert(ArgVal && YDimVal &&
-             "Argument should be an integer constant expression");
-      assert(ArgVal && ZDimVal &&
-             "Argument should be an integer constant expression");
-
-      int64_t NumSimdWorkItems = ArgVal->getSExtValue();
-      if (NumSimdWorkItems == 0)
-        return;
-
       if (!(XDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
             YDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
             ZDimVal->getZExtValue() % NumSimdWorkItems == 0)) {
@@ -6035,14 +6047,14 @@ 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
+          << 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;
     }
   }
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index 624bbf794765d..db0d47044150d 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -63,7 +63,7 @@ struct TRIFuncObjBad4 {
 };
 
 struct TRIFuncObjBad5 {
-  [[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+  [[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
   [[intel::reqd_work_group_size(5, 5, 5)]] void
   operator()() const {}
 };
@@ -84,7 +84,7 @@ struct TRIFuncObjBad7 {
 
 struct TRIFuncObjBad8 {
   [[intel::num_simd_work_items(3)]]     // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
-  [[intel::reqd_work_group_size(5, 5)]] //expected-note{{conflicting attribute is here}}
+  [[intel::reqd_work_group_size(5, 5)]] // expected-note{{conflicting attribute is here}}
   void
   operator()() const {}
 };
@@ -95,6 +95,54 @@ struct TRIFuncObjBad9 {
   void
   operator()() const {}
 };
+
+struct TRIFuncObjBad10 {
+  [[intel::reqd_work_group_size(5, 5, 5)]]
+  [[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad11 {
+  [[intel::num_simd_work_items(3.f)]] // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  [[intel::reqd_work_group_size(64, 64, 64)]]
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad12 {
+  [[intel::reqd_work_group_size(64, 64, 64)]]
+  [[intel::num_simd_work_items(3.f)]]  // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad13 {
+  [[intel::reqd_work_group_size(0)]] // expected-error{{'reqd_work_group_size' attribute must be greater than 0}}
+  [[intel::num_simd_work_items(0)]]  // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad14 {
+  [[intel::num_simd_work_items(0)]]  // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
+  [[intel::reqd_work_group_size(0)]] // expected-error{{'reqd_work_group_size' attribute must be greater than 0}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad15 {
+  [[intel::num_simd_work_items(3.f)]]  // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  [[intel::reqd_work_group_size(3.f)]] // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad16 {
+  [[intel::reqd_work_group_size(3.f)]] // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  [[intel::num_simd_work_items(3.f)]]  // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad17 {
+  [[intel::num_simd_work_items(-1)]]  // expected-error{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
+  [[intel::reqd_work_group_size(-1)]] // expected-warning{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
+  void operator()() const {}
+};
 #endif // TRIGGER_ERROR
 
 struct TRIFuncObjGood1 {
@@ -187,101 +235,181 @@ int main() {
     h.single_task<class test_kernel4>(TRIFuncObjGood1());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel4
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel5>(TRIFuncObjGood2());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel5
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel6>(TRIFuncObjGood3());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel6
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel7>(TRIFuncObjGood4());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel7
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel8>(TRIFuncObjGood5());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel8
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 3
     // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
     // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
 
     h.single_task<class test_kernel9>(TRIFuncObjGood6());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel9
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 3
     // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
     // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 5
     // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
 
     h.single_task<class test_kernel10>(TRIFuncObjGood7());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel10
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel11>(TRIFuncObjGood8());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel11
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel12>(TRIFuncObjGood9());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel12
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
     h.single_task<class test_kernel13>(TRIFuncObjGood10());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel13
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 64
     // CHECK-NEXT:  IntegerLiteral{{.*}}64{{$}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 1
     // CHECK-NEXT:  IntegerLiteral{{.*}}1{{$}}
     // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
+    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
+    // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
 #ifdef TRIGGER_ERROR
     [[intel::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}}
 
     h.single_task<class test_kernel14>(
-        []() [[intel::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+        []() [[intel::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
 
     h.single_task<class test_kernel15>(
-        []() [[intel::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+        []() [[intel::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
 
     h.single_task<class test_kernel16>(TRIFuncObjBad1());
 
@@ -297,11 +425,27 @@ int main() {
 
     h.single_task<class test_kernel22>(TRIFuncObjBad7());
 
-    h.single_task<class test_kernel13>(TRIFuncObjBad8());
+    h.single_task<class test_kernel23>(TRIFuncObjBad8());
 
     h.single_task<class test_kernel24>(TRIFuncObjBad9());
 
-    h.single_task<class test_kernel25>(
+    h.single_task<class test_kernel25>(TRIFuncObjBad10());
+
+    h.single_task<class test_kernel26>(TRIFuncObjBad11());
+
+    h.single_task<class test_kernel27>(TRIFuncObjBad12());
+
+    h.single_task<class test_kernel28>(TRIFuncObjBad13());
+
+    h.single_task<class test_kernel29>(TRIFuncObjBad14());
+
+    h.single_task<class test_kernel30>(TRIFuncObjBad15());
+
+    h.single_task<class test_kernel31>(TRIFuncObjBad16());
+
+    h.single_task<class test_kernel32>(TRIFuncObjBad17());
+
+    h.single_task<class test_kernel33>(
         []() [[intel::num_simd_work_items(1), intel::num_simd_work_items(2)]]{}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}}
 #endif // TRIGGER_ERROR
   });
diff --git a/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp b/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp
index 9d94d3a8e88ba..f308cb4f9e1d9 100644
--- a/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp
+++ b/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp
@@ -5,7 +5,6 @@
 // Test that checks wrong function template instantiation and ensures that the type
 // is checked properly when instantiating from the template definition.
 template <typename Ty>
-// expected-error@+3{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
 // expected-error@+2{{integral constant expression must have integral or unscoped enumeration type, not 'S'}}
 // expected-error@+1{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
 [[intel::num_simd_work_items(Ty{})]] void func() {}
@@ -16,7 +15,6 @@ void test() {
   func<S>();
   //expected-note@+1{{in instantiation of function template specialization 'func<float>' requested here}}
   func<float>();
-  //expected-note@+1{{in instantiation of function template specialization 'func<int>' requested here}}
   func<int>();
 }
 
@@ -35,7 +33,7 @@ constexpr int bar() { return 0; }
 template <int SIZE>
 class KernelFunctor {
 public:
-  // expected-error@+1{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+  // expected-error@+1{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
   [[intel::num_simd_work_items(SIZE)]] void operator()() {}
 };
 
@@ -57,7 +55,7 @@ int main() {
 
 // Test that checks template parameter support on function.
 template <int N>
-// expected-error@+1{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
+// expected-error@+1{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
 [[intel::num_simd_work_items(N)]] void func3() {}
 
 int check() {

From 66e2cc773732714b853641f720c4bf47bc52a70e Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Fri, 12 Feb 2021 09:48:27 -0800
Subject: [PATCH 09/12] Fix clang format issues

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Sema/Sema.h |  4 ++--
 clang/lib/Sema/SemaDeclAttr.cpp | 12 +++++-------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 45df062873d6b..d56721e3a2066 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13079,7 +13079,7 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
       }
     }
     if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim ||
-	CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems) {
+        CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems) {
       if (ArgInt < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
             << CI << /*non-negative*/ 1;
@@ -13099,7 +13099,7 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
 }
 
 static Expr *checkWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
-                                      Expr *E) {
+                                   Expr *E) {
   assert(E && "Attribute must have an argument.");
 
   if (!E->isInstantiationDependent()) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index e2721ac8cd970..c855e966bc6ee 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3095,8 +3095,7 @@ static bool checkWorkGroupSizeValues(Sema &S, Decl *D, const ParsedAttr &AL) {
   return Result;
 }
 
-static Expr *checkWorkSizeAttrExpr(Sema &S, const ParsedAttr &CI,
-                                   Expr *E) {
+static Expr *checkWorkSizeAttrExpr(Sema &S, const ParsedAttr &CI, Expr *E) {
   assert(E && "Attribute must have an argument.");
 
   if (!E->isInstantiationDependent()) {
@@ -3148,8 +3147,8 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   ASTContext &Ctx = S.getASTContext();
 
-  if (!XDimExpr->isValueDependent() &&
-      !YDimExpr->isValueDependent() && !ZDimExpr->isValueDependent()) {
+  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);
@@ -3161,7 +3160,7 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
     if (!XDimExpr || !YDimExpr || !ZDimExpr)
       return;
 
-     // Skip SEMA if we're in a template, this will be diagnosed later.
+    // Skip SEMA if we're in a template, this will be diagnosed later.
     if (S.getCurLexicalContext()->isDependentContext())
       return;
 
@@ -6047,8 +6046,7 @@ 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 << AANT_ArgumentIntegerConstant
-          << E->getSourceRange();
+          << Attr << AANT_ArgumentIntegerConstant << E->getSourceRange();
       return;
     }
     int32_t ArgInt = ArgVal->getSExtValue();

From 242054d5e5e4525e2dae4e6ee9c77f8420a926b4 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Tue, 16 Feb 2021 07:02:36 -0800
Subject: [PATCH 10/12] Update patch and address review comments

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Basic/Attr.td             |   7 +-
 clang/include/clang/Sema/Sema.h               |  23 ++--
 clang/lib/Sema/SemaDeclAttr.cpp               |  76 +++++-------
 .../SemaSYCL/num_simd_work_items_device.cpp   | 112 ++++++------------
 4 files changed, 83 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 6fc32b559ab0b..b52f26cf91e6a 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1304,6 +1304,11 @@ def SYCLIntelMaxWorkGroupSize : InheritableAttr {
     ArrayRef<const Expr *> dimensions() const {
       return {getXDim(), getYDim(), getZDim()};
     }
+    bool isValueDependent() const {
+      return (getXDim() && getXDim()->isValueDependent()) ||
+             (getYDim() && getYDim()->isValueDependent()) ||
+             (getZDim() && getZDim()->isValueDependent());
+    }
     Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
       return getXDim()->getIntegerConstantExpr(Ctx);
     }
@@ -2845,7 +2850,7 @@ def ReqdWorkGroupSize : InheritableAttr {
     ArrayRef<const Expr *> dimensions() const {
       return {getXDim(), getYDim(), getZDim()};
     }
-    bool isDependent() const {
+    bool isValueDependent() const {
       return (getXDim() && getXDim()->isValueDependent()) ||
              (getYDim() && getYDim()->isValueDependent()) ||
              (getZDim() && getZDim()->isValueDependent());
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d56721e3a2066..9a67b89603ff2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13074,7 +13074,7 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
     if (CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
       if (ArgInt <= 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI << /*positive*/ 0;
+            << CI.getAttrName() << /*positive*/ 0;
         return;
       }
     }
@@ -13082,14 +13082,14 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
         CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems) {
       if (ArgInt < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI << /*non-negative*/ 1;
+            << CI.getAttrName() << /*non-negative*/ 1;
         return;
       }
     }
     if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
       if (ArgInt > 3) {
         Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
-            << CI << 0 << 3 << E->getSourceRange();
+            << CI.getAttrName() << 0 << 3 << E->getSourceRange();
         return;
       }
     }
@@ -13098,8 +13098,8 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
   D->addAttr(::new (Context) AttrType(Context, CI, E));
 }
 
-static Expr *checkWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
-                                   Expr *E) {
+static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
+                                      Expr *E) {
   assert(E && "Attribute must have an argument.");
 
   if (!E->isInstantiationDependent()) {
@@ -13144,9 +13144,9 @@ void Sema::addIntelSYCLTripleArgFunctionAttr(Decl *D,
       !ZDimExpr->isValueDependent()) {
 
     // Save ConstantExpr in semantic attribute
-    XDimExpr = checkWorkSizeAttrExpr(*this, CI, XDimExpr);
-    YDimExpr = checkWorkSizeAttrExpr(*this, CI, YDimExpr);
-    ZDimExpr = checkWorkSizeAttrExpr(*this, CI, ZDimExpr);
+    XDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, XDimExpr);
+    YDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, YDimExpr);
+    ZDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, ZDimExpr);
 
     if (!XDimExpr || !YDimExpr || !ZDimExpr)
       return;
@@ -13231,7 +13231,8 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
 
     if (!ArgVal) {
       Diag(E->getExprLoc(), diag::err_attribute_argument_type)
-          << A << AANT_ArgumentIntegerConstant << E->getSourceRange();
+          << A.getAttrName() << AANT_ArgumentIntegerConstant
+          << E->getSourceRange();
       return nullptr;
     }
 
@@ -13241,7 +13242,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 << /* positive */ 0;
+            << A.getAttrName() << /* positive */ 0;
         return nullptr;
       }
     } else if (A.getParsedKind() ==
@@ -13252,7 +13253,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
                    ParsedAttr::AT_SYCLIntelFPGASpeculatedIterations) {
       if (Val < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << A << /* non-negative */ 1;
+            << A.getAttrName() << /* non-negative */ 1;
         return nullptr;
       }
     } else {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c855e966bc6ee..2df835d0654ec 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3095,21 +3095,6 @@ 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) {
@@ -3149,50 +3134,45 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   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);
-
-    XDimExpr = checkWorkSizeAttrExpr(S, AL, XDimExpr);
-    YDimExpr = checkWorkSizeAttrExpr(S, AL, YDimExpr);
-    ZDimExpr = checkWorkSizeAttrExpr(S, AL, ZDimExpr);
+    llvm::APSInt XDimVal, YDimVal, ZDimVal;
+    ExprResult XDim = S.VerifyIntegerConstantExpression(XDimExpr, &XDimVal);
+    ExprResult YDim = S.VerifyIntegerConstantExpression(YDimExpr, &YDimVal);
+    ExprResult ZDim = S.VerifyIntegerConstantExpression(ZDimExpr, &ZDimVal);
 
-    if (!XDimExpr || !YDimExpr || !ZDimExpr)
+    if (XDim.isInvalid())
       return;
+    XDimExpr = XDim.get();
 
-    // Skip SEMA if we're in a template, this will be diagnosed later.
-    if (S.getCurLexicalContext()->isDependentContext())
+    if (YDim.isInvalid())
       return;
+    YDimExpr = YDim.get();
 
-    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 (ZDim.isInvalid())
+      return;
+    ZDimExpr = ZDim.get();
+
+    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 (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);
-
+    if (const auto *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
       // Compare attribute arguments value and warn for a mismatch.
-      if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
-          ExistingZDimVal != ZDimVal) {
+       if (ExistingAttr->getXDimVal(Ctx) != XDimVal ||
+           ExistingAttr->getYDimVal(Ctx) != YDimVal ||
+           ExistingAttr->getZDimVal(Ctx) != ZDimVal) {
         S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
         S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
       }
     }
-
     if (!checkWorkGroupSizeValues(S, D, AL))
       return;
   }
@@ -3255,7 +3235,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   S.CheckDeprecatedSYCLAttributeSpelling(AL);
 
-  if (!E->isValueDependent() || !E->isInstantiationDependent()) {
+  if (!E->isValueDependent()) {
     ASTContext &Ctx = S.getASTContext();
     llvm::APSInt ArgVal;
     ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
index db0d47044150d..96c7db29de8ea 100644
--- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp
+++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp
@@ -139,6 +139,12 @@ struct TRIFuncObjBad16 {
 };
 
 struct TRIFuncObjBad17 {
+  [[intel::num_simd_work_items(3)]]
+  [[intel::reqd_work_group_size(3, 3, 3.f)]] // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
+  void operator()() const {}
+};
+
+struct TRIFuncObjBad18 {
   [[intel::num_simd_work_items(-1)]]  // expected-error{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
   [[intel::reqd_work_group_size(-1)]] // expected-warning{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
   void operator()() const {}
@@ -170,36 +176,24 @@ struct TRIFuncObjGood4 {
 };
 
 struct TRIFuncObjGood5 {
-  [[intel::num_simd_work_items(3)]]
-  [[intel::max_work_group_size(5, 5, 5)]] void
-  operator()() const {}
-};
-
-struct TRIFuncObjGood6 {
-  [[intel::max_work_group_size(5, 5, 5)]]
-  [[intel::num_simd_work_items(3)]] void
-  operator()() const {}
-};
-
-struct TRIFuncObjGood7 {
   [[intel::num_simd_work_items(4)]]
   [[intel::reqd_work_group_size(64)]] void
   operator()() const {}
 };
 
-struct TRIFuncObjGood8 {
+struct TRIFuncObjGood6 {
   [[intel::reqd_work_group_size(64)]]
   [[intel::num_simd_work_items(4)]] void
   operator()() const {}
 };
 
-struct TRIFuncObjGood9 {
+struct TRIFuncObjGood7 {
   [[intel::num_simd_work_items(4)]]
   [[intel::reqd_work_group_size(64, 64)]] void
   operator()() const {}
 };
 
-struct TRIFuncObjGood10 {
+struct TRIFuncObjGood8 {
   [[intel::reqd_work_group_size(64, 64)]]
   [[intel::num_simd_work_items(4)]] void
   operator()() const {}
@@ -302,40 +296,6 @@ int main() {
 
     h.single_task<class test_kernel8>(TRIFuncObjGood5());
     // CHECK-LABEL: FunctionDecl {{.*}}test_kernel8
-    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 3
-    // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
-    // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-
-    h.single_task<class test_kernel9>(TRIFuncObjGood6());
-    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel9
-    // CHECK:       SYCLIntelNumSimdWorkItemsAttr {{.*}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 3
-    // CHECK-NEXT:  IntegerLiteral{{.*}}3{{$}}
-    // CHECK:       SYCLIntelMaxWorkGroupSizeAttr {{.*}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-    // CHECK-NEXT:  ConstantExpr{{.*}}'int'
-    // CHECK-NEXT:  value: Int 5
-    // CHECK-NEXT:  IntegerLiteral{{.*}}5{{$}}
-
-    h.single_task<class test_kernel10>(TRIFuncObjGood7());
-    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel10
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
     // CHECK-NEXT:  ConstantExpr{{.*}}'int'
     // CHECK-NEXT:  value: Int 64
@@ -351,8 +311,8 @@ int main() {
     // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
-    h.single_task<class test_kernel11>(TRIFuncObjGood8());
-    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel11
+    h.single_task<class test_kernel9>(TRIFuncObjGood6());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel9
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
     // CHECK-NEXT:  ConstantExpr{{.*}}'int'
     // CHECK-NEXT:  value: Int 64
@@ -368,8 +328,8 @@ int main() {
     // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
-    h.single_task<class test_kernel12>(TRIFuncObjGood9());
-    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel12
+    h.single_task<class test_kernel10>(TRIFuncObjGood7());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel10
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
     // CHECK-NEXT:  ConstantExpr{{.*}}'int'
     // CHECK-NEXT:  value: Int 64
@@ -385,8 +345,8 @@ int main() {
     // CHECK-NEXT:  value: Int 4
     // CHECK-NEXT:  IntegerLiteral{{.*}}4{{$}}
 
-    h.single_task<class test_kernel13>(TRIFuncObjGood10());
-    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel13
+    h.single_task<class test_kernel11>(TRIFuncObjGood8());
+    // CHECK-LABEL: FunctionDecl {{.*}}test_kernel11
     // CHECK:       ReqdWorkGroupSizeAttr {{.*}}
     // CHECK-NEXT:  ConstantExpr{{.*}}'int'
     // CHECK-NEXT:  value: Int 64
@@ -405,47 +365,49 @@ int main() {
 #ifdef TRIGGER_ERROR
     [[intel::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}}
 
-    h.single_task<class test_kernel14>(
+    h.single_task<class test_kernel12>(
         []() [[intel::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute must be greater than 0}}
 
-    h.single_task<class test_kernel15>(
+    h.single_task<class test_kernel13>(
         []() [[intel::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}}
 
-    h.single_task<class test_kernel16>(TRIFuncObjBad1());
+    h.single_task<class test_kernel14>(TRIFuncObjBad1());
+
+    h.single_task<class test_kernel15>(TRIFuncObjBad2());
 
-    h.single_task<class test_kernel17>(TRIFuncObjBad2());
+    h.single_task<class test_kernel16>(TRIFuncObjBad3());
 
-    h.single_task<class test_kernel18>(TRIFuncObjBad3());
+    h.single_task<class test_kernel17>(TRIFuncObjBad4());
 
-    h.single_task<class test_kernel19>(TRIFuncObjBad4());
+    h.single_task<class test_kernel18>(TRIFuncObjBad5());
 
-    h.single_task<class test_kernel20>(TRIFuncObjBad5());
+    h.single_task<class test_kernel19>(TRIFuncObjBad6());
 
-    h.single_task<class test_kernel21>(TRIFuncObjBad6());
+    h.single_task<class test_kernel20>(TRIFuncObjBad7());
 
-    h.single_task<class test_kernel22>(TRIFuncObjBad7());
+    h.single_task<class test_kernel21>(TRIFuncObjBad8());
 
-    h.single_task<class test_kernel23>(TRIFuncObjBad8());
+    h.single_task<class test_kernel22>(TRIFuncObjBad9());
 
-    h.single_task<class test_kernel24>(TRIFuncObjBad9());
+    h.single_task<class test_kernel23>(TRIFuncObjBad10());
 
-    h.single_task<class test_kernel25>(TRIFuncObjBad10());
+    h.single_task<class test_kernel24>(TRIFuncObjBad11());
 
-    h.single_task<class test_kernel26>(TRIFuncObjBad11());
+    h.single_task<class test_kernel25>(TRIFuncObjBad12());
 
-    h.single_task<class test_kernel27>(TRIFuncObjBad12());
+    h.single_task<class test_kernel26>(TRIFuncObjBad13());
 
-    h.single_task<class test_kernel28>(TRIFuncObjBad13());
+    h.single_task<class test_kernel27>(TRIFuncObjBad14());
 
-    h.single_task<class test_kernel29>(TRIFuncObjBad14());
+    h.single_task<class test_kernel28>(TRIFuncObjBad15());
 
-    h.single_task<class test_kernel30>(TRIFuncObjBad15());
+    h.single_task<class test_kernel29>(TRIFuncObjBad16());
 
-    h.single_task<class test_kernel31>(TRIFuncObjBad16());
+    h.single_task<class test_kernel30>(TRIFuncObjBad17());
 
-    h.single_task<class test_kernel32>(TRIFuncObjBad17());
+    h.single_task<class test_kernel31>(TRIFuncObjBad18());
 
-    h.single_task<class test_kernel33>(
+    h.single_task<class test_kernel32>(
         []() [[intel::num_simd_work_items(1), intel::num_simd_work_items(2)]]{}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}}
 #endif // TRIGGER_ERROR
   });

From f4f08ec3689b0678ca5f93fb225bdca9edfac303 Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Tue, 16 Feb 2021 07:07:45 -0800
Subject: [PATCH 11/12] Fix clang format issues

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/lib/Sema/SemaDeclAttr.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2df835d0654ec..9a2812978a48b 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3166,9 +3166,9 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
     }
     if (const auto *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
       // Compare attribute arguments value and warn for a mismatch.
-       if (ExistingAttr->getXDimVal(Ctx) != XDimVal ||
-           ExistingAttr->getYDimVal(Ctx) != YDimVal ||
-           ExistingAttr->getZDimVal(Ctx) != ZDimVal) {
+      if (ExistingAttr->getXDimVal(Ctx) != XDimVal ||
+          ExistingAttr->getYDimVal(Ctx) != YDimVal ||
+          ExistingAttr->getZDimVal(Ctx) != ZDimVal) {
         S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
         S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
       }

From c8e1105934d23b8fab800c540bf9971e5258f66b Mon Sep 17 00:00:00 2001
From: Soumi Manna <soumi.manna@intel.com>
Date: Wed, 17 Feb 2021 05:36:04 -0800
Subject: [PATCH 12/12] Fix conflicts errors, move  and remove dedundant codes

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
---
 clang/include/clang/Basic/Attr.td | 10 ----------
 clang/lib/Sema/SemaDeclAttr.cpp   |  4 ++--
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index fd01e5708efb7..5935897980b2d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1296,11 +1296,6 @@ def SYCLIntelMaxWorkGroupSize : InheritableAttr {
     ArrayRef<const Expr *> dimensions() const {
       return {getXDim(), getYDim(), getZDim()};
     }
-    bool isValueDependent() const {
-      return (getXDim() && getXDim()->isValueDependent()) ||
-             (getYDim() && getYDim()->isValueDependent()) ||
-             (getZDim() && getZDim()->isValueDependent());
-    }
     Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
       return getXDim()->getIntegerConstantExpr(Ctx);
     }
@@ -2794,11 +2789,6 @@ def ReqdWorkGroupSize : InheritableAttr {
     ArrayRef<const Expr *> dimensions() const {
       return {getXDim(), getYDim(), getZDim()};
     }
-    bool isValueDependent() const {
-      return (getXDim() && getXDim()->isValueDependent()) ||
-             (getYDim() && getYDim()->isValueDependent()) ||
-             (getZDim() && getZDim()->isValueDependent());
-    }
     Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
       return getXDim()->getIntegerConstantExpr(Ctx);
     }
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 8108fe3d033e8..0558baa1e4ece 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3235,7 +3235,6 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   S.CheckDeprecatedSYCLAttributeSpelling(AL);
 
   if (!E->isValueDependent()) {
-    ASTContext &Ctx = S.getASTContext();
     llvm::APSInt ArgVal;
     ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
 
@@ -3252,6 +3251,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     }
 
     if (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
+      ASTContext &Ctx = S.getASTContext();
       Optional<llvm::APSInt> XDimVal = A->getXDimVal(Ctx);
       Optional<llvm::APSInt> YDimVal = A->getYDimVal(Ctx);
       Optional<llvm::APSInt> ZDimVal = A->getZDimVal(Ctx);
@@ -3267,7 +3267,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     }
   }
 
-  S.addIntelSingleArgAttr<SYCLIntelNumSimdWorkItemsAttr>(D, A, E);
+  S.addIntelSingleArgAttr<SYCLIntelNumSimdWorkItemsAttr>(D, AL, E);
 }
 
 // Handles use_stall_enable_clusters