[SYCL][FPGA] Allow safelen = 0 with ivdep attribute (#6336)

smanna12 · web-flow · commit 558b3ba40f70 · 2022-06-23T12:51:17.000-07:00
This patch allows safelen = 0 with ivdep attribute and emits suppressible warning when safelen of 0 or 1 is used

Current behaviour:

ivdep with safelen of 1 is allowed with no warning. Safelen of 0 causes an error.

Desired behaviour:

safelen of 1 and 0 should both be allowed but will have no effect on the loop.

Both should emit a warning. The warning should be suppressible with a -Wno flag.

Justification:

The safelen parameter of an ivdep will sometimes be set based on a template argument.
It makes this type of code much cleaner if the setting of 0 is permitted.

The warning for both the 0 and 1 case is helpful for users who might misunderstand the
meaning of the safelen parameter and set a value of 1 (or possibly 0) thinking it will have some effect.
The warning should be suppressible for those developers using the templated coding pattern described
above who understand and accept that a safelen of 0 will have no effect.

Signed-off-by: Soumi Manna soumi.manna@intel.com
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
@@ -3161,6 +3161,9 @@ is unspecified and is therefore considered infinite.
 In case of ivdep being applied both w/o an array variable and for a particular
 array, the array variables that were not designated a separate ivdep will receive
 the no-array ivdep's safelen, with the correspondent treatment by the backend.
+The ``[[intel::ivdep]]`` attribute applies a safelen value of 0 or 1 that has
+no effect on the loop and emits a suppressible warning when safelen value of 0
+or 1 is used.
 
 .. code-block:: c++
 
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -737,9 +737,10 @@ def NSobjectAttribute : DiagGroup<"NSObject-attribute">;
 def NSConsumedMismatch : DiagGroup<"nsconsumed-mismatch">;
 def NSReturnsMismatch : DiagGroup<"nsreturns-mismatch">;
 
+def SyclIvdepAttribute : DiagGroup<"ivdep-compat">;
 def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;
 def UnknownAttributes : DiagGroup<"unknown-attributes">;
-def IgnoredAttributes : DiagGroup<"ignored-attributes">;
+def IgnoredAttributes : DiagGroup<"ignored-attributes", [SyclIvdepAttribute]>;
 def AcceptedAttributes : DiagGroup<"accepted-attributes">;
 def Attributes : DiagGroup<"attributes", [UnknownAttributes,
                                           IgnoredAttributes,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11794,6 +11794,9 @@ def err_ivdep_declrefexpr_arg : Error<
 def warn_ivdep_redundant : Warning <"ignoring redundant Intel FPGA loop "
   "attribute 'ivdep': safelen %select{INF|%1}0 >= safelen %select{INF|%3}2">,
   InGroup<IgnoredAttributes>;
+def warn_ivdep_attribute_argument : Warning<
+  "'ivdep' attribute with value %0 has no effect; attribute ignored">,
+  InGroup<SyclIvdepAttribute>;
 def warn_attribute_spelling_deprecated : Warning<
   "attribute %0 is deprecated">,
   InGroup<DeprecatedAttributes>;
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -248,10 +248,11 @@ Sema::BuildSYCLIntelFPGAPipelineAttr(const AttributeCommonInfo &A, Expr *E) {
 
 static bool checkSYCLIntelFPGAIVDepSafeLen(Sema &S, llvm::APSInt &Value,
                                            Expr *E) {
-  if (!Value.isStrictlyPositive())
+  // This attribute requires a non-negative value.
+  if (!Value.isNonNegative())
     return S.Diag(E->getExprLoc(),
                   diag::err_attribute_requires_positive_integer)
-           << "'ivdep'" << /* positive */ 0;
+           << "'ivdep'" << /*non-negative*/ 1;
   return false;
 }
 
@@ -276,6 +277,12 @@ static IVDepExprResult HandleFPGAIVDepAttrExpr(Sema &S, Expr *E,
     if (checkSYCLIntelFPGAIVDepSafeLen(S, *ArgVal, E))
       return IVDepExprResult::Invalid;
     SafelenValue = ArgVal->getZExtValue();
+    // ivdep attribute allows both safelen = 0 and safelen = 1 with a warning.
+    if (SafelenValue == 0 || SafelenValue == 1) {
+      S.Diag(E->getExprLoc(), diag::warn_ivdep_attribute_argument)
+          << SafelenValue;
+      return IVDepExprResult::Invalid;
+    }
     return IVDepExprResult::SafeLen;
   }
 
diff --git a/clang/test/CodeGenSYCL/intel-fpga-ivdep-global.cpp b/clang/test/CodeGenSYCL/intel-fpga-ivdep-global.cpp
@@ -75,6 +75,34 @@ void ivdep_conflicting_safelen() {
   }
 }
 
+// Global ivdep w/ safelen value 1 is specified - do not annotate GEP
+//
+// CHECK: define {{.*}}spir_func void @_Z{{[0-9]+}}ivdep_safelen_onev()
+void ivdep_safelen_one() {
+  // CHECK: %[[ARRAY_A:[0-9a-z]+]] = alloca [10 x i32]
+  int a[10];
+  [[intel::ivdep(1)]] for (int i = 0; i != 10; ++i) {
+    // CHECK:  %{{[0-9a-z]+}} = getelementptr inbounds [10 x i32], ptr addrspace(4) %[[ARRAY_A]].ascast, i64 0, i64 %{{[0-9a-z]+}}
+    // CHECK-NOT: !llvm.index.group
+    a[i] = 0;
+    // CHECK: br label %for.cond, !llvm.loop ![[MD_NO_LOOP_SAFELEN1:[0-9]+]]
+  }
+}
+
+// Global ivdep w/ safelen value 0 is specified - do not annotate GEP
+//
+// CHECK: define {{.*}}spir_func void @_Z{{[0-9]+}}ivdep_safelen_zerov()
+void ivdep_safelen_zero() {
+  // CHECK: %[[ARRAY_A:[0-9a-z]+]] = alloca [10 x i32]
+  int a[10];
+  [[intel::ivdep(0)]] for (int i = 0; i != 10; ++i) {
+    // CHECK:  %{{[0-9a-z]+}} = getelementptr inbounds [10 x i32], ptr addrspace(4) %[[ARRAY_A]].ascast, i64 0, i64 %{{[0-9a-z]+}}
+    // CHECK-NOT: !llvm.index.group
+    a[i] = 0;
+    // CHECK: br label %for.cond, !llvm.loop ![[MD_NO_LOOP_SAFELEN2:[0-9]+]]
+  }
+}
+
 template <typename name, typename Func>
 __attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
   kernelFunc();
@@ -86,6 +114,8 @@ int main() {
     ivdep_no_param_multiple_geps();
     ivdep_safelen();
     ivdep_conflicting_safelen();
+    ivdep_safelen_one();
+    ivdep_safelen_zero();
   });
   return 0;
 }
@@ -122,3 +152,11 @@ int main() {
 // CHECK-DAG: ![[IDX_GROUP_B_CONFL_SAFELEN]] = distinct !{}
 // CHECK-DAG: ![[MD_LOOP_CONFL_SAFELEN]] = distinct !{![[MD_LOOP_CONFL_SAFELEN]], ![[#]], ![[IVDEP_CONFL_SAFELEN:[0-9]+]], ![[IVDEP_LEGACY_SAFELEN_5]]}
 // CHECK-DAG: ![[IVDEP_CONFL_SAFELEN]] = !{!"llvm.loop.parallel_access_indices", ![[IDX_GROUP_A_CONFL_SAFELEN]], ![[IDX_GROUP_B_CONFL_SAFELEN]], i32 5}
+
+/// Global ivdep w/ safelen value of 1 has no effect. Attribute is ignored and no IR is generated with safelen value of 1.
+// CHECK-DAG: ![[MD_NO_LOOP_SAFELEN1]] = distinct !{![[MD_NO_LOOP_SAFELEN1]], ![[#]]}
+// CHECK-NOT: !{!"llvm.loop.ivdep.safelen", i32 1}
+
+/// Global ivdep w/ safelen value of 0 has no effect. Attribute is ignored and no IR is generated with safelen value of 0.
+// CHECK-DAG: ![[MD_NO_LOOP_SAFELEN2]] = distinct !{![[MD_NO_LOOP_SAFELEN2]], ![[#]]}
+// CHECK-NOT: !{!"llvm.loop.ivdep.enable"}
diff --git a/clang/test/SemaSYCL/intel-fpga-loops.cpp b/clang/test/SemaSYCL/intel-fpga-loops.cpp
@@ -139,7 +139,7 @@ void goo() {
   // no diagnostics are expected
   [[intel::max_concurrency(0)]] for (int i = 0; i != 10; ++i)
       a[i] = 0;
-  // expected-error@+1 {{'ivdep' attribute requires a positive integral compile time constant expression}}
+  // expected-warning@+1 {{'ivdep' attribute with value 0 has no effect; attribute ignored}}
   [[intel::ivdep(0)]] for (int i = 0; i != 10; ++i)
       a[i] = 0;
   // expected-error@+1 {{'initiation_interval' attribute requires a positive integral compile time constant expression}}
@@ -418,10 +418,11 @@ void ivdep_dependent() {
   [[intel::ivdep(5)]] for (int i = 0; i != 10; ++i)
       a[i] = 0;
 
+  // expected-warning@+1 {{'ivdep' attribute with value 1 has no effect; attribute ignored}}
   [[intel::ivdep(C)]]
-  // expected-error@-1 {{'ivdep' attribute requires a positive integral compile time constant expression}}
+  // expected-error@-1 {{'ivdep' attribute requires a non-negative integral compile time constant expression}}
   for (int i = 0; i != 10; ++i)
-      a[i] = 0;
+    a[i] = 0;
 
   // expected-warning@+3 {{ignoring redundant Intel FPGA loop attribute 'ivdep': safelen 4 >= safelen 2}}
   // expected-note@+1 {{previous attribute is here}}
diff --git a/clang/test/SemaSYCL/sycl-ivdep-compat.cpp b/clang/test/SemaSYCL/sycl-ivdep-compat.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -Wno-ivdep-compat -verify %s
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -Wno-ignored-attributes -verify %s
+
+// expected-no-diagnostics
+
+// Test that the warning gets suppressed with a -Wno flag when the
+// [[intel::ivdep]] attribute applies a safelen value of 0 or 1 to the loop.
+
+void test_zero() {
+  int a[10];
+  [[intel::ivdep(0)]] for (int i = 0; i != 10; ++i)
+    a[i] = 0;
+}
+
+void test_one() {
+  int b[10];
+  [[intel::ivdep(1)]] for (int i = 0; i != 10; ++i)
+    b[i] = 0;
+}
