8000 [SYCL] Correct the tablegen for checking mutually exclusive stmt attrs by AaronBallman · Pull Request #3519 · intel/llvm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[SYCL] Correct the tablegen for 8000 checking mutually exclusive stmt attrs #3519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 13, 2021
Merged

[SYCL] Correct the tablegen for checking mutually exclusive stmt attrs #3519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85c78a5
Correct the tablegen for checking mutually exclusive stmt attrs
AaronBallman Apr 9, 2021
9ecdad2
Adding missing test coverage for [[likely, unlikely]]
AaronBallman Apr 9, 2021
d3d99c6
Make the code uglier to appease clang-format
AaronBallman Apr 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,10 @@ def SYCLIntelFPGADisableLoopPipelining : DeclOrStmtAttr {
}
def : MutualExclusions<[SYCLIntelFPGAInitiationInterval,
SYCLIntelFPGADisableLoopPipelining]>;
def : MutualExclusions<[SYCLIntelFPGAIVDep,
SYCLIntelFPGADisableLoopPipelining]>;
def : MutualExclusions<[SYCLIntelFPGAMaxConcurrency,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: this is done for completeness in this review but will be in conflict with #3512. Assuming that one lands first, I'll correct the merge conflict in this review.

SYCLIntelFPGADisableLoopPipelining]>;

def SYCLIntelFPGAMaxInterleaving : StmtAttr {
let Spellings = [CXX11<"intelfpga","max_interleaving">,
Expand All @@ -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">,
Expand All @@ -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">];
Expand Down
13 changes: 6 additions & 7 deletions clang/include/clang/Sema/ParsedAttr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like this design but I could not think of a more elegant solution. If you have better ideas that don't result in code duplication in checkCommonAttributeFeatures(), I'm all ears.

// 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<std::pair<attr::SubjectMatchRule, bool>>
Expand Down
4 changes: 0 additions & 4 deletions clang/lib/Sema/ParsedAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2540,9 +2540,9 @@ static bool mergeAlignedAttrs(Sema &S, NamedDecl *New, Decl *Old) {
return A ED48 nyAdded;
}

#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,
Expand Down
55 changes: 13 additions & 42 deletions clang/lib/Sema/SemaStmtAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const Attr *> &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
Expand Down Expand Up @@ -532,33 +545,6 @@ CheckForDuplicationSYCLLoopAttribute(Sema &S,
}
}

/// Diagnose mutually exclusive attributes when present on a given
/// declaration. Returns true if diagnosed.
template <typename LoopAttrT, typename LoopAttrT2>
static void CheckMutualExclusionSYCLLoopAttribute(
Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {
std::pair<bool, bool> 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<LoopAttrT, LoopAttrT2>(I))
FirstSeen = I;

// Remember if we've seen either of the attribute types.
if (isa<LoopAttrT>(I))
SeenAttrs.first = true;
else if (isa<LoopAttrT2>(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<const Attr *> &Attrs) {
CheckForDuplicationSYCLLoopAttribute<SYCLIntelFPGAInitiationIntervalAttr>(
Expand All @@ -573,21 +559,6 @@ static void CheckForIncompatibleSYCLLoopAttributes(
CheckForDuplicationSYCLLoopAttribute<SYCLIntelFPGASpeculatedIterationsAttr>(
S, Attrs);
CheckForDuplicationSYCLLoopAttribute<LoopUnrollHintAttr>(S, Attrs, false);
CheckMutualExclusionSYCLLoopAttribute<SYCLIntelFPGADisableLoopPipeliningAttr,
SYCLIntelFPGAMaxInterleavingAttr>(
S, Attrs);
CheckMutualExclusionSYCLLoopAttribute<SYCLIntelFPGADisableLoopPipeliningAttr,
SYCLIntelFPGASpeculatedIterationsAttr>(
S, Attrs);
CheckMutualExclusionSYCLLoopAttribute<SYCLIntelFPGADisableLoopPipeliningAttr,
SYCLIntelFPGAInitiationIntervalAttr>(
S, Attrs);
CheckMutualExclusionSYCLLoopAttribute<SYCLIntelFPGADisableLoopPipeliningAttr,
SYCLIntelFPGAIVDepAttr>(S, Attrs);
CheckMutualExclusionSYCLLoopAttribute<SYCLIntelFPGADisableLoopPipeliningAttr,
SYCLIntelFPGAMaxConcurrencyAttr>(S,
Attrs);

CheckRedundantSYCLIntelFPGAIVDepAttrs(S, Attrs);
CheckForDuplicationSYCLLoopAttribute<SYCLIntelFPGANofusionAttr>(S, Attrs);
}
Expand Down
6 changes: 6 additions & 0 deletions clang/test/SemaCXX/attr-likelihood.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 10 additions & 5 deletions clang/test/SemaSYCL/intel-fpga-loops.cpp
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
109 changes: 69 additions & 40 deletions clang/utils/TableGen/ClangAttrEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment on lines -3656 to +3657
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not in love with this function generating code into three separate streams, but I couldn't think of a better approach that avoids walking the entire list of records from Attr.td multiple times (which slows down compile speeds because there are a fair amount of records in that file these days).

// Find all of the definitions that inherit from MutualExclusions and include
// the given attribute in the list of exclusions to generate the
// diagMutualExclusion() check.
Expand Down Expand Up @@ -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<const Attr *> &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<AttributedStmt>(St)) {\n";
OS << " const ArrayRef<const Attr *> &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";
}
}

Expand Down Expand Up @@ -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);

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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<const Attr *> &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
Expand Down
0