[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen
Aaron Ballman
aaron at aaronballman.com
Sat May 24 08:16:47 PDT 2014
On Fri, May 23, 2014 at 5:17 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi,
>
> As requested here is a patch to add PragmaAttr and Pragma spelling to tablgen and make use of those in the LoopHintAttr. This patch builds off the patch patch given in the '#pragma vectorize’ thread. This is a git patch so please using 'patch -p1’ to apply it.
>
> diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h
> index fc48816..49a13f8 100644
> --- a/include/clang/AST/Attr.h
> +++ b/include/clang/AST/Attr.h
> @@ -148,0 +149,17 @@ public:
> +class PragmaAttr : public Attr {
> +protected:
> + PragmaAttr(attr::Kind AK, SourceRange R, unsigned SpellingListIndex = 0)
> + : Attr(AK, R, SpellingListIndex) {}
> +
> +public:
> + // Print this the pragma. Implemented uniquely by each PragmaAttr def.
> + virtual void print(raw_ostream &OS, const PrintingPolicy &Policy) const = 0;
> +
> + // Implement isa/cast/dyncast/etc.
> + static bool classof(const Attr *A) {
> + // Relies on relative order of enum emission with respect to MS inheritance
> + // attrs.
> + return A->getKind() <= attr::LAST_PRAGMA;
> + }
> +};
I don't think we actually need a PragmaAttr class. This construct is
really only useful if we need to wholesale modify behavior of
attribute semantics. Since pragma attributes aren't that different
from any other attribute (in terms of semantics), this seems
unnecessary.
Also, all attribute already get a printPretty function that's
tablegenned for them, and that should be sufficient for printing
pragmas as well (perhaps with some modification).
Really, a pragma is simply a different *spelling* for an attribute
moreso than a different *kind* of attribute.
> +
> diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
> index 2cba851..74a21e9 100644
> --- a/include/clang/Basic/Attr.td
> +++ b/include/clang/Basic/Attr.td
> @@ -192,0 +193 @@ class Keyword<string name> : Spelling<name, "Keyword">;
> +class Pragma<string name> : Spelling<name, "Pragma">;
I'm not certain this spelling is quite sufficient as many pragmas have
a namespace of sorts. As best I can tell, most pragmas come in one of
four forms:
#pragma foo bar(args)
#pragma foo bar args
#pragma foo(args)
#pragma foo args
For instance, your own pragma is #pragma loop vectorize(args) --or--
#pragma loop interleave(args). So I think we want this to be:
class Pragma<string prefix, string name> : Spelling<name, "Pragma"> {
let Prefix = prefix;
}
Arguments are a considerably more difficult problem to solve since
there's no real standarization for their syntax. Also, it's a bit
tricky to draw the line between a namespace (like #pragma STDC
FP_CONTRACT) and arguments (like #pramga loop vectorize(on)).
> @@ -292,0 +294,3 @@ class InheritableAttr : Attr;
> +/// A pragma attribute is inherited by later redeclarations.
> +class PragmaAttr : Attr;
We don't need something like this for the same reason we don't need
the PragmaAttr class above.
> +
> @@ -1766 +1770 @@ def Unaligned : IgnoredAttr {
> -def LoopHint : Attr {
> +def LoopHint : PragmaAttr {
> @@ -1777,3 +1781 @@ def LoopHint : Attr {
> - // FIXME: Add Pragma spelling to tablegen and
> - // use it here.
> - let Spellings = [Keyword<"loop">];
> + let Spellings = [Pragma<"loop">];
> diff --git a/include/clang/Basic/AttrKinds.h b/include/clang/Basic/AttrKinds.h
> index 150a30e..2ae02ba 100644
> --- a/include/clang/Basic/AttrKinds.h
> +++ b/include/clang/Basic/AttrKinds.h
> @@ -26,0 +27 @@ enum Kind {
> +#define LAST_PRAGMA_ATTR(X) X, LAST_PRAGMA = X,
> diff --git a/include/clang/Basic/Attributes.h b/include/clang/Basic/Attributes.h
> index 4a7e462..5783b3b 100644
> --- a/include/clang/Basic/Attributes.h
> +++ b/include/clang/Basic/Attributes.h
> @@ -28 +28,3 @@ enum class AttrSyntax {
> - CXX
> + CXX,
> + // Is the identifier known as a pragma attribute?
> + Pragma
> diff --git a/include/clang/Sema/AttributeList.h b/include/clang/Sema/AttributeList.h
> index 6872ccc..24e4cd4 100644
> --- a/include/clang/Sema/AttributeList.h
> +++ b/include/clang/Sema/AttributeList.h
> @@ -83 +83,3 @@ public:
> - AS_Keyword
> + AS_Keyword,
> + /// #pragma ...
> + AS_Pragma
> diff --git a/lib/AST/StmtPrinter.cpp b/lib/AST/StmtPrinter.cpp
> index cc60e11..e572725 100644
> --- a/lib/AST/StmtPrinter.cpp
> +++ b/lib/AST/StmtPrinter.cpp
> @@ -174,6 +174,3 @@ void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
> - // FIXME: This hack will be removed when printPretty
> - // has been modified to print pretty pragmas
> - if (const LoopHintAttr *LHA = dyn_cast<LoopHintAttr>(Attr)) {
> - OS << "#pragma loop ";
> - LHA->print(OS, Policy);
> - } else
> + if (isa<PragmaAttr>(Attr))
> + Attr->printPretty(OS, Policy);
> + else
I don't believe this is required any longer -- should just be able to
go off printPretty by itself (at least, that's the goal!).
> diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
> index 9ab23d6..8534a69 100644
> --- a/lib/Parse/ParseStmt.cpp
> +++ b/lib/Parse/ParseStmt.cpp
> @@ -1784 +1783,0 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts, bool OnlyStatement,
> - // FIXME: Replace AS_Keyword with Pragma spelling AS_Pragma.
> @@ -1786 +1785 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts, bool OnlyStatement,
> - ArgHints, 3, AttributeList::AS_Keyword);
> + ArgHints, 3, AttributeList::AS_Pragma);
I would bet there's quite a few more attributes that need this modification.
> diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp
> index c409218..ab47e70 100644
> --- a/utils/TableGen/ClangAttrEmitter.cpp
> +++ b/utils/TableGen/ClangAttrEmitter.cpp
> @@ -1056 +1056 @@ writePrettyPrintFunction(Record &R,
> - if (Spellings.size() == 0) {
> + if (Spellings.empty()) {
> @@ -1092,0 +1093,3 @@ writePrettyPrintFunction(Record &R,
> + } else if (Variety == "Pragma") {
> + Prefix = "#pragma ";
> + Suffix = "\n";
> @@ -1102,0 +1106,8 @@ writePrettyPrintFunction(Record &R,
> + if (Variety == "Pragma") {
> + OS << " \";\n";
> + OS << " print(OS, Policy);\n";
> + OS << " break;\n";
> + OS << " }\n";
> + continue;
> + }
The pretty printing should not delegate to the attribute like this. I
think it should be entirely table generated.
> +
> @@ -1613,0 +1625,9 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> + OS << "#ifndef PRAGMA_ATTR\n";
> + OS << "#define PRAGMA_ATTR(NAME) ATTR(NAME)\n";
> + OS << "#endif\n\n";
> +
> + OS << "#ifndef LAST_PRAGMA_ATTR\n";
> + OS << "#define LAST_PRAGMA_ATTR(NAME) PRAGMA_ATTR(NAME)\n";
> + OS << "#endif\n\n";
> +
> + Record *PraClass = Records.getClass("PragmaAttr");
> @@ -1616,3 +1636,3 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> - std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr"),
> - NonInhAttrs, InhAttrs, InhParamAttrs;
> - for (auto *Attr : Attrs) {
> + std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr"),
> + NonInhAttrs, InhAttrs, InhParamAttrs, PraAttrs;
> + for (auto Attr : Attrs) {
> @@ -1625,0 +1646,2 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> + else if (Attr->isSubClassOf(PraClass))
> + PraAttrs.push_back(Attr);
> @@ -1631,0 +1654 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> + EmitAttrList(OS, "PRAGMA_ATTR", PraAttrs);
> @@ -1634,0 +1658 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> + OS << "#undef PRAGMA_ATTR\n";
> @@ -1637,0 +1662 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
> + OS << "#undef LAST_PRAGMA_ATTR\n";
> @@ -1779 +1804 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
> - std::vector<Record *> Declspec, GNU;
> + std::vector<Record *> Declspec, GNU, Pragma;
> @@ -1792 +1817 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
> - else if (Variety == "CXX11") {
> + else if (Variety == "CXX11")
> @@ -1794 +1819,2 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
> - }
> + else if (Variety == "Pragma")
> + Pragma.push_back(R);
> @@ -1807,0 +1834,3 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
> + OS << "case AttrSyntax::Pragma:\n";
> + OS << " return llvm::StringSwitch<bool>(Name)\n";
> + GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
> @@ -1843,11 +1872,11 @@ void EmitClangAttrSpellingListIndex(RecordKeeper &Records, raw_ostream &OS) {
> - OS << " if (Name == \""
> - << Spellings[I].name() << "\" && "
> - << "SyntaxUsed == "
> - << StringSwitch<unsigned>(Spellings[I].variety())
> - .Case("GNU", 0)
> - .Case("CXX11", 1)
> - .Case("Declspec", 2)
> - .Case("Keyword", 3)
> - .Default(0)
> - << " && Scope == \"" << Spellings[I].nameSpace() << "\")\n"
> - << " return " << I << ";\n";
> + OS << " if (Name == \"" << Spellings[I].name() << "\" && "
> + << "SyntaxUsed == "
> + << StringSwitch<unsigned>(Spellings[I].variety())
> + .Case("GNU", 0)
> + .Case("CXX11", 1)
> + .Case("Declspec", 2)
> + .Case("Keyword", 3)
> + .Case("Pragma", 4)
> + .Default(0)
> + << " && Scope == \"" << Spellings[I].nameSpace() << "\")\n"
> + << " return " << I << ";\n";
> @@ -2473 +2502 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) {
> - std::vector<StringMatcher::StringPair> GNU, Declspec, CXX11, Keywords;
> + std::vector<StringMatcher::StringPair> GNU, Declspec, CXX11, Keywords, Pragma;
> @@ -2515,0 +2545,2 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) {
> + else if (Variety == "Pragma")
> + Matches = &Pragma;
> @@ -2539,0 +2571,2 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) {
> + OS << " } else if (AttributeList::AS_Pragma == Syntax) {\n";
> + StringMatcher("Name", Pragma, OS).Emit();
> @@ -2645 +2678,2 @@ enum SpellingKind {
> - Keyword = 1 << 3
> + Keyword = 1 << 3,
> + Pragma = 1 << 4
> @@ -2690,4 +2724,5 @@ static void WriteDocumentation(const DocumentationData &Doc,
> - .Case("GNU", GNU)
> - .Case("CXX11", CXX11)
> - .Case("Declspec", Declspec)
> - .Case("Keyword", Keyword);
> + .Case("GNU", GNU)
> + .Case("CXX11", CXX11)
> + .Case("Declspec", Declspec)
> + .Case("Keyword", Keyword)
> + .Case("Pragma", Pragma);
> @@ -2728 +2763,2 @@ static void WriteDocumentation(const DocumentationData &Doc,
> - OS << " :header: \"GNU\", \"C++11\", \"__declspec\", \"Keyword\"\n\n";
> + OS << " :header: \"GNU\", \"C++11\", \"__declspec\", \"Keyword\",";
> + OS << " \"Pragma\"\n\n";
> @@ -2736,0 +2773,2 @@ static void WriteDocumentation(const DocumentationData &Doc,
> + OS << "\"\n\n";
> + if (SupportedSpellings & Pragma) OS << "X";
>
So one of the big reasons I am pushing back on the PragmaAttr stuff is
because there's nothing special about attributes that have a pragma
spelling. For instance, there's no reason why we shouldn't allow an
attribute to have a pragma spelling *and* a GNU spelling. As an
example, we already have attributes which are a keyword, declspec, CXX
and GNU spelling all rolled into the same attribute. By making pragmas
something different, that would make this impossible.
~Aaron
More information about the cfe-commits
mailing list