[llvm] 42a6821 - [AttrBuilder] Change storage to sorted vector (NFC)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 19 03:29:13 PST 2022
Author: Nikita Popov
Date: 2022-01-19T12:29:04+01:00
New Revision: 42a68215a1291981d0f7d32c397eb64fd1ff0b11
URL: https://github.com/llvm/llvm-project/commit/42a68215a1291981d0f7d32c397eb64fd1ff0b11
DIFF: https://github.com/llvm/llvm-project/commit/42a68215a1291981d0f7d32c397eb64fd1ff0b11.diff
LOG: [AttrBuilder] Change storage to sorted vector (NFC)
This follows up on the work in D116599, which changed AttrBuilder
to store string attributes as SmallVector<Attribute>. This patch
changes the implementation to store *all* attributes as a sorted
vector.
This both makes the implementation simpler and improves compile-time.
We get a -0.5% geomean compile-time improvement on CTMark at O0.
Differential Revision: https://reviews.llvm.org/D117558
Added:
Modified:
llvm/include/llvm/IR/Attributes.h
llvm/lib/IR/Attributes.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index 9f3ed27c7a92..5e2cfe6d81ac 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -27,7 +27,6 @@
#include <bitset>
#include <cassert>
#include <cstdint>
-#include <map>
#include <set>
#include <string>
#include <utility>
@@ -981,12 +980,12 @@ class AttributeMask {
/// attribute.
bool contains(StringRef A) const { return TargetDepAttrs.count(A); }
- using td_const_iterator = decltype(TargetDepAttrs)::const_iterator;
- using td_const_range = iterator_range<td_const_iterator>;
- td_const_range td_attrs() const {
- return {TargetDepAttrs.begin(), TargetDepAttrs.end()};
+ /// Return true if the mask contains the specified attribute.
+ bool contains(Attribute A) const {
+ if (A.isStringAttribute())
+ return contains(A.getKindAsString());
+ return contains(A.getKindAsEnum());
}
- auto const &attrs() const { return Attrs; }
};
//===----------------------------------------------------------------------===//
@@ -997,13 +996,7 @@ class AttributeMask {
/// equality, presence of attributes, etc.
class AttrBuilder {
LLVMContext &Ctx;
- std::bitset<Attribute::EndAttrKinds> Attrs;
- SmallVector<Attribute, 8> TargetDepAttrs;
- std::array<uint64_t, Attribute::NumIntAttrKinds> IntAttrs = {};
- std::array<Type *, Attribute::NumTypeAttrKinds> TypeAttrs = {};
-
- Optional<unsigned> kindToIntIndex(Attribute::AttrKind Kind) const;
- Optional<unsigned> kindToTypeIndex(Attribute::AttrKind Kind) const;
+ SmallVector<Attribute, 8> Attrs;
public:
AttrBuilder(LLVMContext &Ctx) : Ctx(Ctx) {}
@@ -1019,14 +1012,7 @@ class AttrBuilder {
void clear();
/// Add an attribute to the builder.
- AttrBuilder &addAttribute(Attribute::AttrKind Val) {
- assert((unsigned)Val < Attribute::EndAttrKinds &&
- "Attribute out of range!");
- assert(Attribute::isEnumAttrKind(Val) &&
- "Adding integer/type attribute without an argument!");
- Attrs[Val] = true;
- return *this;
- }
+ AttrBuilder &addAttribute(Attribute::AttrKind Val);
/// Add the Attribute object to the builder.
AttrBuilder &addAttribute(Attribute A);
@@ -1060,21 +1046,22 @@ class AttrBuilder {
bool overlaps(const AttributeMask &AM) const;
/// Return true if the builder has the specified attribute.
- bool contains(Attribute::AttrKind A) const {
- assert((unsigned)A < Attribute::EndAttrKinds && "Attribute out of range!");
- return Attrs[A];
- }
+ bool contains(Attribute::AttrKind A) const;
/// Return true if the builder has the specified target-dependent
/// attribute.
bool contains(StringRef A) const;
/// Return true if the builder has IR-level attributes.
- bool hasAttributes() const;
+ bool hasAttributes() const { return !Attrs.empty(); }
/// Return true if the builder has an alignment attribute.
bool hasAlignmentAttr() const;
+ /// Return Attribute with the given Kind. The returned attribute will be
+ /// invalid if the Kind is not present in the builder.
+ Attribute getAttribute(Attribute::AttrKind Kind) const;
+
/// Return Attribute with the given Kind. The returned attribute will be
/// invalid if the Kind is not present in the builder.
Attribute getAttribute(StringRef Kind) const;
@@ -1204,16 +1191,7 @@ class AttrBuilder {
/// Attribute.getIntValue().
AttrBuilder &addVScaleRangeAttrFromRawRepr(uint64_t RawVScaleRangeRepr);
- // Iterators for target-dependent attributes.
- using td_const_iterator = decltype(TargetDepAttrs)::const_iterator;
- using td_const_range = iterator_range<td_const_iterator>;
-
- td_const_iterator td_begin() const { return TargetDepAttrs.begin(); }
- td_const_iterator td_end() const { return TargetDepAttrs.end(); }
-
- td_const_range td_attrs() const {
- return td_const_range(td_begin(), td_end());
- }
+ ArrayRef<Attribute> attrs() const { return Attrs; }
bool operator==(const AttrBuilder &B) const;
bool operator!=(const AttrBuilder &B) const { return !(*this == B); }
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index 6220a39a9a78..c92bacaee36d 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -815,28 +815,7 @@ AttributeSetNode *AttributeSetNode::getSorted(LLVMContext &C,
}
AttributeSetNode *AttributeSetNode::get(LLVMContext &C, const AttrBuilder &B) {
- // Add target-independent attributes.
- SmallVector<Attribute, 8> Attrs;
- for (Attribute::AttrKind Kind = Attribute::None;
- Kind != Attribute::EndAttrKinds; Kind = Attribute::AttrKind(Kind + 1)) {
- if (!B.contains(Kind))
- continue;
-
- Attribute Attr;
- if (Attribute::isTypeAttrKind(Kind))
- Attr = Attribute::get(C, Kind, B.getTypeAttr(Kind));
- else if (Attribute::isIntAttrKind(Kind))
- Attr = Attribute::get(C, Kind, B.getRawIntAttr(Kind));
- else
- Attr = Attribute::get(C, Kind);
- Attrs.push_back(Attr);
- }
-
- // Add target-dependent (string) attributes.
- for (const auto &TDA : B.td_attrs())
- Attrs.push_back(TDA);
-
- return getSorted(C, Attrs);
+ return getSorted(C, B.attrs());
}
bool AttributeSetNode::hasAttribute(StringRef Kind) const {
@@ -1538,98 +1517,92 @@ LLVM_DUMP_METHOD void AttributeList::dump() const { print(dbgs()); }
//===----------------------------------------------------------------------===//
AttrBuilder::AttrBuilder(LLVMContext &Ctx, AttributeSet AS) : Ctx(Ctx) {
- for (const auto &A : AS)
- addAttribute(A);
+ append_range(Attrs, AS);
+ assert(is_sorted(Attrs) && "AttributeSet should be sorted");
}
-void AttrBuilder::clear() {
- Attrs.reset();
- TargetDepAttrs.clear();
- IntAttrs = {};
- TypeAttrs = {};
-}
-
-Optional<unsigned>
-AttrBuilder::kindToIntIndex(Attribute::AttrKind Kind) const {
- if (Attribute::isIntAttrKind(Kind))
- return Kind - Attribute::FirstIntAttr;
- return None;
-}
-
-Optional<unsigned>
-AttrBuilder::kindToTypeIndex(Attribute::AttrKind Kind) const {
- if (Attribute::isTypeAttrKind(Kind))
- return Kind - Attribute::FirstTypeAttr;
- return None;
-}
+void AttrBuilder::clear() { Attrs.clear(); }
-struct StringAttributeComparator {
+/// Attribute comparator that only compares attribute keys. Enum attributes are
+/// sorted before string attributes.
+struct AttributeComparator {
bool operator()(Attribute A0, Attribute A1) const {
- return A0.getKindAsString() < A1.getKindAsString();
+ bool A0IsString = A0.isStringAttribute();
+ bool A1IsString = A1.isStringAttribute();
+ if (A0IsString) {
+ if (A1IsString)
+ return A0.getKindAsString() < A1.getKindAsString();
+ else
+ return false;
+ }
+ if (A1IsString)
+ return true;
+ return A0.getKindAsEnum() < A1.getKindAsEnum();
+ }
+ bool operator()(Attribute A0, Attribute::AttrKind Kind) const {
+ if (A0.isStringAttribute())
+ return false;
+ return A0.getKindAsEnum() < Kind;
}
bool operator()(Attribute A0, StringRef Kind) const {
- return A0.getKindAsString() < Kind;
+ if (A0.isStringAttribute())
+ return A0.getKindAsString() < Kind;
+ return true;
}
};
-AttrBuilder &AttrBuilder::addAttribute(Attribute Attr) {
- if (Attr.isStringAttribute()) {
- auto It = lower_bound(TargetDepAttrs, Attr, StringAttributeComparator());
- if (It != TargetDepAttrs.end() && It->hasAttribute(Attr.getKindAsString()))
- std::swap(*It, Attr);
- else
- TargetDepAttrs.insert(It, Attr);
- return *this;
- }
-
- Attribute::AttrKind Kind = Attr.getKindAsEnum();
- Attrs[Kind] = true;
+template <typename K>
+static void addAttributeImpl(SmallVectorImpl<Attribute> &Attrs, K Kind,
+ Attribute Attr) {
+ auto It = lower_bound(Attrs, Kind, AttributeComparator());
+ if (It != Attrs.end() && It->hasAttribute(Kind))
+ std::swap(*It, Attr);
+ else
+ Attrs.insert(It, Attr);
+}
- if (Optional<unsigned> TypeIndex = kindToTypeIndex(Kind))
- TypeAttrs[*TypeIndex] = Attr.getValueAsType();
- else if (Optional<unsigned> IntIndex = kindToIntIndex(Kind))
- IntAttrs[*IntIndex] = Attr.getValueAsInt();
+AttrBuilder &AttrBuilder::addAttribute(Attribute Attr) {
+ if (Attr.isStringAttribute())
+ addAttributeImpl(Attrs, Attr.getKindAsString(), Attr);
+ else
+ addAttributeImpl(Attrs, Attr.getKindAsEnum(), Attr);
+ return *this;
+}
+AttrBuilder &AttrBuilder::addAttribute(Attribute::AttrKind Kind) {
+ addAttributeImpl(Attrs, Kind, Attribute::get(Ctx, Kind));
return *this;
}
AttrBuilder &AttrBuilder::addAttribute(StringRef A, StringRef V) {
- return addAttribute(Attribute::get(Ctx, A, V));
+ addAttributeImpl(Attrs, A, Attribute::get(Ctx, A, V));
+ return *this;
}
AttrBuilder &AttrBuilder::removeAttribute(Attribute::AttrKind Val) {
assert((unsigned)Val < Attribute::EndAttrKinds && "Attribute out of range!");
- Attrs[Val] = false;
-
- if (Optional<unsigned> TypeIndex = kindToTypeIndex(Val))
- TypeAttrs[*TypeIndex] = nullptr;
- else if (Optional<unsigned> IntIndex = kindToIntIndex(Val))
- IntAttrs[*IntIndex] = 0;
-
+ auto It = lower_bound(Attrs, Val, AttributeComparator());
+ if (It != Attrs.end() && It->hasAttribute(Val))
+ Attrs.erase(It);
return *this;
}
AttrBuilder &AttrBuilder::removeAttribute(StringRef A) {
- auto It = lower_bound(TargetDepAttrs, A, StringAttributeComparator());
- if (It != TargetDepAttrs.end() && It->hasAttribute(A))
- TargetDepAttrs.erase(It);
+ auto It = lower_bound(Attrs, A, AttributeComparator());
+ if (It != Attrs.end() && It->hasAttribute(A))
+ Attrs.erase(It);
return *this;
}
uint64_t AttrBuilder::getRawIntAttr(Attribute::AttrKind Kind) const {
- Optional<unsigned> IntIndex = kindToIntIndex(Kind);
- assert(IntIndex && "Not an int attribute");
- return IntAttrs[*IntIndex];
+ assert(Attribute::isIntAttrKind(Kind) && "Not an int attribute");
+ Attribute A = getAttribute(Kind);
+ return A.isValid() ? A.getValueAsInt() : 0;
}
AttrBuilder &AttrBuilder::addRawIntAttr(Attribute::AttrKind Kind,
uint64_t Value) {
- Optional<unsigned> IntIndex = kindToIntIndex(Kind);
- assert(IntIndex && "Not an int attribute");
- assert(Value && "Value cannot be zero");
- Attrs[Kind] = true;
- IntAttrs[*IntIndex] = Value;
- return *this;
+ return addAttribute(Attribute::get(Ctx, Kind, Value));
}
std::pair<unsigned, Optional<unsigned>> AttrBuilder::getAllocSizeArgs() const {
@@ -1699,17 +1672,13 @@ AttrBuilder &AttrBuilder::addVScaleRangeAttrFromRawRepr(uint64_t RawArgs) {
}
Type *AttrBuilder::getTypeAttr(Attribute::AttrKind Kind) const {
- Optional<unsigned> TypeIndex = kindToTypeIndex(Kind);
- assert(TypeIndex && "Not a type attribute");
- return TypeAttrs[*TypeIndex];
+ assert(Attribute::isTypeAttrKind(Kind) && "Not a type attribute");
+ Attribute A = getAttribute(Kind);
+ return A.isValid() ? A.getValueAsType() : nullptr;
}
AttrBuilder &AttrBuilder::addTypeAttr(Attribute::AttrKind Kind, Type *Ty) {
- Optional<unsigned> TypeIndex = kindToTypeIndex(Kind);
- assert(TypeIndex && "Not a type attribute");
- Attrs[Kind] = true;
- TypeAttrs[*TypeIndex] = Ty;
- return *this;
+ return addAttribute(Attribute::get(Ctx, Kind, Ty));
}
AttrBuilder &AttrBuilder::addByValAttr(Type *Ty) {
@@ -1733,65 +1702,43 @@ AttrBuilder &AttrBuilder::addInAllocaAttr(Type *Ty) {
}
AttrBuilder &AttrBuilder::merge(const AttrBuilder &B) {
- for (unsigned Index = 0; Index < Attribute::NumIntAttrKinds; ++Index)
- if (B.IntAttrs[Index])
- IntAttrs[Index] = B.IntAttrs[Index];
-
- for (unsigned Index = 0; Index < Attribute::NumTypeAttrKinds; ++Index)
- if (B.TypeAttrs[Index])
- TypeAttrs[Index] = B.TypeAttrs[Index];
-
- Attrs |= B.Attrs;
-
- // TODO: could merge both lists in one loop
- for (const auto &I : B.td_attrs())
+ // TODO: Could make this O(n) as we're merging two sorted lists.
+ for (const auto &I : B.attrs())
addAttribute(I);
return *this;
}
AttrBuilder &AttrBuilder::remove(const AttributeMask &AM) {
- for (unsigned Index = 0; Index < Attribute::NumIntAttrKinds; ++Index)
- if (AM.contains((Attribute::AttrKind)Index))
- IntAttrs[Index] = 0;
-
- for (unsigned Index = 0; Index < Attribute::NumTypeAttrKinds; ++Index)
- if (AM.contains((Attribute::AttrKind)Index))
- TypeAttrs[Index] = nullptr;
-
- Attrs &= ~AM.attrs();
-
- erase_if(TargetDepAttrs,
- [&AM](Attribute A) { return AM.contains(A.getKindAsString()); });
-
+ erase_if(Attrs, [&](Attribute A) { return AM.contains(A); });
return *this;
}
bool AttrBuilder::overlaps(const AttributeMask &AM) const {
- // First check if any of the target independent attributes overlap.
- if ((Attrs & AM.attrs()).any())
- return true;
+ return any_of(Attrs, [&](Attribute A) { return AM.contains(A); });
+}
- // Then check if any target dependent ones do.
- for (const auto &I : td_attrs())
- if (AM.contains(I.getKindAsString()))
- return true;
- return false;
+Attribute AttrBuilder::getAttribute(Attribute::AttrKind A) const {
+ assert((unsigned)A < Attribute::EndAttrKinds && "Attribute out of range!");
+ auto It = lower_bound(Attrs, A, AttributeComparator());
+ if (It != Attrs.end() && It->hasAttribute(A))
+ return *It;
+ return {};
}
Attribute AttrBuilder::getAttribute(StringRef A) const {
- auto It = lower_bound(TargetDepAttrs, A, StringAttributeComparator());
- if (It != TargetDepAttrs.end() && It->hasAttribute(A))
+ auto It = lower_bound(Attrs, A, AttributeComparator());
+ if (It != Attrs.end() && It->hasAttribute(A))
return *It;
return {};
}
-bool AttrBuilder::contains(StringRef A) const {
+bool AttrBuilder::contains(Attribute::AttrKind A) const {
return getAttribute(A).isValid();
}
-bool AttrBuilder::hasAttributes() const {
- return !Attrs.none() || !TargetDepAttrs.empty();
+bool AttrBuilder::contains(StringRef A) const {
+ return getAttribute(A).isValid();
}
bool AttrBuilder::hasAlignmentAttr() const {
@@ -1799,8 +1746,7 @@ bool AttrBuilder::hasAlignmentAttr() const {
}
bool AttrBuilder::operator==(const AttrBuilder &B) const {
- return Attrs == B.Attrs && IntAttrs == B.IntAttrs &&
- TypeAttrs == B.TypeAttrs && TargetDepAttrs == B.TargetDepAttrs;
+ return Attrs == B.Attrs;
}
//===----------------------------------------------------------------------===//
More information about the llvm-commits
mailing list