r197343 - Allow target-specific attributes to share a spelling between different attributes via the ParseKind field. Attributes will be given a common parsed attribute identifier (the AttributeList::AT_* enum), but retain distinct Attr subclasses.

Aaron Ballman aaron at aaronballman.com
Sun Dec 15 05:05:48 PST 2013


Author: aaronballman
Date: Sun Dec 15 07:05:48 2013
New Revision: 197343

URL: http://llvm.org/viewvc/llvm-project?rev=197343&view=rev
Log:
Allow target-specific attributes to share a spelling between different attributes via the ParseKind field. Attributes will be given a common parsed attribute identifier (the AttributeList::AT_* enum), but retain distinct Attr subclasses.

This new functionality is used to implement the ARM and MSP430 interrupt attribute.

Patch reviewed by Richard Smith over IRC.

Added:
    cfe/trunk/test/Misc/ast-dump-arm-attr.c
    cfe/trunk/test/Misc/ast-dump-msp430-attr.c
    cfe/trunk/test/Sema/attr-msp430.c
Modified:
    cfe/trunk/include/clang/Basic/Attr.td
    cfe/trunk/lib/Sema/TargetAttributesSema.cpp
    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=197343&r1=197342&r2=197343&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Sun Dec 15 07:05:48 2013
@@ -187,7 +187,22 @@ class InheritableAttr : Attr;
 /// A target-specific attribute that is meant to be processed via
 /// TargetAttributesSema::ProcessDeclAttribute.  This class is meant to be used
 /// as a mixin with InheritableAttr or Attr depending on the attribute's needs.
-class TargetSpecificAttr;
+class TargetSpecificAttr {
+  // Attributes are generally required to have unique spellings for their names
+  // so that the parser can determine what kind of attribute it has parsed.
+  // However, target-specific attributes are special in that the attribute only
+  // "exists" for a given target. So two target-specific attributes can share
+  // the same name when they exist in different targets. To support this, a
+  // Kind can be explicitly specified for a target-specific attribute. This
+  // corresponds to the AttributeList::AT_* enum that is generated and it
+  // should contain a shared value between the attributes.
+  //
+  // Target-specific attributes which use this feature should ensure that the
+  // spellings match exactly betweeen the attributes, and if the arguments or
+  // subjects differ, should specify HasCustomParsing = 1 and implement their
+  // own parsing and semantic handling requirements as-needed.
+  string ParseKind;
+}
 
 /// An inheritable parameter attribute is inherited by later
 /// redeclarations, even when it's written on a parameter.
@@ -257,11 +272,15 @@ def Annotate : InheritableParamAttr {
 }
 
 def ARMInterrupt : InheritableAttr, TargetSpecificAttr {
+  // NOTE: If you add any additional spellings, MSP430Interrupt's spellings
+  // must match.
   let Spellings = [GNU<"interrupt">];
   let Args = [EnumArgument<"Interrupt", "InterruptType",
                            ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", ""],
                            ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", "Generic"],
                            1>];
+  let ParseKind = "Interrupt";
+  let HasCustomParsing = 1;
 }
 
 def AsmLabel : InheritableAttr {
@@ -522,13 +541,12 @@ def MSABI : InheritableAttr {
 }
 
 def MSP430Interrupt : InheritableAttr, TargetSpecificAttr {
-  // FIXME: this attribute is spelled the same as the ARMInterrupt attribute,
-  // but two attributes cannot currently share the same name because of the
-  // getAttrKind function. However, in this case, the attributes are for
-  // different targets, so sharing the same name but different arguments is a
-  // reasonable design. For now, this attribute will remain having no spelling.
-  let Spellings = [];
+  // NOTE: If you add any additional spellings, ARMInterrupt's spellings must
+  // match.
+  let Spellings = [GNU<"interrupt">];
   let Args = [UnsignedArgument<"Number">];
+  let ParseKind = "Interrupt";
+  let HasCustomParsing = 1;
 }
 
 def Mips16 : InheritableAttr, TargetSpecificAttr {

Modified: cfe/trunk/lib/Sema/TargetAttributesSema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TargetAttributesSema.cpp?rev=197343&r1=197342&r2=197343&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TargetAttributesSema.cpp (original)
+++ cfe/trunk/lib/Sema/TargetAttributesSema.cpp Sun Dec 15 07:05:48 2013
@@ -61,7 +61,7 @@ namespace {
     ARMAttributesSema() { }
     bool ProcessDeclAttribute(Scope *scope, Decl *D,
                               const AttributeList &Attr, Sema &S) const {
-      if (Attr.getKind() == AttributeList::AT_ARMInterrupt) {
+      if (Attr.getKind() == AttributeList::AT_Interrupt) {
         HandleARMInterruptAttr(D, Attr, S);
         return true;
       }
@@ -72,38 +72,47 @@ namespace {
 
 static void HandleMSP430InterruptAttr(Decl *d,
                                       const AttributeList &Attr, Sema &S) {
-    // FIXME: Check for decl - it should be void ()(void).
+  if (Attr.getNumArgs() != 1) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
+      << Attr.getName() << 1;
+    return;
+  }
 
-    Expr *NumParamsExpr = static_cast<Expr *>(Attr.getArgAsExpr(0));
-    llvm::APSInt NumParams(32);
-    if (!NumParamsExpr->isIntegerConstantExpr(NumParams, S.Context)) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-        << Attr.getName() << AANT_ArgumentIntegerConstant
-        << NumParamsExpr->getSourceRange();
-      return;
-    }
+  if (!Attr.isArgExpr(0)) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName()
+      << AANT_ArgumentIntegerConstant;
+    return;
+  }
 
-    unsigned Num = NumParams.getLimitedValue(255);
-    if ((Num & 1) || Num > 30) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)
-        << "interrupt" << (int)NumParams.getSExtValue()
-        << NumParamsExpr->getSourceRange();
-      return;
-    }
+  // FIXME: Check for decl - it should be void ()(void).
+  Expr *NumParamsExpr = Attr.getArgAsExpr(0);
+  llvm::APSInt NumParams(32);
+  if (!NumParamsExpr->isIntegerConstantExpr(NumParams, S.Context)) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
+      << Attr.getName() << AANT_ArgumentIntegerConstant
+      << NumParamsExpr->getSourceRange();
+    return;
+  }
 
-    d->addAttr(::new (S.Context) MSP430InterruptAttr(Attr.getLoc(), S.Context, Num));
-    d->addAttr(::new (S.Context) UsedAttr(Attr.getLoc(), S.Context));
+  unsigned Num = NumParams.getLimitedValue(255);
+  if ((Num & 1) || Num > 30) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)
+      << "interrupt" << (int)NumParams.getSExtValue()
+      << NumParamsExpr->getSourceRange();
+    return;
   }
 
+  d->addAttr(::new (S.Context) MSP430InterruptAttr(Attr.getLoc(), S.Context, Num));
+  d->addAttr(::new (S.Context) UsedAttr(Attr.getLoc(), S.Context));
+}
+
 namespace {
   class MSP430AttributesSema : public TargetAttributesSema {
   public:
     MSP430AttributesSema() { }
     bool ProcessDeclAttribute(Scope *scope, Decl *D,
                               const AttributeList &Attr, Sema &S) const {
-      // Because this attribute has no spelling (see the FIXME in Attr.td as to
-      // why), we must check for the name instead of the attribute kind.
-      if (Attr.getName()->getName() == "interrupt") {
+      if (Attr.getKind() == AttributeList::AT_Interrupt) {
         HandleMSP430InterruptAttr(D, Attr, S);
         return true;
       }

Added: cfe/trunk/test/Misc/ast-dump-arm-attr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-arm-attr.c?rev=197343&view=auto
==============================================================================
--- cfe/trunk/test/Misc/ast-dump-arm-attr.c (added)
+++ cfe/trunk/test/Misc/ast-dump-arm-attr.c Sun Dec 15 07:05:48 2013
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple arm-apple-darwin -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
+
+__attribute__((interrupt)) void Test(void);
+// CHECK: FunctionDecl{{.*}}Test
+// CHECK-NEXT: ARMInterruptAttr

Added: cfe/trunk/test/Misc/ast-dump-msp430-attr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-msp430-attr.c?rev=197343&view=auto
==============================================================================
--- cfe/trunk/test/Misc/ast-dump-msp430-attr.c (added)
+++ cfe/trunk/test/Misc/ast-dump-msp430-attr.c Sun Dec 15 07:05:48 2013
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
+
+__attribute__((interrupt(12))) void Test(void);
+// CHECK: FunctionDecl{{.*}}Test
+// CHECK-NEXT: MSP430InterruptAttr

Added: cfe/trunk/test/Sema/attr-msp430.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-msp430.c?rev=197343&view=auto
==============================================================================
--- cfe/trunk/test/Sema/attr-msp430.c (added)
+++ cfe/trunk/test/Sema/attr-msp430.c Sun Dec 15 07:05:48 2013
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -fsyntax-only -verify %s
+
+int i;
+void f(void) __attribute__((interrupt(i))); /* expected-error {{'interrupt' attribute requires an integer constant}} */
+
+void f2(void) __attribute__((interrupt(12)));

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=197343&r1=197342&r2=197343&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Sun Dec 15 07:05:48 2013
@@ -96,6 +96,35 @@ static StringRef NormalizeAttrSpelling(S
   return AttrSpelling;
 }
 
+typedef std::vector<std::pair<std::string, Record *> > ParsedAttrMap;
+
+static ParsedAttrMap getParsedAttrList(const RecordKeeper &Records) {
+  std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
+  std::set<std::string> Seen;
+  ParsedAttrMap R;
+  for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();
+       I != E; ++I) {
+    Record &Attr = **I;
+    if (Attr.getValueAsBit("SemaHandler")) {
+      std::string AN;
+      if (Attr.isSubClassOf("TargetSpecificAttr") &&
+          !Attr.isValueUnset("ParseKind")) {
+        AN = Attr.getValueAsString("ParseKind");
+
+        // If this attribute has already been handled, it does not need to be
+        // handled again.
+        if (Seen.find(AN) != Seen.end())
+          continue;
+        Seen.insert(AN);
+      } else
+        AN = NormalizeAttrName(Attr.getName()).str();
+
+      R.push_back(std::make_pair(AN, *I));
+    }
+  }
+  return R;
+}
+
 namespace {
   class Argument {
     std::string lowerName, upperName;
@@ -1500,23 +1529,17 @@ void EmitClangAttrSpellingListIndex(Reco
                        "into internal identifiers", OS);
 
   OS <<
-    "  unsigned Index = 0;\n"
     "  switch (AttrKind) {\n"
     "  default:\n"
     "    llvm_unreachable(\"Unknown attribute kind!\");\n"
     "    break;\n";
 
-  std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
-  for (std::vector<Record*>::const_iterator I = Attrs.begin(), E = Attrs.end();
+  ParsedAttrMap Attrs = getParsedAttrList(Records);
+  for (ParsedAttrMap::const_iterator I = Attrs.begin(), E = Attrs.end();
        I != E; ++I) {
-    Record &R = **I;
-    // We only care about attributes that participate in Sema checking, so
-    // skip those attributes that are not able to make their way to Sema.
-    if (!R.getValueAsBit("SemaHandler"))
-      continue;
-
+    Record &R = *I->second;
     std::vector<Record*> Spellings = R.getValueAsListOfDefs("Spellings");
-    OS << "  case AT_" << R.getName() << " : {\n";
+    OS << "  case AT_" << I->first << ": {\n";
     for (unsigned I = 0; I < Spellings.size(); ++ I) {
       SmallString<16> Namespace;
       if (Spellings[I]->getValueAsString("Variety") == "CXX11")
@@ -1542,7 +1565,7 @@ void EmitClangAttrSpellingListIndex(Reco
   }
 
   OS << "  }\n";
-  OS << "  return Index;\n";
+  OS << "  return 0;\n";
 }
 
 // Emits the LateParsed property for attributes.
@@ -1647,23 +1670,6 @@ void EmitClangAttrTemplateInstantiate(Re
      << "} // end namespace clang\n";
 }
 
-typedef std::vector<std::pair<std::string, Record *> > ParsedAttrMap;
-
-static ParsedAttrMap getParsedAttrList(const RecordKeeper &Records) {
-  std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
-  ParsedAttrMap R;
-  for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();
-       I != E; ++I) {
-    Record &Attr = **I;
-    if (Attr.getValueAsBit("SemaHandler")) {
-      StringRef AttrName = Attr.getName();
-      AttrName = NormalizeAttrName(AttrName);
-      R.push_back(std::make_pair(AttrName.str(), *I));
-    }
-  }
-  return R;
-}
-
 // Emits the list of parsed attributes.
 void EmitClangAttrParsedAttrList(RecordKeeper &Records, raw_ostream &OS) {
   emitSourceFileHeader("List of all attributes that Clang recognizes", OS);
@@ -1996,6 +2002,7 @@ void EmitClangAttrParsedAttrKinds(Record
   std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
 
   std::vector<StringMatcher::StringPair> Matches;
+  std::set<std::string> Seen;
   for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();
        I != E; ++I) {
     Record &Attr = **I;
@@ -2005,7 +2012,16 @@ void EmitClangAttrParsedAttrKinds(Record
     if (SemaHandler || Ignored) {
       std::vector<Record*> Spellings = Attr.getValueAsListOfDefs("Spellings");
 
-      StringRef AttrName = NormalizeAttrName(StringRef(Attr.getName()));
+      std::string AttrName;
+      if (Attr.isSubClassOf("TargetSpecificAttr") &&
+          !Attr.isValueUnset("ParseKind")) {
+        AttrName = Attr.getValueAsString("ParseKind");
+        if (Seen.find(AttrName) != Seen.end())
+          continue;
+        Seen.insert(AttrName);
+      } else
+        AttrName = NormalizeAttrName(StringRef(Attr.getName())).str();
+
       for (std::vector<Record*>::const_iterator I = Spellings.begin(),
            E = Spellings.end(); I != E; ++I) {
         std::string RawSpelling = (*I)->getValueAsString("Name");
@@ -2021,7 +2037,7 @@ void EmitClangAttrParsedAttrKinds(Record
           Matches.push_back(
             StringMatcher::StringPair(
               StringRef(Spelling),
-              "return AttributeList::AT_" + AttrName.str() + ";"));
+              "return AttributeList::AT_" + AttrName + ";"));
         else
           Matches.push_back(
             StringMatcher::StringPair(





More information about the cfe-commits mailing list