From 6c1cc25022ad58c4c497c1a78e9c6989b91d99e7 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Sun, 26 Apr 2020 16:52:53 +0200 Subject: [PATCH] Sort EnumAttr so it matches Attribute::operator< This means AttrBuilder will always create a sorted set of attributes and we can skip the sorting step. Sorting attributes is surprisingly expensive, and I recently made it worse by making it use array_pod_sort. --- include/llvm/IR/Attributes.td | 18 ++++++++++++------ lib/IR/AttributeImpl.h | 3 +++ lib/IR/Attributes.cpp | 17 +++++++++++------ utils/TableGen/Attributes.cpp | 22 +++++++++++----------- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/include/llvm/IR/Attributes.td b/include/llvm/IR/Attributes.td index 5d4a5f6743b..ee07813b995 100644 --- a/include/llvm/IR/Attributes.td +++ b/include/llvm/IR/Attributes.td @@ -7,18 +7,24 @@ class Attr { /// Enum attribute. class EnumAttr : Attr; +/// Int attribute. +class IntAttr : Attr; + /// StringBool attribute. class StrBoolAttr : Attr; +/// Type attribute. +class TypeAttr : Attr; + /// Target-independent enum attributes. /// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias. /// 0 means unaligned (different from align(1)). -def Alignment : EnumAttr<"align">; +def Alignment : IntAttr<"align">; /// The result of the function is guaranteed to point to a number of bytes that /// we can determine if we know the value of the function's arguments. -def AllocSize : EnumAttr<"allocsize">; +def AllocSize : IntAttr<"allocsize">; /// inline=always. def AlwaysInline : EnumAttr<"alwaysinline">; @@ -31,7 +37,7 @@ def ArgMemOnly : EnumAttr<"argmemonly">; def Builtin : EnumAttr<"builtin">; /// Pass structure by value. -def ByVal : EnumAttr<"byval">; +def ByVal : TypeAttr<"byval">; /// Marks function as being in a cold path. def Cold : EnumAttr<"cold">; @@ -40,10 +46,10 @@ def Cold : EnumAttr<"cold">; def Convergent : EnumAttr<"convergent">; /// Pointer is known to be dereferenceable. -def Dereferenceable : EnumAttr<"dereferenceable">; +def Dereferenceable : IntAttr<"dereferenceable">; /// Pointer is either null or dereferenceable. -def DereferenceableOrNull : EnumAttr<"dereferenceable_or_null">; +def DereferenceableOrNull : IntAttr<"dereferenceable_or_null">; /// Function may only access memory that is inaccessible from IR. def InaccessibleMemOnly : EnumAttr<"inaccessiblememonly">; @@ -153,7 +159,7 @@ def SExt : EnumAttr<"signext">; /// Alignment of stack for function (3 bits) stored as log2 of alignment with /// +1 bias 0 means unaligned (different from alignstack=(1)). -def StackAlignment : EnumAttr<"alignstack">; +def StackAlignment : IntAttr<"alignstack">; /// Function can be speculated. def Speculatable : EnumAttr<"speculatable">; diff --git a/lib/IR/AttributeImpl.h b/lib/IR/AttributeImpl.h index 875b71bcfe2..8cea7bf03dd 100644 --- a/lib/IR/AttributeImpl.h +++ b/lib/IR/AttributeImpl.h @@ -186,6 +186,9 @@ class AttributeSetNode final AttributeSetNode(ArrayRef Attrs); + static AttributeSetNode *getSorted(LLVMContext &C, + ArrayRef SortedAttrs); + public: // AttributesSetNode is uniqued, these should not be available. AttributeSetNode(const AttributeSetNode &) = delete; diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp index 28e0505de89..40952a1d5e5 100644 --- a/lib/IR/Attributes.cpp +++ b/lib/IR/Attributes.cpp @@ -781,16 +781,21 @@ AttributeSetNode::AttributeSetNode(ArrayRef Attrs) AttributeSetNode *AttributeSetNode::get(LLVMContext &C, ArrayRef Attrs) { - if (Attrs.empty()) + SmallVector SortedAttrs(Attrs.begin(), Attrs.end()); + llvm::sort(SortedAttrs); + return getSorted(C, SortedAttrs); +} + +AttributeSetNode *AttributeSetNode::getSorted(LLVMContext &C, + ArrayRef SortedAttrs) { + if (SortedAttrs.empty()) return nullptr; - // Otherwise, build a key to look up the existing attributes. + // Build a key to look up the existing attributes. LLVMContextImpl *pImpl = C.pImpl; FoldingSetNodeID ID; - SmallVector SortedAttrs(Attrs.begin(), Attrs.end()); - llvm::sort(SortedAttrs); - + assert(llvm::is_sorted(SortedAttrs) && "Expected sorted attributes!"); for (const auto &Attr : SortedAttrs) Attr.Profile(ID); @@ -855,7 +860,7 @@ AttributeSetNode *AttributeSetNode::get(LLVMContext &C, const AttrBuilder &B) { for (const auto &TDA : B.td_attrs()) Attrs.emplace_back(Attribute::get(C, TDA.first, TDA.second)); - return get(C, Attrs); + return getSorted(C, Attrs); } bool AttributeSetNode::hasAttribute(StringRef Kind) const { diff --git a/utils/TableGen/Attributes.cpp b/utils/TableGen/Attributes.cpp index b784f53e04f..f3f875e8ce0 100644 --- a/utils/TableGen/Attributes.cpp +++ b/utils/TableGen/Attributes.cpp @@ -39,23 +39,23 @@ void Attributes::emitTargetIndependentNames(raw_ostream &OS) { OS << "#define ATTRIBUTE_ALL(FIRST, SECOND)\n"; OS << "#endif\n\n"; - auto Emiter = [&](StringRef KindName, StringRef MacroName) { - std::vector Attrs = Records.getAllDerivedDefinitions(KindName); - + auto Emit = [&](ArrayRef KindNames, StringRef MacroName) { OS << "#ifndef " << MacroName << "\n"; - OS << "#define " << MacroName << "(FIRST, SECOND) ATTRIBUTE_ALL(FIRST, " - "SECOND)\n"; + OS << "#define " << MacroName + << "(FIRST, SECOND) ATTRIBUTE_ALL(FIRST, SECOND)\n"; OS << "#endif\n\n"; - - for (auto A : Attrs) { - OS << "" << MacroName << "(" << A->getName() << "," - << A->getValueAsString("AttrString") << ")\n"; + for (StringRef KindName : KindNames) { + for (auto A : Records.getAllDerivedDefinitions(KindName)) { + OS << MacroName << "(" << A->getName() << "," + << A->getValueAsString("AttrString") << ")\n"; + } } OS << "#undef " << MacroName << "\n\n"; }; - Emiter("EnumAttr", "ATTRIBUTE_ENUM"); - Emiter("StrBoolAttr", "ATTRIBUTE_STRBOOL"); + // Emit attribute enums in the same order llvm::Attribute::operator< expects. + Emit({"EnumAttr", "TypeAttr", "IntAttr"}, "ATTRIBUTE_ENUM"); + Emit({"StrBoolAttr"}, "ATTRIBUTE_STRBOOL"); OS << "#undef ATTRIBUTE_ALL\n"; OS << "#endif\n";