[ODRHash] Drive attribute hashing through TableGen. NFC intended.

[vsapsai]

Summary

In this change deliberately keeping the tests intact to make sure
TableGen implementation works the same way as the manual one.

[erichkeane]

I dont have a concern on this in general, but does it cause problems with modules built? I would think this flag needs to be written in ASTWriter/picked back up.

[vsapsai]

This patch builds on top of D135472 and aims to show that hashing attributes with TableGen works fine and doesn't have unexpected problems.

As the next step I plan to hash more attributes. Though there is one question regarding arguments excluded from the hash, for example, deprecation message. My plan for that is to introduce a separate attribute tentatively called ExcludeFromHashingArguments. I've decided not to repeat hashable pieces for each attribute but only to exclude a few of them as I expect it provide more signal.

[vsapsai]

In D138859#3954943, @erichkeane wrote:

I dont have a concern on this in general, but does it cause problems with modules built? I would think this flag needs to be written in ASTWriter/picked back up.

For an attribute attached to a Decl, attribute's hash contributes to Decl's hash which is serialized/deserialized (D135472 has more details).

[erichkeane]

The hash there isn't the problem, its that you're adding a field to Attr.td that isn't serialized in ASTWriter/ASTReader.

[vsapsai]

In D138859#3954975, @erichkeane wrote:

The hash there isn't the problem, its that you're adding a field to Attr.td that isn't serialized in ASTWriter/ASTReader.

That's a good point. Sorry I haven't realized it at first and thanks for your patience. So, IsODRHashable is a property of the attribute kind, not the attribute instance. Unfortunately, I don't know what is the appropriate way to fix FIXME below (and how urgent it is).

// FIXME: These are properties of the attribute kind, not state for this
// instance of the attribute.
...
unsigned IsODRHashable : 1;

Anyway, big chunk of attribute deserialization happens in "AttrPCHRead.inc" and for AMDGPUFlatWorkGroupSizeAttr (non-trivial example) we have

case attr::AMDGPUFlatWorkGroupSize: {
  bool isInherited = Record.readInt();
  bool isImplicit = Record.readInt();
  bool isPackExpansion = Record.readInt();
  Expr * min = Record.readExpr();
  Expr * max = Record.readExpr();
  New = new (Context) AMDGPUFlatWorkGroupSizeAttr(Context, Info, min, max);
  cast<InheritableAttr>(New)->setInherited(isInherited);
  New->setImplicit(isImplicit);
  New->setPackExpansion(isPackExpansion);
  break;
}

which calls the generated constructor

AMDGPUFlatWorkGroupSizeAttr::AMDGPUFlatWorkGroupSizeAttr(ASTContext &Ctx, const AttributeCommonInfo &CommonInfo
              , Expr * Min
              , Expr * Max
             )
  : InheritableAttr(Ctx, CommonInfo, attr::AMDGPUFlatWorkGroupSize, false, false, false)
              , min(Min)
              , max(Max)
  {
}

where false, false, false part corresponds to bool IsLateParsed, bool IsODRHashable, bool InheritEvenIfAlreadyPresent. So IsODRHashable is never serialized/deserialized and compiler knows what the value should be. It can be a problem if one clang version writes a .pcm file and another version reads it because we can change if an attribute is hashable over time. But as far as I know, we don't support such cross-version scenario already.

[ChuanqiXu]

In D138859#3955578, @vsapsai wrote:

In D138859#3954975, @erichkeane wrote:

The hash there isn't the problem, its that you're adding a field to Attr.td that isn't serialized in ASTWriter/ASTReader.

That's a good point. Sorry I haven't realized it at first and thanks for your patience. So, IsODRHashable is a property of the attribute kind, not the attribute instance. Unfortunately, I don't know what is the appropriate way to fix FIXME below (and how urgent it is).

// FIXME: These are properties of the attribute kind, not state for this
// instance of the attribute.
...
unsigned IsODRHashable : 1;

Anyway, big chunk of attribute deserialization happens in "AttrPCHRead.inc" and for AMDGPUFlatWorkGroupSizeAttr (non-trivial example) we have

case attr::AMDGPUFlatWorkGroupSize: {
  bool isInherited = Record.readInt();
  bool isImplicit = Record.readInt();
  bool isPackExpansion = Record.readInt();
  Expr * min = Record.readExpr();
  Expr * max = Record.readExpr();
  New = new (Context) AMDGPUFlatWorkGroupSizeAttr(Context, Info, min, max);
  cast<InheritableAttr>(New)->setInherited(isInherited);
  New->setImplicit(isImplicit);
  New->setPackExpansion(isPackExpansion);
  break;
}

which calls the generated constructor

AMDGPUFlatWorkGroupSizeAttr::AMDGPUFlatWorkGroupSizeAttr(ASTContext &Ctx, const AttributeCommonInfo &CommonInfo
              , Expr * Min
              , Expr * Max
             )
  : InheritableAttr(Ctx, CommonInfo, attr::AMDGPUFlatWorkGroupSize, false, false, false)
              , min(Min)
              , max(Max)
  {
}

where false, false, false part corresponds to bool IsLateParsed, bool IsODRHashable, bool InheritEvenIfAlreadyPresent. So IsODRHashable is never serialized/deserialized and compiler knows what the value should be. It can be a problem if one clang version writes a .pcm file and another version reads it because we can change if an attribute is hashable over time. But as far as I know, we don't support such cross-version scenario already.

The explanation makes sense to me.

And I think we should add tests for this. e.g., in the ASTImporterTests.cpp.

clang/lib/AST/AttrImpl.cpp

16

Is this change necessary?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

552–553

Do we want to change the default for any of the derived classes? e.g., should something that is a TypeAttr or DeclOrTypeAttr default to being ODR hashable because they impact the type system?

Also, can we expand the comment somewhat to help folks understand the circumstances under which they should set that to 1?

[vsapsai]

In D138859#3955806, @ChuanqiXu wrote:

And I think we should add tests for this. e.g., in the ASTImporterTests.cpp.

I'm not sure which test would be useful in this case. We kinda already test that different modules agree which attributes should be ODR hashable. That is enforced by detecting attribute mismatches in odr_hash.cpp (if attributes aren't hashable - no mismatches).

clang/include/clang/Basic/Attr.td

552–553

At this point I don't know what the default value should be, for that I'll need to see how many attributes should be hashable. This change is mostly to prove the direction is viable and reasonable. And TypeAttr, DeclOrTypeAttr is a good starting point to decide which attributes should be hashed.

Good idea with expanding the comment. Though I need some time to prepare non-abhorrent explanation (gosh, docs are harder than code).

clang/lib/AST/AttrImpl.cpp

16

Yep. AttrImpl.cpp includes "clang/AST/AttrImpl.inc" where, for example, we have

void AlignValueAttr::processODRHash(
    llvm::FoldingSetNodeID &ID, ODRHash &Hash) const {
  ID.AddInteger(Attr::getKind());
  const Expr *AlignmentExpr = getAlignment();
  ID.AddBoolean(AlignmentExpr);
  if (AlignmentExpr)
    Hash.AddStmt(AlignmentExpr);
}

and we need the full declaration of ODRHash to call ODRHash::AddStmt.

242

AttrImpl.inc include I've mentioned.

[ChuanqiXu]

In D138859#3958958, @vsapsai wrote:

In D138859#3955806, @ChuanqiXu wrote:

And I think we should add tests for this. e.g., in the ASTImporterTests.cpp.

I'm not sure which test would be useful in this case. We kinda already test that different modules agree which attributes should be ODR hashable. That is enforced by detecting attribute mismatches in odr_hash.cpp (if attributes aren't hashable - no mismatches).

Yeah, I admit it is redundant from some perspective. I think the lit tests are different with the unittest. The lit tests check if the end behavior is expected. And the unittest tests checks if the status of the attribute is expected. For example, it is possible to discard the changes in this patch and keep these lit tests passes.

I think more tests are always good. Especially now it is not hard to add these tests.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

552–553

Okay, I'm fine being conservative until we figure out what the right default is.

clang/include/clang/AST/Attr.h

protected:
   /// An index into the spelling list of an
   /// attribute defined in Attr.td file.
   unsigned Inherited : 1;
   unsigned IsPackExpansion : 1;
   unsigned Implicit : 1;
   // FIXME: These are properties of the attribute kind, not state for this
   // instance of the attribute.
   unsigned IsLateParsed : 1;
+  unsigned IsODRHashable : 1;
   unsigned InheritEvenIfAlreadyPresent : 1;
 
   void *operator new(size_t bytes) noexcept {
     llvm_unreachable("Attrs cannot be allocated with regular 'new'.");
   }
   void operator delete(void *data) noexcept {
     llvm_unreachable("Attrs cannot be released with regular 'delete'.");
   }
 
 public:
   // Forward so that the regular new and delete do not hide global ones.
   void *operator new(size_t Bytes, ASTContext &C,
                      size_t Alignment = 8) noexcept {
     return ::operator new(Bytes, C, Alignment);
   }
   void operator delete(void *Ptr, ASTContext &C, size_t Alignment) noexcept {
     return ::operator delete(Ptr, C, Alignment);
   }
 
 protected:
   Attr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-       attr::Kind AK, bool IsLateParsed)
+       attr::Kind AK, bool IsLateParsed, bool IsODRHashable)
       : AttributeCommonInfo(CommonInfo), AttrKind(AK), Inherited(false),
         IsPackExpansion(false), Implicit(false), IsLateParsed(IsLateParsed),
-        InheritEvenIfAlreadyPresent(false) {}
+        IsODRHashable(IsODRHashable), InheritEvenIfAlreadyPresent(false) {}
 
 public:
   attr::Kind getKind() const { return static_cast<attr::Kind>(AttrKind); }
 
   unsigned getSpellingListIndex() const {
     return getAttributeSpellingListIndex();
   }
   const char *getSpelling() const;
public:
   void setPackExpansion(bool PE) { IsPackExpansion = PE; }
   bool isPackExpansion() const { return IsPackExpansion; }
 
   // Clone this attribute.
   Attr *clone(ASTContext &C) const;
 
   bool isLateParsed() const { return IsLateParsed; }
 
+  bool isODRHashable() const { return IsODRHashable; }
+
   // Pretty print this attribute.
   void printPretty(raw_ostream &OS, const PrintingPolicy &Policy) const;
 
+  /// Hash the attribute and its salient arguments.
+  ///
+  /// It is an implementation detail and for obtaining a hash you should use
+  /// ODRHash, specifically ODRHash::AddAttr.
+  void processODRHash(llvm::FoldingSetNodeID &ID, class ODRHash &Hash) const;
+
   static StringRef getDocumentation(attr::Kind);
 };
 
 class TypeAttr : public Attr {
 protected:
   TypeAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-           attr::Kind AK, bool IsLateParsed)
-      : Attr(Context, CommonInfo, AK, IsLateParsed) {}
+           attr::Kind AK, bool IsLateParsed, bool IsODRHashable)
+      : Attr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable) {}
 
 public:
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstTypeAttr &&
            A->getKind() <= attr::LastTypeAttr;
   }
 };
 
 class StmtAttr : public Attr {
 protected:
   StmtAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-           attr::Kind AK, bool IsLateParsed)
-      : Attr(Context, CommonInfo, AK, IsLateParsed) {}
+           attr::Kind AK, bool IsLateParsed, bool IsODRHashable)
+      : Attr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable) {}
 
 public:
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstStmtAttr &&
            A->getKind() <= attr::LastStmtAttr;
   }
 };
 
 class InheritableAttr : public Attr {
 protected:
   InheritableAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-                  attr::Kind AK, bool IsLateParsed,
+                  attr::Kind AK, bool IsLateParsed, bool IsODRHashable,
                   bool InheritEvenIfAlreadyPresent)
-      : Attr(Context, CommonInfo, AK, IsLateParsed) {
+      : Attr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable) {
     this->InheritEvenIfAlreadyPresent = InheritEvenIfAlreadyPresent;
   }
 
 public:
   void setInherited(bool I) { Inherited = I; }
 
   /// Should this attribute be inherited from a prior declaration even if it's
   /// explicitly provided in the current declaration?
   bool shouldInheritEvenIfAlreadyPresent() const {
     return InheritEvenIfAlreadyPresent;
   }
 
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstInheritableAttr &&
            A->getKind() <= attr::LastInheritableAttr;
   }
 };
 
 class DeclOrStmtAttr : public InheritableAttr {
 protected:
   DeclOrStmtAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-                 attr::Kind AK, bool IsLateParsed,
+                 attr::Kind AK, bool IsLateParsed, bool IsODRHashable,
                  bool InheritEvenIfAlreadyPresent)
-      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed,
+      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable,
                         InheritEvenIfAlreadyPresent) {}
 
 public:
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstDeclOrStmtAttr &&
            A->getKind() <= attr::LastDeclOrStmtAttr;
   }
 };
 
 class InheritableParamAttr : public InheritableAttr {
 protected:
   InheritableParamAttr(ASTContext &Context,
                        const AttributeCommonInfo &CommonInfo, attr::Kind AK,
-                       bool IsLateParsed, bool InheritEvenIfAlreadyPresent)
-      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed,
+                       bool IsLateParsed, bool IsODRHashable,
+                       bool InheritEvenIfAlreadyPresent)
+      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable,
                         InheritEvenIfAlreadyPresent) {}
 
 public:
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstInheritableParamAttr &&
            A->getKind() <= attr::LastInheritableParamAttr;
   }
 };
 
 class HLSLAnnotationAttr : public InheritableAttr {
 protected:
   HLSLAnnotationAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-                     attr::Kind AK, bool IsLateParsed,
+                     attr::Kind AK, bool IsLateParsed, bool IsODRHashable,
                      bool InheritEvenIfAlreadyPresent)
-      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed,
+      : InheritableAttr(Context, CommonInfo, AK, IsLateParsed, IsODRHashable,
                         InheritEvenIfAlreadyPresent) {}
 
 public:
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstHLSLAnnotationAttr &&
            A->getKind() <= attr::LastHLSLAnnotationAttr;
   }
 };
 
 /// A parameter attribute which changes the argument-passing ABI rule
 /// for the parameter.
 class ParameterABIAttr : public InheritableParamAttr {
 protected:
   ParameterABIAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
-                   attr::Kind AK, bool IsLateParsed,
+                   attr::Kind AK, bool IsLateParsed, bool IsODRHashable,
                    bool InheritEvenIfAlreadyPresent)
       : InheritableParamAttr(Context, CommonInfo, AK, IsLateParsed,
-                             InheritEvenIfAlreadyPresent) {}
+                             IsODRHashable, InheritEvenIfAlreadyPresent) {}
 
 public:
   ParameterABI getABI() const {
     switch (getKind()) {
     case attr::SwiftContext:
       return ParameterABI::SwiftContext;
     case attr::SwiftAsyncContext:
       return ParameterABI::SwiftAsyncContext;

clang/include/clang/Basic/Attr.td

class Attr {
   // The things to which an attribute can appertain
   SubjectList Subjects;
   // The arguments allowed on an attribute
   list<Argument> Args = [];
   // Accessors which should be generated for the attribute.
   list<Accessor> Accessors = [];
   // Set to true for attributes with arguments which require delayed parsing.
   bit LateParsed = 0;
+  // Set to true for attributes that should participate in ODR hashing.
+  bit IsODRHashable = 0;
   // Set to false to prevent an attribute from being propagated from a template
   // to the instantiation.
   bit Clone = 1;
   // Set to true for attributes which must be instantiated within templates
   bit TemplateDependent = 0;
   // Set to true for attributes that have a corresponding AST node.
   bit ASTNode = 1;
   // Set to true for attributes which have handler in Sema.

 // Attributes begin here
 //
 
 def AbiTag : Attr {
   let Spellings = [GCC<"abi_tag", /*AllowInC*/0>];
   let Args = [VariadicStringArgument<"Tags">];
   let Subjects = SubjectList<[Struct, Var, Function, Namespace], ErrorDiag>;
   let MeaningfulToClassTemplateDefinition = 1;
+  let IsODRHashable = 1;
   let Documentation = [AbiTagsDocs];
 }
 
 def AddressSpace : TypeAttr {
   let Spellings = [Clang<"address_space">];
   let Args = [IntArgument<"AddressSpace">];
   let Documentation = [Undocumented];
 }
def Aligned : InheritableAttr {
   let Spellings = [GCC<"aligned">, Declspec<"align">, Keyword<"alignas">,
                    Keyword<"_Alignas">];
   let Args = [AlignedArgument<"Alignment", 1>];
   let Accessors = [Accessor<"isGNU", [GCC<"aligned">]>,
                    Accessor<"isC11", [Keyword<"_Alignas">]>,
                    Accessor<"isAlignas", [Keyword<"alignas">,
                                           Keyword<"_Alignas">]>,
                    Accessor<"isDeclspec",[Declspec<"align">]>];
+  let IsODRHashable = 1;
   let Documentation = [Undocumented];
 }
 
 def AlignValue : Attr {
   let Spellings = [
     // Unfortunately, this is semantically an assertion, not a directive
     // (something else must ensure the alignment), so aligned_value is a
     // probably a better name. We might want to add an aligned_value spelling in
     // the future (and a corresponding C++ attribute), but this can be done
     // later once we decide if we also want them to have slightly-different
     // semantics than Intel's align_value.
     //
     // Does not get a [[]] spelling because the attribute is not exposed as such
     // by Intel.
     GNU<"align_value">
     // Intel's compiler on Windows also supports:
     // , Declspec<"align_value">
   ];
   let Args = [ExprArgument<"Alignment">];
   let Subjects = SubjectList<[Var, TypedefName]>;
+  let IsODRHashable = 1;
   let Documentation = [AlignValueDocs];
 }
 
 def AlignMac68k : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];
   let SemaHandler = 0;
   let Documentation = [InternalOnly];
def FlagEnum : InheritableAttr {
   let SimpleHandler = 1;
 }
 
 def EnumExtensibility : InheritableAttr {
   let Spellings = [Clang<"enum_extensibility">];
   let Subjects = SubjectList<[Enum]>;
   let Args = [EnumArgument<"Extensibility", "Kind",
               ["closed", "open"], ["Closed", "Open"]>];
+  let IsODRHashable = 1;
   let Documentation = [EnumExtensibilityDocs];
 }
 
 def Flatten : InheritableAttr {
   let Spellings = [GCC<"flatten">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [FlattenDocs];
   let SimpleHandler = 1;
def Mode : Attr {
   // This is notionally a type attribute, which #pragma clang attribute
   // generally does not support.
   let PragmaAttributeSupport = 0;
 }
 
 def Naked : InheritableAttr {
   let Spellings = [GCC<"naked">, Declspec<"naked">];
   let Subjects = SubjectList<[Function]>;
+  let IsODRHashable = 1;
   let Documentation = [Undocumented];
 }
 
 def NeonPolyVectorType : TypeAttr {
   let Spellings = [Clang<"neon_polyvector_type">];
   let Args = [IntArgument<"NumElements">];
   let Documentation = [Undocumented];
   // Represented as VectorType instead.
def ObjCBoxable : Attr {
   let Spellings = [Clang<"objc_boxable">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [ObjCBoxableDocs];
 }
 
 def OptimizeNone : InheritableAttr {
   let Spellings = [Clang<"optnone">];
   let Subjects = SubjectList<[Function, ObjCMethod]>;
+  let IsODRHashable = 1;
   let Documentation = [OptnoneDocs];
 }
 
 def Overloadable : Attr {
   let Spellings = [Clang<"overloadable">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [OverloadableDocs];
   let SimpleHandler = 1;
let Args = [IdentifierArgument<"Module">,
               VariadicParamIdxArgument<"Args">];
   let Subjects = SubjectList<[HasFunctionProto]>;
   let Documentation = [Undocumented];
 }
 
 def Packed : InheritableAttr {
   let Spellings = [GCC<"packed">];
 //  let Subjects = [Tag, Field];
+  let IsODRHashable = 1;
   let Documentation = [Undocumented];
 }
 
 def IntelOclBicc : DeclOrTypeAttr {
   let Spellings = [Clang<"intel_ocl_bicc", 0>];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [Undocumented];
 }
def ObjCRequiresPropertyDefs : InheritableAttr {
   let SimpleHandler = 1;
 }
 
 def Unused : InheritableAttr {
   let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">,
                    C2x<"", "maybe_unused", 202106>];
   let Subjects = SubjectList<[Var, ObjCIvar, Type, Enum, EnumConstant, Label,
                               Field, ObjCMethod, FunctionLike]>;
+  let IsODRHashable = 1;
   let Documentation = [WarnMaybeUnusedDocs];
 }
 
 def Used : InheritableAttr {
   let Spellings = [GCC<"used">];
   let Subjects = SubjectList<[NonLocalVar, Function, ObjCMethod]>;
   let Documentation = [UsedDocs];
   let SimpleHandler = 1;

clang/lib/AST/AttrImpl.cpp

 //===--- AttrImpl.cpp - Classes for representing attributes -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file contains out-of-line methods for Attr classes.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ODRHash.h"
 #include "clang/AST/Type.h"
 using namespace clang;
 
 void LoopHintAttr::printPrettyPragma(raw_ostream &OS,
                                      const PrintingPolicy &Policy) const {
   unsigned SpellingIndex = getAttributeSpellingListIndex();
   // For "#pragma unroll" and "#pragma nounroll" the string "unroll" or
   // "nounroll" is already emitted as the pragma name.
void OMPDeclareVariantAttr::printPrettyPragma(
   };
   if (appendArgs_size()) {
     OS << " append_args(";
     PrintInteropInfo(appendArgs_begin(), appendArgs_end());
     OS << ")";
   }
 }
 
 #include "clang/AST/AttrImpl.inc"

clang/lib/AST/ODRHash.cpp

 
 void ODRHash::getHashableAttrs(const Decl *D,
                                SmallVectorImpl<const Attr *> &HashableAttrs) {
   HashableAttrs.clear();
   if (!D->hasAttrs())
     return;
 
   llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
-                [](const Attr *A) { return !A->isImplicit(); });
+                [](const Attr *A) { return A->isODRHashable(); });
 }
 
 void ODRHash::AddAttrs(const Decl *D) {
   llvm::SmallVector<const Attr *, 2> Attrs;
   getHashableAttrs(D, Attrs);
   ID.AddInteger(Attrs.size());
   for (const Attr *A : Attrs)
     AddAttr(A);
 }
 
 void ODRHash::AddAttr(const Attr *A) {
-  ID.AddInteger(A->getKind());
-
-  // FIXME: This should be auto-generated as part of Attr.td
-  switch (A->getKind()) {
-  case attr::Aligned: {
-    auto *M = cast<AlignedAttr>(A);
-    ID.AddBoolean(M->isAlignmentExpr());
-    if (M->isAlignmentExpr()) {
-      Expr *AlignmentExpr = M->getAlignmentExpr();
-      ID.AddBoolean(AlignmentExpr);
-      if (AlignmentExpr)
-        AddStmt(AlignmentExpr);
-    } else {
-      ID.AddString(M->getAlignmentType()->getType().getAsString());
-    }
-    break;
-  }
-  case attr::AlignValue: {
-    auto *M = cast<AlignValueAttr>(A);
-    Expr *AlignmentExpr = M->getAlignment();
-    ID.AddBoolean(AlignmentExpr);
-    if (AlignmentExpr)
-      AddStmt(AlignmentExpr);
-    break;
-  }
-  case attr::EnumExtensibility: {
-    auto *M = cast<EnumExtensibilityAttr>(A);
-    ID.AddInteger(M->getExtensibility());
-    break;
-  }
-  default:
-    break;
-  }
+  A->processODRHash(ID, *this);
 }
 
 void ODRHash::AddSubDecl(const Decl *D) {
   assert(D && "Expecting non-null pointer.");
 
   ODRDeclVisitor(ID, *this).Visit(D);
 }
 

clang/utils/TableGen/ClangAttrEmitter.cpp

public:
     virtual void writePCHReadArgs(raw_ostream &OS) const = 0;
     virtual void writePCHReadDecls(raw_ostream &OS) const = 0;
     virtual void writePCHWrite(raw_ostream &OS) const = 0;
     virtual std::string getIsOmitted() const { return "false"; }
     virtual void writeValue(raw_ostream &OS) const = 0;
     virtual void writeDump(raw_ostream &OS) const = 0;
     virtual void writeDumpChildren(raw_ostream &OS) const {}
     virtual void writeHasChildren(raw_ostream &OS) const { OS << "false"; }
+    virtual void writeODRHashing(raw_ostream &OS) const {}
 
     virtual bool isEnumArg() const { return false; }
     virtual bool isVariadicEnumArg() const { return false; }
     virtual bool isVariadic() const { return false; }
 
     virtual void writeImplicitCtorArgs(raw_ostream &OS) const {
       OS << getUpperName();
     }
public:
     void writeDumpChildren(raw_ostream &OS) const override {
       OS << "    if (SA->is" << getUpperName() << "Expr())\n";
       OS << "      Visit(SA->get" << getUpperName() << "Expr());\n";
     }
 
     void writeHasChildren(raw_ostream &OS) const override {
       OS << "SA->is" << getUpperName() << "Expr()";
     }
+
+    void writeODRHashing(raw_ostream &OS) const override {
+      OS << "  ID.AddBoolean(is" << getUpperName() << "Expr());\n";
+      OS << "  if (is" << getUpperName() << "Expr()) {\n";
+      OS << "    const Expr *" << getUpperName() << "Expr = "
+         << "get" << getUpperName() << "Expr();\n";
+      OS << "    ID.AddBoolean(" << getUpperName() << "Expr);\n";
+      OS << "    if (" << getUpperName() << "Expr)\n"
+         << "      Hash.AddStmt(" << getUpperName() << "Expr);\n";
+      OS << "  } else {\n";
+      OS << "    ID.AddString(get" << getUpperName()
+         << "Type()->getType().getAsString());\n";
+      OS << "  }\n";
+    }
   };
 
   class VariadicArgument : public Argument {
     std::string Type, ArgName, ArgSizeName, RangeName;
 
   protected:
     // Assumed to receive a parameter: raw_ostream OS.
     virtual void writeValueImpl(raw_ostream &OS) const {
void writeConversion(raw_ostream &OS, bool Header) const {
         if (Uniques.insert(enums[I]).second)
           OS << "  case " << getAttrName() << "Attr::" << enums[I]
              << ": return \"" << values[I] << "\";\n";
       }
       OS << "  }\n"
          << "  llvm_unreachable(\"No enumerator with that value\");\n"
          << "}\n";
     }
+
+    void writeODRHashing(raw_ostream &OS) const override {
+      OS << "  ID.AddInteger(get" << getUpperName() << "());\n";
+    }
   };
 
   class VariadicEnumArgument: public VariadicArgument {
     std::string type, QualifiedTypeName;
     std::vector<StringRef> values, enums, uniques;
 
   protected:
     void writeValueImpl(raw_ostream &OS) const override {
public:
 
     void writeDump(raw_ostream &OS) const override {}
 
     void writeDumpChildren(raw_ostream &OS) const override {
       OS << "    Visit(SA->get" << getUpperName() << "());\n";
     }
 
     void writeHasChildren(raw_ostream &OS) const override { OS << "true"; }
+
+    void writeODRHashing(raw_ostream &OS) const override {
+      OS << "  const Expr *" << getUpperName() << "Expr = "
+         << "get" << getUpperName() << "();\n";
+      OS << "  ID.AddBoolean(" << getUpperName() << "Expr);\n";
+      OS << "  if (" << getUpperName() << "Expr)\n"
+         << "    Hash.AddStmt(" << getUpperName() << "Expr);\n";
+    }
   };
 
   class VariadicExprArgument : public VariadicArgument {
   public:
     VariadicExprArgument(const Record &Arg, StringRef Attr)
       : VariadicArgument(Arg, Attr, "Expr *")
     {}
 
writePrettyPrintFunction(const Record &R,
   }
 
   // End of the switch statement.
   OS << "}\n";
   // End of the print function.
   OS << "}\n\n";
 }
 
+static void
+writeProcessODRHashFunction(const Record &R,
+                            const std::vector<std::unique_ptr<Argument>> &Args,
+                            raw_ostream &OS) {
+  OS << "void " << R.getName() << "Attr::processODRHash(\n"
+     << "    llvm::FoldingSetNodeID &ID, ODRHash &Hash) const {\n";
+  if (!R.getValueAsBit("IsODRHashable")) {
+    OS << "  // Not ODR-hashable\n}\n\n";
+    return;
+  }
+
+  OS << "  ID.AddInteger(Attr::getKind());\n";
+  for (const auto &Arg : Args) {
+    if (Arg->isFake())
+      continue;
+    Arg->writeODRHashing(OS);
+  }
+  // End of the processODRHash function.
+  OS << "}\n\n";
+}
+
 /// Return the index of a spelling in a spelling list.
 static unsigned
 getSpellingListIndex(const std::vector<FlattenedSpelling> &SpellingList,
                      const FlattenedSpelling &Spelling) {
   assert(!SpellingList.empty() && "Spelling list is empty!");
 
   for (unsigned Index = 0; Index < SpellingList.size(); ++Index) {
     const FlattenedSpelling &S = SpellingList[Index];
auto emitCtor = [&](bool emitOpt, bool emitFake, bool emitNoArgs) {
 
       OS << "             )";
       if (Header) {
         OS << ";\n";
         return;
       }
       OS << "\n  : " << SuperName << "(Ctx, CommonInfo, ";
       OS << "attr::" << R.getName() << ", "
-         << (R.getValueAsBit("LateParsed") ? "true" : "false");
+         << (R.getValueAsBit("LateParsed") ? "true" : "false") << ", "
+         << (R.getValueAsBit("IsODRHashable") ? "true" : "false");
       if (Inheritable) {
         OS << ", "
            << (R.getValueAsBit("InheritEvenIfAlreadyPresent") ? "true"
                                                               : "false");
       }
       OS << ")\n";
 
       for (auto const &ai : Args) {
if (DelayedArgs && HasRequiredArgs)
       emitCtor(false, false, true);
 
     if (Header) {
       OS << '\n';
       OS << "  " << R.getName() << "Attr *clone(ASTContext &C) const;\n";
       OS << "  void printPretty(raw_ostream &OS,\n"
          << "                   const PrintingPolicy &Policy) const;\n";
       OS << "  const char *getSpelling() const;\n";
+      OS << "  void processODRHash(llvm::FoldingSetNodeID &ID,\n"
+         << "                      class ODRHash &Hash) const;\n";
     }
 
     if (!ElideSpelling) {
       assert(!SemanticToSyntacticMap.empty() && "Empty semantic mapping list");
       if (Header)
         OS << "  Spelling getSemanticSpelling() const;\n";
       else {
         OS << R.getName() << "Attr::Spelling " << R.getName()
if (Header) {
         OS << "  A->setDelayedArgs(C, ";
         DelayedArgs->writeCloneArgs(OS);
         OS << ");\n";
       }
       OS << "  return A;\n}\n\n";
 
       writePrettyPrintFunction(R, Args, OS);
       writeGetSpellingFunction(R, OS);
+      writeProcessODRHashFunction(R, Args, OS);
     }
   }
 }
 // Emits the class definitions for attributes.
 void clang::EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
   emitSourceFileHeader("Attribute classes' definitions", OS);
 
   OS << "#ifndef LLVM_CLANG_ATTR_CLASSES_INC\n";
void clang::EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
   EmitFunc("getSpelling()");
 
   OS << "Attr *Attr::clone(ASTContext &C) const {\n";
   EmitFunc("clone(C)");
 
   OS << "void Attr::printPretty(raw_ostream &OS, "
         "const PrintingPolicy &Policy) const {\n";
   EmitFunc("printPretty(OS, Policy)");
+
+  OS << "void Attr::processODRHash(llvm::FoldingSetNodeID &ID, "
+     << "class ODRHash &Hash) const {";
+  EmitFunc("processODRHash(ID, Hash)");
 }
 
 static void emitAttrList(raw_ostream &OS, StringRef Class,
                          const std::vector<Record*> &AttrList) {
   for (auto Cur : AttrList) {
     OS << Class << "(" << Cur->getName() << ")\n";
   }
 }