diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 74fb6e7045fdb..4a4f14e617332 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1952,6 +1952,10 @@ def SYCLIntelFPGADisableLoopPipelining : DeclOrStmtAttr { } def : MutualExclusions<[SYCLIntelFPGAInitiationInterval, SYCLIntelFPGADisableLoopPipelining]>; +def : MutualExclusions<[SYCLIntelFPGAIVDep, + SYCLIntelFPGADisableLoopPipelining]>; +def : MutualExclusions<[SYCLIntelFPGAMaxConcurrency, + SYCLIntelFPGADisableLoopPipelining]>; def SYCLIntelFPGAMaxInterleaving : StmtAttr { let Spellings = [CXX11<"intelfpga","max_interleaving">, @@ -1963,6 +1967,8 @@ def SYCLIntelFPGAMaxInterleaving : StmtAttr { let HasCustomTypeTransform = 1; let Documentation = [SYCLIntelFPGAMaxInterleavingAttrDocs]; } +def : MutualExclusions<[SYCLIntelFPGADisableLoopPipelining, + SYCLIntelFPGAMaxInterleaving]>; def SYCLIntelFPGASpeculatedIterations : StmtAttr { let Spellings = [CXX11<"intelfpga","speculated_iterations">, @@ -1974,6 +1980,8 @@ def SYCLIntelFPGASpeculatedIterations : StmtAttr { let HasCustomTypeTransform = 1; let Documentation = [SYCLIntelFPGASpeculatedIterationsAttrDocs]; } +def : MutualExclusions<[SYCLIntelFPGADisableLoopPipelining, + SYCLIntelFPGASpeculatedIterations]>; def SYCLIntelFPGANofusion : StmtAttr { let Spellings = [CXX11<"intel","nofusion">]; diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h index a90131d478492..3307d1db19fc0 100644 --- a/clang/include/clang/Sema/ParsedAttr.h +++ b/clang/include/clang/Sema/ParsedAttr.h @@ -95,12 +95,6 @@ struct ParsedAttrInfo { const Decl *D) const { return true; } - /// Check if the given attribute is mutually exclusive with other attributes - /// already applied to the given statement. - virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A, - const Stmt *St) const { - return true; - } /// Check if this attribute is allowed by the language we are compiling, and /// issue a diagnostic if not. virtual bool diagLangOpts(Sema &S, const ParsedAttr &Attr) const { @@ -615,7 +609,12 @@ class ParsedAttr final bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const; bool diagnoseAppertainsTo(class Sema &S, const Stmt *St) const; bool diagnoseMutualExclusion(class Sema &S, const Decl *D) const; - bool diagnoseMutualExclusion(class Sema &S, const Stmt *St) const; + // This function stub exists for parity with the declaration checking code so + // that checkCommonAttributeFeatures() can work generically on declarations + // or statements. + bool diagnoseMutualExclusion(class Sema &S, const Stmt *St) const { + return true; + } bool appliesToDecl(const Decl *D, attr::SubjectMatchRule MatchRule) const; void getMatchRules(const LangOptions &LangOpts, SmallVectorImpl> diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp index 0bf7252695170..845af9a2792ac 100644 --- a/clang/lib/Sema/ParsedAttr.cpp +++ b/clang/lib/Sema/ParsedAttr.cpp @@ -167,10 +167,6 @@ bool ParsedAttr::diagnoseMutualExclusion(Sema &S, const Decl *D) const { return getInfo().diagMutualExclusion(S, *this, D); } -bool ParsedAttr::diagnoseMutualExclusion(Sema &S, const Stmt *St) const { - return getInfo().diagMutualExclusion(S, *this, St); -} - bool ParsedAttr::appliesToDecl(const Decl *D, attr::SubjectMatchRule MatchRule) const { return checkAttributeMatchRuleAppliesTo(D, MatchRule); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 71ceb8aefefc5..433edf6261668 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2540,9 +2540,9 @@ static bool mergeAlignedAttrs(Sema &S, NamedDecl *New, Decl *Old) { return AnyAdded; } -#define WANT_MERGE_LOGIC +#define WANT_DECL_MERGE_LOGIC #include "clang/Sema/AttrParsedAttrImpl.inc" -#undef WANT_MERGE_LOGIC +#undef WANT_DECL_MERGE_LOGIC static bool mergeDeclAttribute(Sema &S, NamedDecl *D, const InheritableAttr *Attr, diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp index 15a86309677d1..32c6edc771687 100644 --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -407,9 +407,22 @@ static Attr *handleUnlikely(Sema &S, Stmt *St, const ParsedAttr &A, return ::new (S.Context) UnlikelyAttr(S.Context, A); } +#define WANT_STMT_MERGE_LOGIC +#include "clang/Sema/AttrParsedAttrImpl.inc" +#undef WANT_STMT_MERGE_LOGIC + static void CheckForIncompatibleAttributes(Sema &S, const SmallVectorImpl &Attrs) { + // The vast majority of attributed statements will only have one attribute + // on them, so skip all of the checking in the common case. + if (Attrs.size() < 2) + return; + + // First, check for the easy cases that are table-generated for us. + if (!DiagnoseMutualExclusions(S, Attrs)) + return; + // There are 6 categories of loop hints attributes: vectorize, interleave, // unroll, unroll_and_jam, pipeline and distribute. Except for distribute they // come in two variants: a state form and a numeric form. The state form @@ -532,33 +545,6 @@ CheckForDuplicationSYCLLoopAttribute(Sema &S, } } -/// Diagnose mutually exclusive attributes when present on a given -/// declaration. Returns true if diagnosed. -template -static void CheckMutualExclusionSYCLLoopAttribute( - Sema &S, const SmallVectorImpl &Attrs) { - std::pair SeenAttrs; - const Attr *FirstSeen = nullptr; - - for (const auto *I : Attrs) { - // Remember the first attribute of the problematic type so that we can - // potentially diagnose it later. - if (!FirstSeen && isa(I)) - FirstSeen = I; - - // Remember if we've seen either of the attribute types. - if (isa(I)) - SeenAttrs.first = true; - else if (isa(I)) - SeenAttrs.second = true; - - // If we've seen both of the attribute types, then diagnose them both. - if (SeenAttrs.first && SeenAttrs.second) - S.Diag(I->getLocation(), diag::err_attributes_are_not_compatible) - << FirstSeen << I; - } -} - static void CheckForIncompatibleSYCLLoopAttributes( Sema &S, const SmallVectorImpl &Attrs) { CheckForDuplicationSYCLLoopAttribute( @@ -573,21 +559,6 @@ static void CheckForIncompatibleSYCLLoopAttributes( CheckForDuplicationSYCLLoopAttribute( S, Attrs); CheckForDuplicationSYCLLoopAttribute(S, Attrs, false); - CheckMutualExclusionSYCLLoopAttribute( - S, Attrs); - CheckMutualExclusionSYCLLoopAttribute( - S, Attrs); - CheckMutualExclusionSYCLLoopAttribute( - S, Attrs); - CheckMutualExclusionSYCLLoopAttribute(S, Attrs); - CheckMutualExclusionSYCLLoopAttribute(S, - Attrs); - CheckRedundantSYCLIntelFPGAIVDepAttrs(S, Attrs); CheckForDuplicationSYCLLoopAttribute(S, Attrs); } diff --git a/clang/test/SemaCXX/attr-likelihood.cpp b/clang/test/SemaCXX/attr-likelihood.cpp index d2f95d17069ca..03ba301251d78 100644 --- a/clang/test/SemaCXX/attr-likelihood.cpp +++ b/clang/test/SemaCXX/attr-likelihood.cpp @@ -147,5 +147,11 @@ void o() if constexpr (true) [[likely]] { // expected-warning@+1 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}} } else [[likely]]; + + if (1) [[likely, unlikely]] { // expected-error {{'unlikely' and 'likely' attributes are not compatible}} \ + // expected-note {{conflicting attribute is here}} + } else [[unlikely]][[likely]] { // expected-error {{'likely' and 'unlikely' attributes are not compatible}} \ + // expected-note {{conflicting attribute is here}} + } } #endif diff --git a/clang/test/SemaSYCL/intel-fpga-loops.cpp b/clang/test/SemaSYCL/intel-fpga-loops.cpp index 082899e84ab28..6bec33ff00b7a 100644 --- a/clang/test/SemaSYCL/intel-fpga-loops.cpp +++ b/clang/test/SemaSYCL/intel-fpga-loops.cpp @@ -313,23 +313,28 @@ void loop_attrs_compatibility() { [[intel::disable_loop_pipelining]] [[intel::loop_coalesce]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{'disable_loop_pipelining' and 'max_interleaving' attributes are not compatible}} + // expected-error@+3 {{'max_interleaving' and 'disable_loop_pipelining' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} [[intel::disable_loop_pipelining]] [[intel::max_interleaving(0)]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{'speculated_iterations' and 'disable_loop_pipelining' attributes are not compatible}} + // expected-error@+3 {{'disable_loop_pipelining' and 'speculated_iterations' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} [[intel::speculated_iterations(0)]] [[intel::disable_loop_pipelining]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{'disable_loop_pipelining' and 'max_concurrency' attributes are not compatible}} + // expected-error@+3 {{'max_concurrency' and 'disable_loop_pipelining' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} [[intel::disable_loop_pipelining]] [[intel::max_concurrency(0)]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{'initiation_interval' and 'disable_loop_pipelining' attributes are not compatible}} + // expected-error@+3 {{'disable_loop_pipelining' and 'initiation_interval' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} [[intel::initiation_interval(10)]] [[intel::disable_loop_pipelining]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{'disable_loop_pipelining' and 'ivdep' attributes are not compatible}} + // expected-error@+3 {{'ivdep' and 'disable_loop_pipelining' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} [[intel::disable_loop_pipelining]] [[intel::ivdep]] for (int i = 0; i != 10; ++i) a[i] = 0; diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 30f1ea103c902..29d358ebd12c3 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -3653,7 +3653,8 @@ static void GenerateAppertainsTo(const Record &Attr, raw_ostream &OS) { static void GenerateMutualExclusionsChecks(const Record &Attr, const RecordKeeper &Records, raw_ostream &OS, - raw_ostream &MergeOS) { + raw_ostream &MergeDeclOS, + raw_ostream &MergeStmtOS) { // Find all of the definitions that inherit from MutualExclusions and include // the given attribute in the list of exclusions to generate the // diagMutualExclusion() check. @@ -3717,43 +3718,59 @@ static void GenerateMutualExclusionsChecks(const Record &Attr, // Sema &S, Decl *D, Attr *A and that returns a bool (false on diagnostic, // true on success). if (Attr.isSubClassOf("InheritableAttr")) { - MergeOS << " if (const auto *Second = dyn_cast<" - << (Attr.getName() + "Attr").str() << ">(A)) {\n"; + MergeDeclOS << " if (const auto *Second = dyn_cast<" + << (Attr.getName() + "Attr").str() << ">(A)) {\n"; for (const std::string &A : DeclAttrs) { - MergeOS << " if (const auto *First = D->getAttr<" << A << ">()) {\n"; - MergeOS << " S.Diag(First->getLocation(), " - << "diag::err_attributes_are_not_compatible) << First << " - << "Second;\n"; - MergeOS << " S.Diag(Second->getLocation(), " - << "diag::note_conflicting_attribute);\n"; - MergeOS << " return false;\n"; - MergeOS << " }\n"; + MergeDeclOS << " if (const auto *First = D->getAttr<" << A + << ">()) {\n"; + MergeDeclOS << " S.Diag(First->getLocation(), " + << "diag::err_attributes_are_not_compatible) << First << " + << "Second;\n"; + MergeDeclOS << " S.Diag(Second->getLocation(), " + << "diag::note_conflicting_attribute);\n"; + MergeDeclOS << " return false;\n"; + MergeDeclOS << " }\n"; } - MergeOS << " return true;\n"; - MergeOS << " }\n"; + MergeDeclOS << " return true;\n"; + MergeDeclOS << " }\n"; } } + + // Statement attributes are a bit different from declarations. With + // declarations, each attribute is added to the declaration as it is + // processed, and so you can look on the Decl * itself to see if there is a + // conflicting attribute. Statement attributes are processed as a group + // because AttributedStmt needs to tail-allocate all of the attribute nodes + // at once. This means we cannot check whether the statement already contains + // an attribute to check for the conflict. Instead, we need to check whether + // the given list of semantic attributes contain any conflicts. It is assumed + // this code will be executed in the context of a function with parameters: + // Sema &S, const SmallVectorImpl &C. The code will be within a + // loop which loops over the container C with a loop variable named A to + // represent the current attribute to check for conflicts. + // + // FIXME: it would be nice not to walk over the list of potential attributes + // to apply to the statement more than once, but statements typically don't + // have long lists of attributes on them, so re-walking the list should not + // be an expensive operation. if (!StmtAttrs.empty()) { - // Generate the ParsedAttrInfo subclass logic for statements. - OS << " bool diagMutualExclusion(Sema &S, const ParsedAttr &AL, " - << "const Stmt *St) const override {\n"; - OS << " if (const auto *AS = dyn_cast(St)) {\n"; - OS << " const ArrayRef &Attrs = AS->getAttrs();\n"; - for (const std::string &A : StmtAttrs) { - OS << " auto Iter" << A << " = llvm::find_if(Attrs, [](const Attr " - << "*A) { return isa<" << A << ">(A); });\n"; - OS << " if (Iter" << A << " != Attrs.end()) {\n"; - OS << " S.Diag(AL.getLoc(), " - << "diag::err_attributes_are_not_compatible) << AL << *Iter" << A - << ";\n"; - OS << " S.Diag((*Iter" << A << ")->getLocation(), " - << "diag::note_conflicting_attribute);\n"; - OS << " return false;\n"; - OS << " }\n"; - } - OS << " }\n"; - OS << " return true;\n"; - OS << " }\n\n"; + MergeStmtOS << " if (const auto *Second = dyn_cast<" + << (Attr.getName() + "Attr").str() << ">(A)) {\n"; + MergeStmtOS << " auto Iter = llvm::find_if(C, [](const Attr *Check) " + << "{ return isa<"; + interleave( + StmtAttrs, [&](const std::string &Name) { MergeStmtOS << Name; }, + [&] { MergeStmtOS << ", "; }); + MergeStmtOS << ">(Check); });\n"; + MergeStmtOS << " if (Iter != C.end()) {\n"; + MergeStmtOS << " S.Diag((*Iter)->getLocation(), " + << "diag::err_attributes_are_not_compatible) << *Iter << " + << "Second;\n"; + MergeStmtOS << " S.Diag(Second->getLocation(), " + << "diag::note_conflicting_attribute);\n"; + MergeStmtOS << " return false;\n"; + MergeStmtOS << " }\n"; + MergeStmtOS << " }\n"; } } @@ -3905,7 +3922,8 @@ static bool IsKnownToGCC(const Record &Attr) { void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) { emitSourceFileHeader("Parsed attribute helpers", OS); - OS << "#if !defined(WANT_MERGE_LOGIC)\n"; + OS << "#if !defined(WANT_DECL_MERGE_LOGIC) && " + << "!defined(WANT_STMT_MERGE_LOGIC)\n"; PragmaClangAttributeSupport &PragmaAttributeSupport = getPragmaAttributeSupport(Records); @@ -3929,8 +3947,8 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) { // This stream is used to collect all of the declaration attribute merging // logic for performing mutual exclusion checks. This gets emitted at the // end of the file in a helper function of its own. - std::string DeclMergeChecks; - raw_string_ostream MergeOS(DeclMergeChecks); + std::string DeclMergeChecks, StmtMergeChecks; + raw_string_ostream MergeDeclOS(DeclMergeChecks), MergeStmtOS(StmtMergeChecks); // Generate a ParsedAttrInfo struct for each of the attributes. for (auto I = Attrs.begin(), E = Attrs.end(); I != E; ++I) { @@ -3987,7 +4005,7 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) { OS << " Spellings = " << I->first << "Spellings;\n"; OS << " }\n"; GenerateAppertainsTo(Attr, OS); - GenerateMutualExclusionsChecks(Attr, Records, OS, MergeOS); + GenerateMutualExclusionsChecks(Attr, Records, OS, MergeDeclOS, MergeStmtOS); GenerateLangOptRequirements(Attr, OS); GenerateTargetRequirements(Attr, Dupes, OS); GenerateSpellingIndexToSemanticSpelling(Attr, OS); @@ -4008,16 +4026,27 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) { // Generate the attribute match rules. emitAttributeMatchRules(PragmaAttributeSupport, OS); - OS << "#else // WANT_MERGE_LOGIC\n\n"; + OS << "#elif defined(WANT_DECL_MERGE_LOGIC)\n\n"; // Write out the declaration merging check logic. OS << "static bool DiagnoseMutualExclusions(Sema &S, const NamedDecl *D, " << "const Attr *A) {\n"; - OS << MergeOS.str(); + OS << MergeDeclOS.str(); OS << " return true;\n"; OS << "}\n\n"; - OS << "#endif // WANT_MERGE_LOGIC\n"; + OS << "#elif defined(WANT_STMT_MERGE_LOGIC)\n\n"; + + // Write out the statement merging check logic. + OS << "static bool DiagnoseMutualExclusions(Sema &S, " + << "const SmallVectorImpl &C) {\n"; + OS << " for (const Attr *A : C) {\n"; + OS << MergeStmtOS.str(); + OS << " }\n"; + OS << " return true;\n"; + OS << "}\n\n"; + + OS << "#endif\n"; } // Emits the kind list of parsed attributes