[SYCL] Refactor of [[intel::private_copies()] and [[intel::max_replicates()]] attributes

clang/include/clang/Basic/Attr.td

@@ -2038,7 +2038,7 @@ def IntelFPGAMerge : Attr {
   let Documentation = [IntelFPGAMergeAttrDocs];
 }
 
-def IntelFPGAMaxReplicates : Attr {
+def IntelFPGAMaxReplicates : InheritableAttr {
   let Spellings = [CXX11<"intelfpga","max_replicates">,
                    CXX11<"intel","max_replicates">];
   let Args = [ExprArgument<"Value">];

clang/include/clang/Sema/Sema.h

@@ -10234,6 +10234,12 @@ class Sema final {
                                 Expr *E);
   SYCLIntelLoopFuseAttr *
   MergeSYCLIntelLoopFuseAttr(Decl *D, const SYCLIntelLoopFuseAttr &A);
+  void AddIntelFPGAPrivateCopiesAttr(Decl *D, const AttributeCommonInfo &CI,
+                                     Expr *E);
+  void AddIntelFPGAMaxReplicatesAttr(Decl *D, const AttributeCommonInfo &CI,
+                                     Expr *E);
+  IntelFPGAMaxReplicatesAttr *
+  MergeIntelFPGAMaxReplicatesAttr(Decl *D, const IntelFPGAMaxReplicatesAttr &A);
 
   /// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
   void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
@@ -13093,13 +13099,6 @@ void Sema::addIntelSingleArgAttr(Decl *D, const AttributeCommonInfo &CI,
       return;
     E = ICE.get();
     int32_t ArgInt = ArgVal.getSExtValue();
-    if (CI.getParsedKind() == ParsedAttr::AT_IntelFPGAMaxReplicates) {
-      if (ArgInt <= 0) {
-        Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI << /*positive*/ 0;
-        return;
-      }
-    }
     if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
       if (ArgInt < 0) {
         Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
@@ -13114,19 +13113,6 @@ void Sema::addIntelSingleArgAttr(Decl *D, const AttributeCommonInfo &CI,
         return;
       }
     }
-    if (CI.getParsedKind() == ParsedAttr::AT_IntelFPGAPrivateCopies) {
-      if (ArgInt < 0) {
-        Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-            << CI << /*non-negative*/ 1;
-        return;
-      }
-    }
-  }
-
-  if (CI.getParsedKind() == ParsedAttr::AT_IntelFPGAPrivateCopies) {
-    if (!D->hasAttr<IntelFPGAMemoryAttr>())
-      D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
-          Context, IntelFPGAMemoryAttr::Default));
   }
 
   D->addAttr(::new (Context) AttrType(Context, CI, E));

clang/lib/Sema/SemaDecl.cpp

@@ -2626,6 +2626,8 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
     NewAttr = S.MergeSYCLIntelSchedulerTargetFmaxMhzAttr(D, *A);
   else if (const auto *A = dyn_cast<SYCLIntelNoGlobalWorkOffsetAttr>(Attr))
     NewAttr = S.MergeSYCLIntelNoGlobalWorkOffsetAttr(D, *A);
+  else if (const auto *A = dyn_cast<IntelFPGAMaxReplicatesAttr>(Attr))
+    NewAttr = S.MergeIntelFPGAMaxReplicatesAttr(D, *A);
   else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
     NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
 

clang/lib/Sema/SemaDeclAttr.cpp

@@ -300,7 +300,8 @@ static bool checkPositiveIntArgument(Sema &S, const AttrInfo &AI, const Expr *Ex
 /// Diagnose mutually exclusive attributes when present on a given
 /// declaration. Returns true if diagnosed.
 template <typename AttrTy>
-static bool checkAttrMutualExclusion(Sema &S, Decl *D, const ParsedAttr &AL) {
+static bool checkAttrMutualExclusion(Sema &S, Decl *D,
+                                     const AttributeCommonInfo &AL) {
   if (const auto *A = D->getAttr<AttrTy>()) {
     S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) << AL << A;
     S.Diag(A->getLocation(), diag::note_conflicting_attribute);
@@ -5961,16 +5962,73 @@ static void handleIntelFPGASimpleDualPortAttr(Sema &S, Decl *D,
                  IntelFPGASimpleDualPortAttr(S.Context, AL));
 }
 
-static void handleIntelFPGAMaxReplicatesAttr(Sema &S, Decl *D,
-                                             const ParsedAttr &A) {
-  checkForDuplicateAttribute<IntelFPGAMaxReplicatesAttr>(S, D, A);
+void Sema::AddIntelFPGAMaxReplicatesAttr(Decl *D, const AttributeCommonInfo &CI,
+                                         Expr *E) {
+  if (!E->isValueDependent()) {
+    // Validate that we have an integer constant expression and then store the
+    // converted constant expression into the semantic attribute so that we
+    // don't have to evaluate it again later.
+    llvm::APSInt ArgVal;
+    ExprResult Res = VerifyIntegerConstantExpression(E, &ArgVal);
+    if (Res.isInvalid())
+      return;
+    E = Res.get();
+    // This attribute requires a strictly positive value.
+    if (ArgVal <= 0) {
+      Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
+          << CI << /*positive*/ 0;
+      return;
+    }
+    // Check to see if there's a duplicate attribute with different values
+    // already applied to the declaration.
+    if (const auto *DeclAttr = D->getAttr<IntelFPGAMaxReplicatesAttr>()) {
+      // If the other attribute argument is instantiation dependent, we won't
+      // have converted it to a constant expression yet and thus we test
+      // whether this is a null pointer.
+      const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue());
+      if (DeclExpr && ArgVal != DeclExpr->getResultAsAPSInt()) {
+        Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
+        Diag(DeclAttr->getLocation(), diag::note_previous_attribute);
+        return;
+      }
+    }
+    // [[intel::fpga_register]] and [[intel::max_replicates()]]
+    // attributes are incompatible.
+    if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
+      return;
+  }
 
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
-    return;
+  D->addAttr(::new (Context) IntelFPGAMaxReplicatesAttr(Context, CI, E));
+}
 
+IntelFPGAMaxReplicatesAttr *
+Sema::MergeIntelFPGAMaxReplicatesAttr(Decl *D,
+                                      const IntelFPGAMaxReplicatesAttr &A) {
+  // Check to see if there's a duplicate attribute with different values
+  // already applied to the declaration.
+  if (const auto *DeclAttr = D->getAttr<IntelFPGAMaxReplicatesAttr>()) {
+    const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue());
+    const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getValue());
+    if (DeclExpr && MergeExpr &&
+        DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
+      Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
+      Diag(A.getLoc(), diag::note_previous_attribute);
+      return nullptr;
+    }
+  }
+  // [[intel::fpga_register]] and [[intel::max_replicates()]]
+  // attributes are incompatible.
+  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, A))
+    return nullptr;
+
+  return ::new (Context) IntelFPGAMaxReplicatesAttr(Context, A, A.getValue());
+}
+
+static void handleIntelFPGAMaxReplicatesAttr(Sema &S, Decl *D,
+                                             const ParsedAttr &A) {
   S.CheckDeprecatedSYCLAttributeSpelling(A);
 
-  S.addIntelSingleArgAttr<IntelFPGAMaxReplicatesAttr>(D, A, A.getArgAsExpr(0));
+  S.AddIntelFPGAMaxReplicatesAttr(D, A, A.getArgAsExpr(0));
 }
 
 /// Handle the merge attribute.
@@ -6096,15 +6154,55 @@ void Sema::AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI,
                  IntelFPGABankBitsAttr(Context, CI, Args.data(), Args.size()));
 }
 
+void Sema::AddIntelFPGAPrivateCopiesAttr(Decl *D, const AttributeCommonInfo &CI,
+                                         Expr *E) {
+  if (!E->isValueDependent()) {
+    // Validate that we have an integer constant expression and then store the
+    // converted constant expression into the semantic attribute so that we
+    // don't have to evaluate it again later.
+    llvm::APSInt ArgVal;
+    ExprResult Res = VerifyIntegerConstantExpression(E, &ArgVal);
+    if (Res.isInvalid())
+      return;
+    E = Res.get();
+    // This attribute requires a non-negative value.
+    if (ArgVal < 0) {
+      Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
+          << CI << /*non-negative*/ 1;
+      return;
+    }
+    // Check to see if there's a duplicate attribute with different values
+    // already applied to the declaration.
+    if (const auto *DeclAttr = D->getAttr<IntelFPGAPrivateCopiesAttr>()) {
+      // If the other attribute argument is instantiation dependent, we won't
+      // have converted it to a constant expression yet and thus we test
+      // whether this is a null pointer.
+      const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue());
+      if (DeclExpr && ArgVal != DeclExpr->getResultAsAPSInt()) {
+        Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
+        Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
+        return;
+      }
+    }
+    // [[intel::fpga_register]] and [[intel::private_copies()]]
+    // attributes are incompatible.
+    if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
+      return;
+    // If the declaration does not have [[intel::memory]]
+    // attribute, this creates default implicit memory.
+    if (!D->hasAttr<IntelFPGAMemoryAttr>())
+      D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
+          Context, IntelFPGAMemoryAttr::Default));
+  }
+
+  D->addAttr(::new (Context) IntelFPGAPrivateCopiesAttr(Context, CI, E));
+}
+
 static void handleIntelFPGAPrivateCopiesAttr(Sema &S, Decl *D,
                                              const ParsedAttr &A) {
-  checkForDuplicateAttribute<IntelFPGAPrivateCopiesAttr>(S, D, A);
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
-    return;
-
   S.CheckDeprecatedSYCLAttributeSpelling(A);
 
-  S.addIntelSingleArgAttr<IntelFPGAPrivateCopiesAttr>(D, A, A.getArgAsExpr(0));
+  S.AddIntelFPGAPrivateCopiesAttr(D, A, A.getArgAsExpr(0));
 }
 
 static void handleIntelFPGAForcePow2DepthAttr(Sema &S, Decl *D,

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -694,6 +694,26 @@ static void instantiateIntelSYCLFunctionAttr(
     S.addIntelSingleArgAttr<AttrName>(New, *Attr, Result.getAs<Expr>());
 }
 
+static void instantiateIntelFPGAPrivateCopiesAttr(
+    Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
+    const IntelFPGAPrivateCopiesAttr *A, Decl *New) {
+  EnterExpressionEvaluationContext Unevaluated(
+      S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  ExprResult Result = S.SubstExpr(A->getValue(), TemplateArgs);
+  if (!Result.isInvalid())
+    S.AddIntelFPGAPrivateCopiesAttr(New, *A, Result.getAs<Expr>());
+}
+
+static void instantiateIntelFPGAMaxReplicatesAttr(
+    Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
+    const IntelFPGAMaxReplicatesAttr *A, Decl *New) {
+  EnterExpressionEvaluationContext Unevaluated(
+      S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  ExprResult Result = S.SubstExpr(A->getValue(), TemplateArgs);
+  if (!Result.isInvalid())
+    S.AddIntelFPGAMaxReplicatesAttr(New, *A, Result.getAs<Expr>());
+}
+
 /// Determine whether the attribute A might be relevent to the declaration D.
 /// If not, we can skip instantiating it. The attribute may or may not have
 /// been instantiated yet.
@@ -850,13 +870,13 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
     }
     if (const auto *IntelFPGAPrivateCopies =
             dyn_cast<IntelFPGAPrivateCopiesAttr>(TmplAttr)) {
-      instantiateIntelSYCLFunctionAttr<IntelFPGAPrivateCopiesAttr>(
-          *this, TemplateArgs, IntelFPGAPrivateCopies, New);
+      instantiateIntelFPGAPrivateCopiesAttr(*this, TemplateArgs,
+                                            IntelFPGAPrivateCopies, New);
     }
     if (const auto *IntelFPGAMaxReplicates =
             dyn_cast<IntelFPGAMaxReplicatesAttr>(TmplAttr)) {
-      instantiateIntelSYCLFunctionAttr<IntelFPGAMaxReplicatesAttr>(
-          *this, TemplateArgs, IntelFPGAMaxReplicates, New);
+      instantiateIntelFPGAMaxReplicatesAttr(*this, TemplateArgs,
+                                            IntelFPGAMaxReplicates, New);
     }
     if (const auto *IntelFPGABankBits =
             dyn_cast<IntelFPGABankBitsAttr>(TmplAttr)) {

clang/test/SemaSYCL/intel-fpga-global-const.cpp

@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -ast-dump -verify -pedantic %s | FileCheck %s
+
+// Test that checks global constant variable (which allows the redeclaration) since
+// IntelFPGAConstVar is one of the subjects listed for [[intel::max_replicates()]] attribute.
+
+// Checking of duplicate argument values.
+//CHECK: VarDecl{{.*}}var_max_replicates
+//CHECK: IntelFPGAMaxReplicatesAttr
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value:{{.*}}12
+//CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+//CHECK: IntelFPGAMaxReplicatesAttr
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value:{{.*}}12
+//CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+[[intel::max_replicates(12)]] extern const int var_max_replicates;
+[[intel::max_replicates(12)]] const int var_max_replicates = 0; // OK
+
+// Merging of different arg values.
+//expected-warning@+2{{attribute 'max_replicates' is already applied with different arguments}}
+[[intel::max_replicates(12)]] extern const int var_max_replicates_1;
+[[intel::max_replicates(14)]] const int var_max_replicates_1 = 0;
+//expected-note@-2{{previous attribute is here}}
+
+// Merging of incompatible attributes.
+// FIXME: Diagnostic order isn't correct, this isn't what we'd want here but
+// this is an upstream issue. Merge function is calling here
+// checkAttrMutualExclusion() function that has backwards diagnostic behavior.
+// This should be fixed into upstream.
+//expected-error@+2{{'max_replicates' and 'fpga_register' attributes are not compatible}}
+//expected-note@+2{{conflicting attribute is here}}
+[[intel::max_replicates(12)]] extern const int var_max_replicates_2;
+[[intel::fpga_register]] const int var_max_replicates_2 =0;

clang/test/SemaSYCL/intel-fpga-local.cpp

@@ -145,6 +145,33 @@ void check_ast()
   [[intel::simple_dual_port]] int var_dual_port;
   [[intel::force_pow2_depth(1)]] int var_force_p2d;
   [[intel::force_pow2_depth(1)]] const int const_force_p2d[64] = {0, 1};
+
+  // Checking of duplicate argument values.
+  //CHECK: VarDecl{{.*}}var_max_replicates
+  //CHECK: IntelFPGAMaxReplicatesAttr
+  //CHECK-NEXT: ConstantExpr
+  //CHECK-NEXT: value:{{.*}}12
+  //CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+  //CHECK: IntelFPGAMaxReplicatesAttr
+  //CHECK-NEXT: ConstantExpr
+  //CHECK-NEXT: value:{{.*}}12
+  //CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+  [[intel::max_replicates(12)]]
+  [[intel::max_replicates(12)]] int var_max_replicates; // OK
+
+  // Checking of duplicate argument values.
+  //CHECK: VarDecl{{.*}}var_private_copies
+  //CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
+  //CHECK: IntelFPGAPrivateCopiesAttr
+  //CHECK-NEXT: ConstantExpr
+  //CHECK-NEXT: value:{{.*}}12
+  //CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+  //CHECK: IntelFPGAPrivateCopiesAttr
+  //CHECK-NEXT: ConstantExpr
+  //CHECK-NEXT: value:{{.*}}12
+  //CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
+  [[intel::private_copies(12)]]
+  [[intel::private_copies(12)]] int var_private_copies; // OK
 }
 
 //CHECK: FunctionDecl{{.*}}diagnostics
@@ -318,11 +345,17 @@ void diagnostics()
   //expected-note@+1 {{did you mean to use 'intel::max_replicates' instead?}}
   [[intelfpga::max_replicates(2)]] unsigned int max_replicates[64];
 
+  // Checking of different argument values.
+  //expected-warning@+2{{attribute 'max_replicates' is already applied with different arguments}}
+  [[intel::max_replicates(8)]] //expected-note{{previous attribute is here}}
+  [[intel::max_replicates(16)]] unsigned int max_repl[64];
+
   //expected-error@+1{{'max_replicates' attribute requires a positive integral compile time constant expression}}
   [[intel::max_replicates(0)]] unsigned int maxrepl_zero[64];
   //expected-error@+1{{'max_replicates' attribute requires a positive integral compile time constant expression}}
   [[intel::max_replicates(-1)]] unsigned int maxrepl_negative[64];
 
+  // Checking of incompatible attributes.
   //expected-error@+3{{'max_replicates' and 'fpga_register' attributes are not compatible}}
   [[intel::fpga_register]]
   //expected-note@-1 {{conflicting attribute is here}}
@@ -386,23 +419,16 @@ void diagnostics()
   //expected-note@+1 {{did you mean to use 'intel::private_copies' instead?}}
   [[intelfpga::private_copies(8)]] unsigned int private_copies[64];
 
+  // Checking of incompatible attributes.
   //expected-error@+2{{attributes are not compatible}}
   [[intel::private_copies(16)]]
   [[intel::fpga_register]]
   //expected-note@-2 {{conflicting attribute is here}}
   unsigned int pc_reg[64];
 
-  //CHECK: VarDecl{{.*}}pc_pc
-  //CHECK: IntelFPGAPrivateCopiesAttr
-  //CHECK-NEXT: ConstantExpr
-  //CHECK-NEXT: value:{{.*}}8
-  //CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
-  //CHECK: IntelFPGAPrivateCopiesAttr
-  //CHECK-NEXT: ConstantExpr
-  //CHECK-NEXT: value:{{.*}}16
-  //CHECK-NEXT: IntegerLiteral{{.*}}16{{$}}
-  //expected-warning@+2{{is already applied}}
-  [[intel::private_copies(8)]]
+  // Checking of different argument values.
+  //expected-warning@+2{{attribute 'private_copies' is already applied with different arguments}}
+  [[intel::private_copies(8)]] //expected-note{{previous attribute is here}}
   [[intel::private_copies(16)]] unsigned int pc_pc[64];
 
   //expected-error@+1{{'private_copies' attribute requires a non-negative integral compile time constant expression}}
@@ -803,9 +829,18 @@ void check_template_parameters() {
   //expected-error@+1{{'max_replicates' attribute requires a positive integral compile time constant expression}}
   [[intel::max_replicates(D)]]
   [[intel::max_replicates(C)]]
-  //expected-warning@-1{{attribute 'max_replicates' is already applied}}
   unsigned int max_replicates_duplicate;
 
+  // Test that checks template instantiations for different arg values.
+  [[intel::max_replicates(4)]] // expected-note {{previous attribute is here}}
+  // expected-warning@+1 {{attribute 'max_replicates' is already applied with different arguments}}
+  [[intel::max_replicates(C)]] unsigned int max_repl_duplicate[64];
+
+  // Test that checks template instantiations for different arg values.
+  [[intel::private_copies(4)]] // expected-note {{previous attribute is here}}
+  // expected-warning@+1 {{attribute 'private_copies' is already applied with different arguments}}
+  [[intel::private_copies(C)]] unsigned int var_private_copies;
+
   //expected-error@+3{{'max_replicates' and 'fpga_register' attributes are not compatible}}
   [[intel::fpga_register]]
   //expected-note@-1 {{conflicting attribute is here}}