[SYCL] Let tablegen handle mutually exclusive decl attrs (#3507)

The new MutualExclusions tablegen facilities allow us to remove a bunch
of custom diagnostic checking code. This also fixes an issue with the
upstream implementation of the MutualExclusions facilities for
DeclOrStmtAttr attributes.

Author: AaronBallman

clang/include/clang/Basic/Attr.td

@@ -1950,6 +1950,8 @@ def SYCLIntelFPGADisableLoopPipelining : DeclOrStmtAttr {
   let Documentation = [SYCLIntelFPGADisableLoopPipeliningAttrDocs];
   let SupportsNonconformingLambdaSyntax = 1;
 }
+def : MutualExclusions<[SYCLIntelFPGAInitiationInterval,
+                        SYCLIntelFPGADisableLoopPipelining]>;
 
 def SYCLIntelFPGAMaxInterleaving : StmtAttr {
   let Spellings = [CXX11<"intelfpga","max_interleaving">,
@@ -2061,6 +2063,8 @@ def IntelFPGARegister : Attr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGARegisterAttrDocs];
 }
+def : MutualExclusions<[IntelFPGADoublePump, IntelFPGASinglePump,
+                        IntelFPGARegister]>;
 
 // One integral argument.
 def IntelFPGABankWidth : Attr {
@@ -2072,6 +2076,7 @@ def IntelFPGABankWidth : Attr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGABankWidthAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGABankWidth]>;
 
 def IntelFPGANumBanks : Attr {
   let Spellings = [CXX11<"intelfpga","numbanks">,
@@ -2091,6 +2096,7 @@ def IntelFPGAPrivateCopies : InheritableAttr {
   let Subjects = SubjectList<[IntelFPGALocalNonConstVar, Field], ErrorDiag>;
   let Documentation = [IntelFPGAPrivateCopiesAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGAPrivateCopies]>;
 
 // Two string arguments.
 def IntelFPGAMerge : Attr {
@@ -2102,6 +2108,7 @@ def IntelFPGAMerge : Attr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGAMergeAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGAMerge]>;
 
 def IntelFPGAMaxReplicates : InheritableAttr {
   let Spellings = [CXX11<"intelfpga","max_replicates">,
@@ -2112,6 +2119,7 @@ def IntelFPGAMaxReplicates : InheritableAttr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGAMaxReplicatesAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGAMaxReplicates]>;
 
 def IntelFPGASimpleDualPort : Attr {
   let Spellings = [CXX11<"intelfpga","simple_dual_port">,
@@ -2121,6 +2129,7 @@ def IntelFPGASimpleDualPort : Attr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGASimpleDualPortAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGASimpleDualPort]>;
 
 def SYCLFPGAPipe : TypeAttr {
   let Spellings = [GNU<"pipe">];
@@ -2147,6 +2156,7 @@ def IntelFPGABankBits : Attr {
   let LangOpts = [SYCLIsDevice, SYCLIsHost];
   let Documentation = [IntelFPGABankBitsDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGABankBits]>;
 
 def IntelFPGAForcePow2Depth : InheritableAttr {
   let Spellings = [CXX11<"intelfpga","force_pow2_depth">,
@@ -2157,6 +2167,7 @@ def IntelFPGAForcePow2Depth : InheritableAttr {
   let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
   let Documentation = [IntelFPGAForcePow2DepthAttrDocs];
 }
+def : MutualExclusions<[IntelFPGARegister, IntelFPGAForcePow2Depth]>;
 
 def Naked : InheritableAttr {
   let Spellings = [GCC<"naked">, Declspec<"naked">];

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3307,12 +3307,6 @@ static void handleUseStallEnableClustersAttr(Sema &S, Decl *D,
 static void handleSYCLIntelFPGADisableLoopPipeliningAttr(Sema &S, Decl *D,
                                                          const ParsedAttr &A) {
   S.CheckDeprecatedSYCLAttributeSpelling(A);
-
-  // [[intel::disable_loop_pipelining] and [[intel::initiation_interval()]]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<SYCLIntelFPGAInitiationIntervalAttr>(S, D, A))
-    return;
-
   D->addAttr(::new (S.Context)
                  SYCLIntelFPGADisableLoopPipeliningAttr(S.Context, A));
 }
@@ -3355,12 +3349,6 @@ void Sema::AddSYCLIntelFPGAInitiationIntervalAttr(Decl *D,
     }
   }
 
-  // [[intel::disable_loop_pipelining] and [[intel::initiation_interval()]]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<SYCLIntelFPGADisableLoopPipeliningAttr>(*this, D,
-                                                                       CI))
-    return;
-
   D->addAttr(::new (Context)
                  SYCLIntelFPGAInitiationIntervalAttr(Context, CI, E));
 }
@@ -3385,12 +3373,6 @@ Sema::MergeSYCLIntelFPGAInitiationIntervalAttr(
     }
   }
 
-  // [[intel::initiation_interval()]] and [[intel::disable_loop_pipelining]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<SYCLIntelFPGADisableLoopPipeliningAttr>(*this, D,
-                                                                       A))
-    return nullptr;
-
   return ::new (Context)
       SYCLIntelFPGAInitiationIntervalAttr(Context, A, A.getIntervalExpr());
 }
@@ -5802,17 +5784,11 @@ static void handleSYCLIntelNoGlobalWorkOffsetAttr(Sema &S, Decl *D,
   S.AddSYCLIntelNoGlobalWorkOffsetAttr(D, A, E);
 }
 
-/// Handle the [[intelfpga::doublepump]] and [[intelfpga::singlepump]] attributes.
-/// One but not both can be specified
-/// Both are incompatible with the __register__ attribute.
-template <typename AttrType, typename IncompatAttrType>
+/// Handle the [[intelfpga::doublepump]] and [[intelfpga::singlepump]]
+/// attributes.
+template <typename AttrType>
 static void handleIntelFPGAPumpAttr(Sema &S, Decl *D, const ParsedAttr &A) {
   checkForDuplicateAttribute<AttrType>(S, D, A);
-  if (checkAttrMutualExclusion<IncompatAttrType>(S, D, A))
-    return;
-
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
-    return;
 
   if (!D->hasAttr<IntelFPGAMemoryAttr>())
     D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
@@ -5867,28 +5843,10 @@ static bool checkIntelFPGARegisterAttrCompatibility(Sema &S, Decl *D,
     if (!MA->isImplicit() &&
         checkAttrMutualExclusion<IntelFPGAMemoryAttr>(S, D, Attr))
       InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGADoublePumpAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGASinglePumpAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGABankWidthAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGAPrivateCopiesAttr>(S, D, Attr))
-    InCompat = true;
   if (auto *NBA = D->getAttr<IntelFPGANumBanksAttr>())
     if (!NBA->isImplicit() &&
         checkAttrMutualExclusion<IntelFPGANumBanksAttr>(S, D, Attr))
       InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGAMaxReplicatesAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGASimpleDualPortAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGAMergeAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGABankBitsAttr>(S, D, Attr))
-    InCompat = true;
-  if (checkAttrMutualExclusion<IntelFPGAForcePow2DepthAttr>(S, D, Attr))
-    InCompat = true;
 
   return InCompat;
 }
@@ -5926,9 +5884,6 @@ static void handleIntelFPGASimpleDualPortAttr(Sema &S, Decl *D,
                                               const ParsedAttr &AL) {
   checkForDuplicateAttribute<IntelFPGASimpleDualPortAttr>(S, D, AL);
 
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, AL))
-    return;
-
   if (!D->hasAttr<IntelFPGAMemoryAttr>())
     D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
         S.Context, IntelFPGAMemoryAttr::Default));
@@ -5973,11 +5928,6 @@ void Sema::AddIntelFPGAMaxReplicatesAttr(Decl *D, const AttributeCommonInfo &CI,
     }
   }
 
-  // [[intel::fpga_register]] and [[intel::max_replicates()]]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
-    return;
-
   // If the declaration does not have an [[intel::fpga_memory]]
   // attribute, this creates one as an implicit attribute.
   if (!D->hasAttr<IntelFPGAMemoryAttr>())
@@ -6004,10 +5954,6 @@ Sema::MergeIntelFPGAMaxReplicatesAttr(Decl *D,
       }
     }
   }
-  // [[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());
 }
@@ -6026,9 +5972,6 @@ static void handleIntelFPGAMaxReplicatesAttr(Sema &S, Decl *D,
 static void handleIntelFPGAMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   checkForDuplicateAttribute<IntelFPGAMergeAttr>(S, D, AL);
 
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, AL))
-    return;
-
   SmallVector<StringRef, 2> Results;
   for (int I = 0; I < 2; I++) {
     StringRef Str;
@@ -6066,9 +6009,6 @@ static void handleIntelFPGABankBitsAttr(Sema &S, Decl *D, const ParsedAttr &A) {
   if (!A.checkAtLeastNumArgs(S, 1))
     return;
 
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
-    return;
-
   SmallVector<Expr *, 8> Args;
   for (unsigned I = 0; I < A.getNumArgs(); ++I) {
     Args.push_back(A.getArgAsExpr(I));
@@ -6176,11 +6116,6 @@ void Sema::AddIntelFPGAPrivateCopiesAttr(Decl *D, const AttributeCommonInfo &CI,
     }
   }
 
-  // [[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>())
@@ -6234,11 +6169,6 @@ void Sema::AddIntelFPGAForcePow2DepthAttr(Decl *D,
     }
   }
 
-  // [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
-    return;
-
   // If the declaration does not have an [[intel::fpga_memory]]
   // attribute, this creates one as an implicit attribute.
   if (!D->hasAttr<IntelFPGAMemoryAttr>())
@@ -6266,11 +6196,6 @@ Sema::MergeIntelFPGAForcePow2DepthAttr(Decl *D,
     }
   }
 
-  // [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
-  // attributes are incompatible.
-  if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, A))
-    return nullptr;
-
   return ::new (Context) IntelFPGAForcePow2DepthAttr(Context, A, A.getValue());
 }
 
@@ -9571,12 +9496,10 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
 
   // Intel FPGA specific attributes
   case ParsedAttr::AT_IntelFPGADoublePump:
-    handleIntelFPGAPumpAttr<IntelFPGADoublePumpAttr, IntelFPGASinglePumpAttr>(
-        S, D, AL);
+    handleIntelFPGAPumpAttr<IntelFPGADoublePumpAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_IntelFPGASinglePump:
-    handleIntelFPGAPumpAttr<IntelFPGASinglePumpAttr, IntelFPGADoublePumpAttr>(
-        S, D, AL);
+    handleIntelFPGAPumpAttr<IntelFPGASinglePumpAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_IntelFPGAMemory:
     handleIntelFPGAMemoryAttr(S, D, AL);

clang/test/SemaSYCL/initiation_interval.cpp

@@ -36,8 +36,8 @@
 // expected-note@+1 {{conflicting attribute is here}}
 [[intel::initiation_interval(4)]] [[intel::disable_loop_pipelining]] void func8();
 
-// expected-error@+2 {{'initiation_interval' and 'disable_loop_pipelining' attributes are not compatible}}
-// expected-note@+2 {{conflicting attribute is here}}
+// expected-error@+3 {{'disable_loop_pipelining' and 'initiation_interval' attributes are not compatible}}
+// expected-note@+1 {{conflicting attribute is here}}
 [[intel::initiation_interval(4)]] void func9();
 [[intel::disable_loop_pipelining]] void func9();
 

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

@@ -24,12 +24,8 @@
 //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}}
+//expected-error@+3{{'fpga_register' and 'max_replicates' attributes are not compatible}}
+//expected-note@+1{{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;
 
@@ -57,11 +53,7 @@
 //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{{'force_pow2_depth' and 'fpga_register' attributes are not compatible}}
-//expected-note@+2{{conflicting attribute is here}}
+//expected-error@+3{{'fpga_register' and 'force_pow2_depth' attributes are not compatible}}
+//expected-note@+1{{conflicting attribute is here}}
 [[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth_2;
 [[intel::fpga_register]] const int var_force_pow2_depth_2 =0;

clang/utils/TableGen/ClangAttrEmitter.cpp

@@ -3664,10 +3664,12 @@ static void GenerateMutualExclusionsChecks(const Record &Attr,
   if (Attr.isSubClassOf("TypeAttr"))
     return;
 
-  // This means the attribute is either a statement attribute or a decl
-  // attribute, find out which.
+  // This means the attribute is either a statement attribute, a decl
+  // attribute, or both; find out which.
   bool CurAttrIsStmtAttr =
       Attr.isSubClassOf("StmtAttr") || Attr.isSubClassOf("DeclOrStmtAttr");
+  bool CurAttrIsDeclAttr =
+      !CurAttrIsStmtAttr || Attr.isSubClassOf("DeclOrStmtAttr");
 
   std::vector<std::string> DeclAttrs, StmtAttrs;
 
@@ -3686,7 +3688,7 @@ static void GenerateMutualExclusionsChecks(const Record &Attr,
 
         if (CurAttrIsStmtAttr)
           StmtAttrs.push_back((AttrToExclude->getName() + "Attr").str());
-        else
+        if (CurAttrIsDeclAttr)
           DeclAttrs.push_back((AttrToExclude->getName() + "Attr").str());
       }
     }