[SYCL] Enable template parameter support for loop_unroll attribute (#1060)

vmaksimo · web-flow · commit 8d7a32a079c7 · 2020-01-30T15:46:47.000+03:00
Signed-off-by: Viktoria Maksimova &lt;viktoria.maksimova@intel.com&gt;
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
@@ -1182,17 +1182,22 @@ def OpenCLUnrollHint : InheritableAttr {
 
 def LoopUnrollHint : InheritableAttr {
   let Spellings = [CXX11<"clang","loop_unroll">];
-  let Args = [UnsignedArgument<"UnrollHint">];
+  let Args = [ExprArgument<"UnrollHintExpr">];
   let LangOpts = [SYCLIsDevice, SYCLIsHost];
+  let HasCustomTypeTransform = 1;
   let AdditionalMembers = [{
     static const char *getName() {
       return "loop_unroll";
     }
-    std::string getDiagnosticName() const {
-      std::string Value = "";
-      if (getUnrollHint())
-        Value = "(" + std::to_string(getUnrollHint()) + ")";
-      return "[[clang::loop_unroll" + Value + "]]";
+    std::string getDiagnosticName(const PrintingPolicy &Policy) const {
+      std::string ValueName;
+      llvm::raw_string_ostream OS(ValueName);
+      if (auto *E = getUnrollHintExpr()) {
+        OS << "(";
+        E->printPretty(OS, nullptr, Policy);
+        OS << ")";
+      }
+      return "[[clang::loop_unroll" + OS.str() + "]]";
     }
   }];
   let Documentation = [LoopUnrollHintDocs];
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -1673,6 +1673,11 @@ class Sema final {
   FPGALoopAttrT *BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
                                             Expr *E);
 
+  LoopUnrollHintAttr *BuildLoopUnrollHintAttr(const AttributeCommonInfo &A,
+                                              Expr *E);
+  OpenCLUnrollHintAttr *
+  BuildOpenCLLoopUnrollHintAttr(const AttributeCommonInfo &A, Expr *E);
+
   bool CheckQualifiedFunctionForTypeId(QualType T, SourceLocation Loc);
 
   bool CheckFunctionReturnType(QualType T, SourceLocation Loc);
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -706,8 +706,12 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
     // 1 - disable unroll.
     // other positive integer n - unroll by n.
     if (OpenCLHint || UnrollHint) {
-      ValueInt = OpenCLHint ? OpenCLHint->getUnrollHint()
-                            : UnrollHint->getUnrollHint();
+      ValueInt = 0;
+      if (OpenCLHint)
+        ValueInt = OpenCLHint->getUnrollHint();
+      else if (Expr *E = UnrollHint->getUnrollHintExpr())
+        ValueInt = E->EvaluateKnownConstInt(Ctx).getSExtValue();
+
       if (ValueInt == 0) {
         State = LoopHintAttr::Enable;
       } else if (ValueInt != 1) {
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -554,11 +554,47 @@ void CheckForIncompatibleUnrollHintAttributes(
     SourceLocation Loc = Range.getBegin();
     S.Diag(Loc, diag::err_loop_unroll_compatibility)
         << PragmaUnroll->getDiagnosticName(Policy)
-        << AttrUnroll->getDiagnosticName();
+        << AttrUnroll->getDiagnosticName(Policy);
   }
 }
 
-template <typename LoopUnrollAttrT>
+static bool CheckLoopUnrollAttrExpr(Sema &S, Expr *E,
+                                    const AttributeCommonInfo &A,
+                                    unsigned *UnrollFactor = nullptr) {
+  if (E && !E->isInstantiationDependent()) {
+    llvm::APSInt ArgVal(32);
+
+    if (!E->isIntegerConstantExpr(ArgVal, S.Context))
+      return S.Diag(E->getExprLoc(), diag::err_attribute_argument_type)
+             << A.getAttrName() << AANT_ArgumentIntegerConstant
+             << E->getSourceRange();
+
+    if (ArgVal.isNonPositive())
+      return S.Diag(E->getExprLoc(),
+                    diag::err_attribute_requires_positive_integer)
+             << A.getAttrName() << /* positive */ 0;
+
+    if (UnrollFactor)
+      *UnrollFactor = ArgVal.getZExtValue();
+  }
+  return false;
+}
+
+LoopUnrollHintAttr *Sema::BuildLoopUnrollHintAttr(const AttributeCommonInfo &A,
+                                                  Expr *E) {
+  return !CheckLoopUnrollAttrExpr(*this, E, A)
+             ? new (Context) LoopUnrollHintAttr(Context, A, E)
+             : nullptr;
+}
+
+OpenCLUnrollHintAttr *
+Sema::BuildOpenCLLoopUnrollHintAttr(const AttributeCommonInfo &A, Expr *E) {
+  unsigned UnrollFactor = 0;
+  return !CheckLoopUnrollAttrExpr(*this, E, A, &UnrollFactor)
+             ? new (Context) OpenCLUnrollHintAttr(Context, A, UnrollFactor)
+             : nullptr;
+}
+
 static Attr *handleLoopUnrollHint(Sema &S, Stmt *St, const ParsedAttr &A,
                                   SourceRange Range) {
   // Although the feature was introduced only in OpenCL C v2.0 s6.11.5, it's
@@ -574,30 +610,13 @@ static Attr *handleLoopUnrollHint(Sema &S, Stmt *St, const ParsedAttr &A,
     return nullptr;
   }
 
-  unsigned UnrollFactor = 0;
-
-  if (NumArgs == 1) {
-    Expr *E = A.getArgAsExpr(0);
-    llvm::APSInt ArgVal(32);
-
-    if (!E->isIntegerConstantExpr(ArgVal, S.Context)) {
-      S.Diag(A.getLoc(), diag::err_attribute_argument_type)
-          << A << AANT_ArgumentIntegerConstant << E->getSourceRange();
-      return nullptr;
-    }
-
-    int Val = ArgVal.getSExtValue();
-
-    if (Val <= 0) {
-      S.Diag(A.getRange().getBegin(),
-             diag::err_attribute_requires_positive_integer)
-          << A << /* positive */ 0;
-      return nullptr;
-    }
-    UnrollFactor = Val;
-  }
+  Expr *E = NumArgs ? A.getArgAsExpr(0) : nullptr;
+  if (A.getParsedKind() == ParsedAttr::AT_OpenCLUnrollHint)
+    return S.BuildOpenCLLoopUnrollHintAttr(A, E);
+  else if (A.getParsedKind() == ParsedAttr::AT_LoopUnrollHint)
+    return S.BuildLoopUnrollHintAttr(A, E);
 
-  return LoopUnrollAttrT::CreateImplicit(S.Context, UnrollFactor);
+  return nullptr;
 }
 
 static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
@@ -620,9 +639,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
   case ParsedAttr::AT_SYCLIntelFPGAMaxConcurrency:
     return handleIntelFPGALoopAttr<SYCLIntelFPGAMaxConcurrencyAttr>(S, A);
   case ParsedAttr::AT_OpenCLUnrollHint:
-    return handleLoopUnrollHint<OpenCLUnrollHintAttr>(S, St, A, Range);
   case ParsedAttr::AT_LoopUnrollHint:
-    return handleLoopUnrollHint<LoopUnrollHintAttr>(S, St, A, Range);
+    return handleLoopUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
     return handleSuppressAttr(S, St, A, Range);
   default:
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1008,6 +1008,8 @@ namespace {
     const SYCLIntelFPGAMaxConcurrencyAttr *
     TransformSYCLIntelFPGAMaxConcurrencyAttr(
         const SYCLIntelFPGAMaxConcurrencyAttr *MC);
+    const LoopUnrollHintAttr *
+    TransformLoopUnrollHintAttr(const LoopUnrollHintAttr *LU);
 
     ExprResult TransformPredefinedExpr(PredefinedExpr *E);
     ExprResult TransformDeclRefExpr(DeclRefExpr *E);
@@ -1429,6 +1431,13 @@ TemplateInstantiator::TransformSYCLIntelFPGAMaxConcurrencyAttr(
       *MC, TransformedExpr);
 }
 
+const LoopUnrollHintAttr *TemplateInstantiator::TransformLoopUnrollHintAttr(
+    const LoopUnrollHintAttr *LU) {
+  Expr *TransformedExpr =
+      getDerived().TransformExpr(LU->getUnrollHintExpr()).get();
+  return getSema().BuildLoopUnrollHintAttr(*LU, TransformedExpr);
+}
+
 ExprResult TemplateInstantiator::transformNonTypeTemplateParmRef(
                                                  NonTypeTemplateParmDecl *parm,
                                                  SourceLocation loc,
diff --git a/clang/test/CodeGenSYCL/loop_unroll.cpp b/clang/test/CodeGenSYCL/loop_unroll.cpp
@@ -1,42 +1,54 @@
 // RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
 
-// CHECK: br label %for.cond, !llvm.loop ![[COUNT:[0-9]+]]
-// CHECK: br label %while.cond, !llvm.loop ![[DISABLE:[0-9]+]]
-// CHECK: br i1 %{{.*}}, label %do.body, label %do.end, !llvm.loop ![[ENABLE:[0-9]+]]
 
-// CHECK: ![[COUNT]] = distinct !{![[COUNT]], ![[COUNT_A:[0-9]+]]}
-// CHECK-NEXT: ![[COUNT_A]] = !{!"llvm.loop.unroll.count", i32 8}
+void enable() {
+  int i = 1000;
+  // CHECK: br i1 %{{.*}}, label %do.body, label %do.end, !llvm.loop ![[ENABLE:[0-9]+]]
+  [[clang::loop_unroll]]
+  do {} while (i--);
+}
+
+template <int A>
 void count() {
+  // CHECK: br label %for.cond, !llvm.loop ![[COUNT:[0-9]+]]
   [[clang::loop_unroll(8)]]
   for (int i = 0; i < 1000; ++i);
+  // CHECK: br label %for.cond2, !llvm.loop ![[COUNT_TEMPLATE:[0-9]+]]
+  [[clang::loop_unroll(A)]]
+  for (int i = 0; i < 1000; ++i);
 }
 
-// CHECK: ![[DISABLE]] = distinct !{![[DISABLE]], ![[DISABLE_A:[0-9]+]]}
-// CHECK-NEXT: ![[DISABLE_A]] = !{!"llvm.loop.unroll.disable"}
+template <int A>
 void disable() {
-  int i = 1000;
+  int i = 1000, j = 100;
+  // CHECK: br label %while.cond, !llvm.loop ![[DISABLE:[0-9]+]]
   [[clang::loop_unroll(1)]]
+  while (j--);
+  // CHECK: br label %while.cond1, !llvm.loop ![[DISABLE_TEMPLATE:[0-9]+]]
+  [[clang::loop_unroll(A)]]
   while (i--);
 }
 
-// CHECK: ![[ENABLE]] = distinct !{![[ENABLE]], ![[ENABLE_A:[0-9]+]]}
-// CHECK-NEXT: ![[ENABLE_A]] = !{!"llvm.loop.unroll.enable"}
-void enable() {
-  int i = 1000;
-  [[clang::loop_unroll]]
-  do {} while (i--);
-}
-
 template <typename name, typename Func>
 __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
   kernelFunc();
 }
 
 int main() {
   kernel_single_task<class kernel_function>([]() {
-    count();
-    disable();
+    count<4>();
+    disable<1>();
     enable();
   });
   return 0;
 }
+
+// CHECK: ![[ENABLE]] = distinct !{![[ENABLE]], ![[ENABLE_A:[0-9]+]]}
+// CHECK-NEXT: ![[ENABLE_A]] = !{!"llvm.loop.unroll.enable"}
+// CHECK: ![[COUNT]] = distinct !{![[COUNT]], ![[COUNT_A:[0-9]+]]}
+// CHECK-NEXT: ![[COUNT_A]] = !{!"llvm.loop.unroll.count", i32 8}
+// CHECK: ![[COUNT_TEMPLATE]] = distinct !{![[COUNT_TEMPLATE]], ![[COUNT_TEMPLATE_A:[0-9]+]]}
+// CHECK-NEXT: ![[COUNT_TEMPLATE_A]] = !{!"llvm.loop.unroll.count", i32 4}
+// CHECK: ![[DISABLE]] = distinct !{![[DISABLE]], ![[DISABLE_A:[0-9]+]]}
+// CHECK-NEXT: ![[DISABLE_A]] = !{!"llvm.loop.unroll.disable"}
+// CHECKL ![[DISABLE_TEMPLATE]] = distinct !{!![[DISABLE_TEMPLATE]], ![[DISABLE_A]]}
diff --git a/clang/test/CodeGenSYCL/loop_unroll_host.cpp b/clang/test/CodeGenSYCL/loop_unroll_host.cpp
@@ -2,6 +2,14 @@
 // CHECK: br label %{{.*}}, !llvm.loop ![[COUNT:[0-9]+]]
 // CHECK: br label %{{.*}}, !llvm.loop ![[DISABLE:[0-9]+]]
 // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop ![[ENABLE:[0-9]+]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[COUNT_TEMPLATE:[0-9]+]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[DISABLE_TEMPLATE:[0-9]+]]
+
+template <int A>
+void unroll() {
+  [[clang::loop_unroll(A)]]
+  for (int i = 0; i < 1000; ++i);
+}
 
 int main() {
   // CHECK: ![[COUNT]] = distinct !{![[COUNT]], ![[COUNT_A:[0-9]+]]}
@@ -18,5 +26,11 @@ int main() {
   i = 1000;
   [[clang::loop_unroll]]
   do {} while (i--);
+
+  // CHECK: ![[COUNT_TEMPLATE]] = distinct !{![[COUNT_TEMPLATE]], ![[COUNT_TEMPLATE_A:[0-9]+]]}
+  // CHECK-NEXT: ![[COUNT_TEMPLATE_A]] = !{!"llvm.loop.unroll.count", i32 8}
+  unroll<8>();
+  // CHECK: ![[DISABLE_TEMPLATE]] = distinct !{![[DISABLE_TEMPLATE]], ![[DISABLE_A]]}
+  unroll<1>();
   return 0;
 }
diff --git a/clang/test/SemaSYCL/loop_unroll.cpp b/clang/test/SemaSYCL/loop_unroll.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify -pedantic %s
 
+template <int A>
+void bar() {
+  // expected-error@+1 {{'loop_unroll' attribute requires a positive integral compile time constant expression}}
+  [[clang::loop_unroll(A)]]
+  for (int i = 0; i < 10; ++i);
+}
+
 void foo() {
   // expected-error@+1 {{clang loop attributes must be applied to for, while, or do statements}}
   [[clang::loop_unroll(8)]] int a[10];
@@ -44,6 +51,12 @@ void foo() {
   constexpr int c = 4;
   [[clang::loop_unroll(c)]]
   for (int i = 0; i < 10; ++i);
+
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  bar<-4>();
+
+  // no error expected
+  bar<c>();
 }
 
 template <typename name, typename Func>
