[llvm] r303654 - [IR] Switch AttributeList to use an array for O(1) access
Galina Kistanova via llvm-commits
llvm-commits at lists.llvm.org
Tue May 23 13:59:05 PDT 2017
Hello Reid,
It look like this commit broke one of our builders:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/2524
Please have a look at this?
Thanks
Galina
On Tue, May 23, 2017 at 10:01 AM, Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: rnk
> Date: Tue May 23 12:01:48 2017
> New Revision: 303654
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303654&view=rev
> Log:
> [IR] Switch AttributeList to use an array for O(1) access
>
> Summary:
> Before this change, AttributeLists stored a pair of index and
> AttributeSet. This is memory efficient if most arguments do not have
> attributes. However, it requires doing a search over the pairs to test
> an argument or function attribute. Profiling shows that this loop was
> 0.76% of the time in 'opt -O2' of sqlite3.c, because LLVM constantly
> tests values for nullability.
>
> This was worth about 2.5% of mid-level optimization cycles on the
> sqlite3 amalgamation. Here are the full perf results:
> https://reviews.llvm.org/P7995
>
> Here are just the before and after cycle counts:
> ```
> $ perf stat -r 5 ./opt_before -O2 sqlite3.bc -o /dev/null
> 13,274,181,184 cycles # 3.047 GHz
> ( +- 0.28% )
> $ perf stat -r 5 ./opt_after -O2 sqlite3.bc -o /dev/null
> 12,906,927,263 cycles # 3.043 GHz
> ( +- 0.51% )
> ```
>
> This patch *does not* change the indices used to query attributes, as
> requested by reviewers. Tracking whether an index is usable for array
> indexing is a huge pain that affects many of the internal APIs, so it
> would be good to come back later and do a cleanup to remove this
> internal adjustment.
>
> Reviewers: pete, chandlerc
>
> Subscribers: javed.absar, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D32819
>
> Modified:
> llvm/trunk/include/llvm/IR/Attributes.h
> llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
> llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
> llvm/trunk/lib/IR/AttributeImpl.h
> llvm/trunk/lib/IR/Attributes.cpp
> llvm/trunk/lib/IR/Instructions.cpp
> llvm/trunk/lib/IR/Verifier.cpp
> llvm/trunk/lib/Transforms/Utils/FunctionComparator.cpp
>
> Modified: llvm/trunk/include/llvm/IR/Attributes.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/IR/Attributes.h?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/IR/Attributes.h (original)
> +++ llvm/trunk/include/llvm/IR/Attributes.h Tue May 23 12:01:48 2017
> @@ -322,7 +322,7 @@ template <> struct DenseMapInfo<Attribut
> /// the AttributeList object. The function attributes are at index
> /// `AttributeList::FunctionIndex', the return value is at index
> /// `AttributeList::ReturnIndex', and the attributes for the parameters
> start at
> -/// index `1'.
> +/// index `AttributeList::FirstArgIndex'.
> class AttributeList {
> public:
> enum AttrIndex : unsigned {
> @@ -347,8 +347,8 @@ public:
> /// \brief Create an AttributeList with the specified parameters in it.
> static AttributeList get(LLVMContext &C,
> ArrayRef<std::pair<unsigned, Attribute>>
> Attrs);
> - static AttributeList
> - get(LLVMContext &C, ArrayRef<std::pair<unsigned, AttributeSet>> Attrs);
> + static AttributeList get(LLVMContext &C,
> + ArrayRef<std::pair<unsigned, AttributeSet>>
> Attrs);
>
> /// \brief Create an AttributeList from attribute sets for a function,
> its
> /// return value, and all of its arguments.
> @@ -356,13 +356,11 @@ public:
> AttributeSet RetAttrs,
> ArrayRef<AttributeSet> ArgAttrs);
>
> - static AttributeList
> - getImpl(LLVMContext &C,
> - ArrayRef<std::pair<unsigned, AttributeSet>> Attrs);
> -
> private:
> explicit AttributeList(AttributeListImpl *LI) : pImpl(LI) {}
>
> + static AttributeList getImpl(LLVMContext &C, ArrayRef<AttributeSet>
> AttrSets);
> +
> public:
> AttributeList() = default;
>
> @@ -521,39 +519,31 @@ public:
> /// \brief Return the attributes at the index as a string.
> std::string getAsString(unsigned Index, bool InAttrGrp = false) const;
>
> - using iterator = ArrayRef<Attribute>::iterator;
> + //===-------------------------------------------------------
> -------------===//
> + // AttributeList Introspection
> + //===-------------------------------------------------------
> -------------===//
>
> - iterator begin(unsigned Slot) const;
> - iterator end(unsigned Slot) const;
> + typedef const AttributeSet *iterator;
> + iterator begin() const;
> + iterator end() const;
> +
> + unsigned getNumAttrSets() const;
> +
> + /// Use these to iterate over the valid attribute indices.
> + unsigned index_begin() const { return AttributeList::FunctionIndex; }
> + unsigned index_end() const { return getNumAttrSets() - 1; }
>
> /// operator==/!= - Provide equality predicates.
> bool operator==(const AttributeList &RHS) const { return pImpl ==
> RHS.pImpl; }
> bool operator!=(const AttributeList &RHS) const { return pImpl !=
> RHS.pImpl; }
>
> - //===-------------------------------------------------------
> -------------===//
> - // AttributeList Introspection
> - //===-------------------------------------------------------
> -------------===//
> -
> /// \brief Return a raw pointer that uniquely identifies this attribute
> list.
> void *getRawPointer() const {
> return pImpl;
> }
>
> /// \brief Return true if there are no attributes.
> - bool isEmpty() const {
> - return getNumSlots() == 0;
> - }
> -
> - /// \brief Return the number of slots used in this attribute list.
> This is
> - /// the number of arguments that have an attribute set on them
> (including the
> - /// function itself).
> - unsigned getNumSlots() const;
> -
> - /// \brief Return the index for the given slot.
> - unsigned getSlotIndex(unsigned Slot) const;
> -
> - /// \brief Return the attributes at the given slot.
> - AttributeSet getSlotAttributes(unsigned Slot) const;
> + bool isEmpty() const { return pImpl == nullptr; }
>
> void dump() const;
> };
>
> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Bitcode/Writer/BitcodeWriter.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Tue May 23 12:01:48
> 2017
> @@ -660,10 +660,12 @@ void ModuleBitcodeWriter::writeAttribute
>
> SmallVector<uint64_t, 64> Record;
> for (unsigned i = 0, e = Attrs.size(); i != e; ++i) {
> - const AttributeList &A = Attrs[i];
> - for (unsigned i = 0, e = A.getNumSlots(); i != e; ++i)
> - Record.push_back(
> - VE.getAttributeGroupID({A.getSlotIndex(i),
> A.getSlotAttributes(i)}));
> + AttributeList AL = Attrs[i];
> + for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) {
> + AttributeSet AS = AL.getAttributes(i);
> + if (AS.hasAttributes())
> + Record.push_back(VE.getAttributeGroupID({i, AS}));
> + }
>
> Stream.EmitRecord(bitc::PARAMATTR_CODE_ENTRY, Record);
> Record.clear();
>
> Modified: llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/
> ValueEnumerator.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp Tue May 23 12:01:48
> 2017
> @@ -902,8 +902,11 @@ void ValueEnumerator::EnumerateAttribute
> }
>
> // Do lookups for all attribute groups.
> - for (unsigned i = 0, e = PAL.getNumSlots(); i != e; ++i) {
> - IndexAndAttrSet Pair = {PAL.getSlotIndex(i),
> PAL.getSlotAttributes(i)};
> + for (unsigned i = PAL.index_begin(), e = PAL.index_end(); i != e; ++i) {
> + AttributeSet AS = PAL.getAttributes(i);
> + if (!AS.hasAttributes())
> + continue;
> + IndexAndAttrSet Pair = {i, AS};
> unsigned &Entry = AttributeGroupMap[Pair];
> if (Entry == 0) {
> AttributeGroups.push_back(Pair);
>
> Modified: llvm/trunk/lib/IR/AttributeImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> AttributeImpl.h?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/AttributeImpl.h (original)
> +++ llvm/trunk/lib/IR/AttributeImpl.h Tue May 23 12:01:48 2017
> @@ -212,27 +212,21 @@ using IndexAttrPair = std::pair<unsigned
> /// return type, and parameters.
> class AttributeListImpl final
> : public FoldingSetNode,
> - private TrailingObjects<AttributeListImpl, IndexAttrPair> {
> + private TrailingObjects<AttributeListImpl, AttributeSet> {
> friend class AttributeList;
> friend TrailingObjects;
>
> private:
> - LLVMContext &Context;
> - unsigned NumSlots; ///< Number of entries in this set.
> /// Bitset with a bit for each available attribute Attribute::AttrKind.
> uint64_t AvailableFunctionAttrs;
> + LLVMContext &Context;
> + unsigned NumAttrSets; ///< Number of entries in this set.
>
> // Helper fn for TrailingObjects class.
> - size_t numTrailingObjects(OverloadToken<IndexAttrPair>) { return
> NumSlots; }
> -
> - /// \brief Return a pointer to the IndexAttrPair for the specified slot.
> - const IndexAttrPair *getSlotPair(unsigned Slot) const {
> - return getTrailingObjects<IndexAttrPair>() + Slot;
> - }
> + size_t numTrailingObjects(OverloadToken<AttributeSet>) { return
> NumAttrSets; }
>
> public:
> - AttributeListImpl(LLVMContext &C,
> - ArrayRef<std::pair<unsigned, AttributeSet>> Slots);
> + AttributeListImpl(LLVMContext &C, ArrayRef<AttributeSet> Sets);
>
> // AttributesSetImpt is uniqued, these should not be available.
> AttributeListImpl(const AttributeListImpl &) = delete;
> @@ -243,41 +237,18 @@ public:
> /// \brief Get the context that created this AttributeListImpl.
> LLVMContext &getContext() { return Context; }
>
> - /// \brief Return the number of slots used in this attribute list. This
> is
> - /// the number of arguments that have an attribute set on them
> (including the
> - /// function itself).
> - unsigned getNumSlots() const { return NumSlots; }
> -
> - /// \brief Get the index of the given "slot" in the AttrNodes list.
> This index
> - /// is the index of the return, parameter, or function object that the
> - /// attributes are applied to, not the index into the AttrNodes list
> where the
> - /// attributes reside.
> - unsigned getSlotIndex(unsigned Slot) const {
> - return getSlotPair(Slot)->first;
> - }
> -
> - /// \brief Retrieve the attribute set node for the given "slot" in the
> - /// AttrNode list.
> - AttributeSet getSlotAttributes(unsigned Slot) const {
> - return getSlotPair(Slot)->second;
> - }
> -
> /// \brief Return true if the AttributeSet or the FunctionIndex has an
> /// enum attribute of the given kind.
> bool hasFnAttribute(Attribute::AttrKind Kind) const {
> return AvailableFunctionAttrs & ((uint64_t)1) << Kind;
> }
>
> - using iterator = AttributeSet::iterator;
> -
> - iterator begin(unsigned Slot) const {
> - return getSlotAttributes(Slot).begin();
> - }
> - iterator end(unsigned Slot) const { return
> getSlotAttributes(Slot).end(); }
> + typedef const AttributeSet *iterator;
> + iterator begin() const { return getTrailingObjects<AttributeSet>(); }
> + iterator end() const { return begin() + NumAttrSets; }
>
> void Profile(FoldingSetNodeID &ID) const;
> - static void Profile(FoldingSetNodeID &ID,
> - ArrayRef<std::pair<unsigned, AttributeSet>> Nodes);
> + static void Profile(FoldingSetNodeID &ID, ArrayRef<AttributeSet> Nodes);
>
> void dump() const;
> };
>
> Modified: llvm/trunk/lib/IR/Attributes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> Attributes.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/Attributes.cpp (original)
> +++ llvm/trunk/lib/IR/Attributes.cpp Tue May 23 12:01:48 2017
> @@ -507,7 +507,7 @@ AttributeSet AttributeSet::get(LLVMConte
> }
>
> AttributeSet AttributeSet::addAttribute(LLVMContext &C,
> - Attribute::AttrKind Kind) const {
> + Attribute::AttrKind Kind) const {
> if (hasAttribute(Kind)) return *this;
> AttrBuilder B;
> B.addAttribute(Kind);
> @@ -515,7 +515,7 @@ AttributeSet AttributeSet::addAttribute(
> }
>
> AttributeSet AttributeSet::addAttribute(LLVMContext &C, StringRef Kind,
> - StringRef Value) const {
> + StringRef Value) const {
> AttrBuilder B;
> B.addAttribute(Kind, Value);
> return addAttributes(C, AttributeSet::get(C, B));
> @@ -788,48 +788,42 @@ std::string AttributeSetNode::getAsStrin
> // AttributeListImpl Definition
> //===-------------------------------------------------------
> ---------------===//
>
> -AttributeListImpl::AttributeListImpl(
> - LLVMContext &C, ArrayRef<std::pair<unsigned, AttributeSet>> Slots)
> - : Context(C), NumSlots(Slots.size()), AvailableFunctionAttrs(0) {
> -#ifndef NDEBUG
> - assert(!Slots.empty() && "pointless AttributeListImpl");
> - if (Slots.size() >= 2) {
> - auto &PrevPair = Slots.front();
> - for (auto &CurPair : Slots.drop_front()) {
> - assert(PrevPair.first <= CurPair.first && "Attribute set not
> ordered!");
> - }
> - }
> -#endif
> +/// Map from AttributeList index to the internal array index. Adding one
> works:
> +/// FunctionIndex: ~0U -> 0
> +/// ReturnIndex: 0 -> 1
> +/// FirstArgIndex: 1.. -> 2..
> +static constexpr unsigned attrIdxToArrayIdx(unsigned Index) {
> + return Index + 1;
> +}
> +
> +AttributeListImpl::AttributeListImpl(LLVMContext &C,
> + ArrayRef<AttributeSet> Sets)
> + : AvailableFunctionAttrs(0), Context(C), NumAttrSets(Sets.size()) {
> + assert(!Sets.empty() && "pointless AttributeListImpl");
>
> // There's memory after the node where we can store the entries in.
> - std::copy(Slots.begin(), Slots.end(), getTrailingObjects<
> IndexAttrPair>());
> + std::copy(Sets.begin(), Sets.end(), getTrailingObjects<
> AttributeSet>());
>
> // Initialize AvailableFunctionAttrs summary bitset.
> static_assert(Attribute::EndAttrKinds <=
> sizeof(AvailableFunctionAttrs) * CHAR_BIT,
> "Too many attributes");
> - static_assert(AttributeList::FunctionIndex == ~0u,
> - "FunctionIndex should be biggest possible index");
> - const auto &Last = Slots.back();
> - if (Last.first == AttributeList::FunctionIndex) {
> - AttributeSet Node = Last.second;
> - for (Attribute I : Node) {
> - if (!I.isStringAttribute())
> - AvailableFunctionAttrs |= ((uint64_t)1) << I.getKindAsEnum();
> - }
> + static_assert(attrIdxToArrayIdx(AttributeList::FunctionIndex) == 0U,
> + "function should be stored in slot 0");
> + for (Attribute I : Sets[0]) {
> + if (!I.isStringAttribute())
> + AvailableFunctionAttrs |= 1ULL << I.getKindAsEnum();
> }
> }
>
> void AttributeListImpl::Profile(FoldingSetNodeID &ID) const {
> - Profile(ID, makeArrayRef(getSlotPair(0), getNumSlots()));
> + Profile(ID, makeArrayRef(begin(), end()));
> }
>
> -void AttributeListImpl::Profile(
> - FoldingSetNodeID &ID, ArrayRef<std::pair<unsigned, AttributeSet>>
> Nodes) {
> - for (const auto &Node : Nodes) {
> - ID.AddInteger(Node.first);
> - ID.AddPointer(Node.second.SetNode);
> - }
> +void AttributeListImpl::Profile(FoldingSetNodeID &ID,
> + ArrayRef<AttributeSet> Sets) {
> + for (const auto &Set : Sets)
> + ID.AddPointer(Set.SetNode);
> }
>
> #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> @@ -842,24 +836,13 @@ LLVM_DUMP_METHOD void AttributeListImpl:
> // AttributeList Construction and Mutation Methods
> //===-------------------------------------------------------
> ---------------===//
>
> -AttributeList AttributeList::getImpl(
> - LLVMContext &C, ArrayRef<std::pair<unsigned, AttributeSet>> Attrs) {
> - assert(!Attrs.empty() && "creating pointless AttributeList");
> -#ifndef NDEBUG
> - unsigned LastIndex = 0;
> - bool IsFirst = true;
> - for (auto &&AttrPair : Attrs) {
> - assert((IsFirst || LastIndex < AttrPair.first) &&
> - "unsorted or duplicate AttributeList indices");
> - assert(AttrPair.second.hasAttributes() && "pointless AttributeList
> slot");
> - LastIndex = AttrPair.first;
> - IsFirst = false;
> - }
> -#endif
> +AttributeList AttributeList::getImpl(LLVMContext &C,
> + ArrayRef<AttributeSet> AttrSets) {
> + assert(!AttrSets.empty() && "pointless AttributeListImpl");
>
> LLVMContextImpl *pImpl = C.pImpl;
> FoldingSetNodeID ID;
> - AttributeListImpl::Profile(ID, Attrs);
> + AttributeListImpl::Profile(ID, AttrSets);
>
> void *InsertPoint;
> AttributeListImpl *PA =
> @@ -870,8 +853,8 @@ AttributeList AttributeList::getImpl(
> if (!PA) {
> // Coallocate entries after the AttributeListImpl itself.
> void *Mem = ::operator new(
> - AttributeListImpl::totalSizeToAlloc<
> IndexAttrPair>(Attrs.size()));
> - PA = new (Mem) AttributeListImpl(C, Attrs);
> + AttributeListImpl::totalSizeToAlloc<AttributeSet>
> (AttrSets.size()));
> + PA = new (Mem) AttributeListImpl(C, AttrSets);
> pImpl->AttrsLists.InsertNode(PA, InsertPoint);
> }
>
> @@ -912,7 +895,7 @@ AttributeList::get(LLVMContext &C,
> AttrPairVec.emplace_back(Index, AttributeSet::get(C, AttrVec));
> }
>
> - return getImpl(C, AttrPairVec);
> + return get(C, AttrPairVec);
> }
>
> AttributeList
> @@ -922,35 +905,76 @@ AttributeList::get(LLVMContext &C,
> if (Attrs.empty())
> return AttributeList();
>
> - return getImpl(C, Attrs);
> + assert(std::is_sorted(Attrs.begin(), Attrs.end(),
> + [](const std::pair<unsigned, AttributeSet> &LHS,
> + const std::pair<unsigned, AttributeSet> &RHS) {
> + return LHS.first < RHS.first;
> + }) &&
> + "Misordered Attributes list!");
> + assert(none_of(Attrs,
> + [](const std::pair<unsigned, AttributeSet> &Pair) {
> + return !Pair.second.hasAttributes();
> + }) &&
> + "Pointless attribute!");
> +
> + unsigned MaxIndex = Attrs.back().first;
> +
> + SmallVector<AttributeSet, 4> AttrVec(attrIdxToArrayIdx(MaxIndex) + 1);
> + for (auto Pair : Attrs)
> + AttrVec[attrIdxToArrayIdx(Pair.first)] = Pair.second;
> +
> + return getImpl(C, AttrVec);
> }
>
> AttributeList AttributeList::get(LLVMContext &C, AttributeSet FnAttrs,
> AttributeSet RetAttrs,
> ArrayRef<AttributeSet> ArgAttrs) {
> - SmallVector<std::pair<unsigned, AttributeSet>, 8> AttrPairs;
> - if (RetAttrs.hasAttributes())
> - AttrPairs.emplace_back(ReturnIndex, RetAttrs);
> - size_t Index = 1;
> - for (AttributeSet AS : ArgAttrs) {
> - if (AS.hasAttributes())
> - AttrPairs.emplace_back(Index, AS);
> - ++Index;
> - }
> - if (FnAttrs.hasAttributes())
> - AttrPairs.emplace_back(FunctionIndex, FnAttrs);
> - if (AttrPairs.empty())
> + // Scan from the end to find the last argument with attributes. Most
> + // arguments don't have attributes, so it's nice if we can have fewer
> unique
> + // AttributeListImpls by dropping empty attribute sets at the end of
> the list.
> + unsigned NumSets = 0;
> + for (size_t I = ArgAttrs.size(); I != 0; --I) {
> + if (ArgAttrs[I - 1].hasAttributes()) {
> + NumSets = I + 2;
> + break;
> + }
> + }
> + if (NumSets == 0) {
> + // Check function and return attributes if we didn't have argument
> + // attributes.
> + if (RetAttrs.hasAttributes())
> + NumSets = 2;
> + else if (FnAttrs.hasAttributes())
> + NumSets = 1;
> + }
> +
> + // If all attribute sets were empty, we can use the empty attribute
> list.
> + if (NumSets == 0)
> return AttributeList();
> - return getImpl(C, AttrPairs);
> +
> + SmallVector<AttributeSet, 8> AttrSets;
> + AttrSets.reserve(NumSets);
> + // If we have any attributes, we always have function attributes.
> + AttrSets.push_back(FnAttrs);
> + if (NumSets > 1)
> + AttrSets.push_back(RetAttrs);
> + if (NumSets > 2) {
> + // Drop the empty argument attribute sets at the end.
> + ArgAttrs = ArgAttrs.take_front(NumSets - 2);
> + AttrSets.insert(AttrSets.end(), ArgAttrs.begin(), ArgAttrs.end());
> + }
> +
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::get(LLVMContext &C, unsigned Index,
> const AttrBuilder &B) {
> if (!B.hasAttributes())
> return AttributeList();
> - AttributeSet AS = AttributeSet::get(C, B);
> - std::pair<unsigned, AttributeSet> Arr[1] = {{Index, AS}};
> - return getImpl(C, Arr);
> + Index = attrIdxToArrayIdx(Index);
> + SmallVector<AttributeSet, 8> AttrSets(Index + 1);
> + AttrSets[Index] = AttributeSet::get(C, B);
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::get(LLVMContext &C, unsigned Index,
> @@ -973,32 +997,22 @@ AttributeList AttributeList::get(LLVMCon
> ArrayRef<AttributeList> Attrs) {
> if (Attrs.empty())
> return AttributeList();
> - if (Attrs.size() == 1) return Attrs[0];
> + if (Attrs.size() == 1)
> + return Attrs[0];
>
> - SmallVector<std::pair<unsigned, AttributeSet>, 8> AttrNodeVec;
> - AttributeListImpl *A0 = Attrs[0].pImpl;
> - if (A0)
> - AttrNodeVec.append(A0->getSlotPair(0), A0->getSlotPair(A0->
> getNumSlots()));
> - // Copy all attributes from Attrs into AttrNodeVec while keeping
> AttrNodeVec
> - // ordered by index. Because we know that each list in Attrs is
> ordered by
> - // index we only need to merge each successive list in rather than
> doing a
> - // full sort.
> - for (unsigned I = 1, E = Attrs.size(); I != E; ++I) {
> - AttributeListImpl *ALI = Attrs[I].pImpl;
> - if (!ALI) continue;
> - SmallVector<std::pair<unsigned, AttributeSet>, 8>::iterator
> - ANVI = AttrNodeVec.begin(), ANVE;
> - for (const IndexAttrPair *AI = ALI->getSlotPair(0),
> - *AE = ALI->getSlotPair(ALI->getNumSlots());
> - AI != AE; ++AI) {
> - ANVE = AttrNodeVec.end();
> - while (ANVI != ANVE && ANVI->first <= AI->first)
> - ++ANVI;
> - ANVI = AttrNodeVec.insert(ANVI, *AI) + 1;
> - }
> + unsigned MaxSize = 0;
> + for (AttributeList List : Attrs)
> + MaxSize = std::max(MaxSize, List.getNumAttrSets());
> +
> + SmallVector<AttributeSet, 8> NewAttrSets(MaxSize);
> + for (unsigned I = 0; I < MaxSize; ++I) {
> + AttrBuilder CurBuilder;
> + for (AttributeList List : Attrs)
> + CurBuilder.merge(List.getAttributes(I - 1));
> + NewAttrSets[I] = AttributeSet::get(C, CurBuilder);
> }
>
> - return getImpl(C, AttrNodeVec);
> + return getImpl(C, NewAttrSets);
> }
>
> AttributeList AttributeList::addAttribute(LLVMContext &C, unsigned Index,
> @@ -1022,29 +1036,19 @@ AttributeList AttributeList::addAttribut
> Attribute A) const {
> assert(std::is_sorted(Indices.begin(), Indices.end()));
>
> - unsigned I = 0, E = pImpl ? pImpl->getNumSlots() : 0;
> - SmallVector<IndexAttrPair, 4> AttrVec;
> + SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
> + unsigned MaxIndex = attrIdxToArrayIdx(Indices.back());
> + if (MaxIndex >= AttrSets.size())
> + AttrSets.resize(MaxIndex + 1);
> +
> for (unsigned Index : Indices) {
> - // Add all attribute slots before the current index.
> - for (; I < E && getSlotIndex(I) < Index; ++I)
> - AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotAttributes(I));
> -
> - // Add the attribute at this index. If we already have attributes at
> this
> - // index, merge them into a new set.
> - AttrBuilder B;
> - if (I < E && getSlotIndex(I) == Index) {
> - B.merge(AttrBuilder(pImpl->getSlotAttributes(I)));
> - ++I;
> - }
> + Index = attrIdxToArrayIdx(Index);
> + AttrBuilder B(AttrSets[Index]);
> B.addAttribute(A);
> - AttrVec.emplace_back(Index, AttributeSet::get(C, B));
> + AttrSets[Index] = AttributeSet::get(C, B);
> }
>
> - // Add remaining attributes.
> - for (; I < E; ++I)
> - AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotAttributes(I));
> -
> - return get(C, AttrVec);
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::addAttributes(LLVMContext &C, unsigned
> Index,
> @@ -1064,33 +1068,16 @@ AttributeList AttributeList::addAttribut
> "Attempt to change alignment!");
> #endif
>
> - SmallVector<IndexAttrPair, 4> AttrVec;
> - uint64_t NumAttrs = pImpl->getNumSlots();
> - unsigned I;
> -
> - // Add all the attribute slots before the one we need to merge.
> - for (I = 0; I < NumAttrs; ++I) {
> - if (getSlotIndex(I) >= Index)
> - break;
> - AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotAttributes(I));
> - }
> + Index = attrIdxToArrayIdx(Index);
> + SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
> + if (Index >= AttrSets.size())
> + AttrSets.resize(Index + 1);
> +
> + AttrBuilder Merged(AttrSets[Index]);
> + Merged.merge(B);
> + AttrSets[Index] = AttributeSet::get(C, Merged);
>
> - AttrBuilder NewAttrs;
> - if (I < NumAttrs && getSlotIndex(I) == Index) {
> - // We need to merge the attribute sets.
> - NewAttrs.merge(pImpl->getSlotAttributes(I));
> - ++I;
> - }
> - NewAttrs.merge(B);
> -
> - // Add the new or merged attribute set at this index.
> - AttrVec.emplace_back(Index, AttributeSet::get(C, NewAttrs));
> -
> - // Add the remaining entries.
> - for (; I < NumAttrs; ++I)
> - AttrVec.emplace_back(getSlotIndex(I), pImpl->getSlotAttributes(I));
> -
> - return get(C, AttrVec);
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::removeAttribute(LLVMContext &C, unsigned
> Index,
> @@ -1109,54 +1096,38 @@ AttributeList AttributeList::removeAttri
> return removeAttributes(C, Index, B);
> }
>
> -AttributeList AttributeList::removeAttributes(LLVMContext &C, unsigned
> Index,
> - const AttrBuilder &Attrs)
> const {
> +AttributeList
> +AttributeList::removeAttributes(LLVMContext &C, unsigned Index,
> + const AttrBuilder &AttrsToRemove) const {
> if (!pImpl)
> return AttributeList();
>
> // FIXME it is not obvious how this should work for alignment.
> // For now, say we can't pass in alignment, which no current use does.
> - assert(!Attrs.hasAlignmentAttr() && "Attempt to change alignment!");
> + assert(!AttrsToRemove.hasAlignmentAttr() && "Attempt to change
> alignment!");
>
> - // Add the attribute slots before the one we're trying to add.
> - SmallVector<IndexAttrPair, 4> AttrSets;
> - uint64_t NumAttrs = pImpl->getNumSlots();
> - AttrBuilder B;
> - uint64_t LastIndex = 0;
> - for (unsigned I = 0, E = NumAttrs; I != E; ++I) {
> - if (getSlotIndex(I) >= Index) {
> - if (getSlotIndex(I) == Index)
> - B = AttrBuilder(getSlotAttributes(LastIndex++));
> - break;
> - }
> - LastIndex = I + 1;
> - AttrSets.push_back({getSlotIndex(I), getSlotAttributes(I)});
> - }
> -
> - // Remove the attributes from the existing set and add them.
> - B.remove(Attrs);
> - if (B.hasAttributes())
> - AttrSets.push_back({Index, AttributeSet::get(C, B)});
> -
> - // Add the remaining attribute slots.
> - for (unsigned I = LastIndex, E = NumAttrs; I < E; ++I)
> - AttrSets.push_back({getSlotIndex(I), getSlotAttributes(I)});
> + Index = attrIdxToArrayIdx(Index);
> + SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
> + if (Index >= AttrSets.size())
> + AttrSets.resize(Index + 1);
> +
> + AttrBuilder B(AttrSets[Index]);
> + B.remove(AttrsToRemove);
> + AttrSets[Index] = AttributeSet::get(C, B);
>
> - return get(C, AttrSets);
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::removeAttributes(LLVMContext &C,
> unsigned WithoutIndex)
> const {
> if (!pImpl)
> return AttributeList();
> -
> - SmallVector<std::pair<unsigned, AttributeSet>, 4> AttrSet;
> - for (unsigned I = 0, E = pImpl->getNumSlots(); I != E; ++I) {
> - unsigned Index = getSlotIndex(I);
> - if (Index != WithoutIndex)
> - AttrSet.push_back({Index, pImpl->getSlotAttributes(I)});
> - }
> - return get(C, AttrSet);
> + WithoutIndex = attrIdxToArrayIdx(WithoutIndex);
> + if (WithoutIndex >= getNumAttrSets())
> + return *this;
> + SmallVector<AttributeSet, 4> AttrSets(this->begin(), this->end());
> + AttrSets[WithoutIndex] = AttributeSet();
> + return getImpl(C, AttrSets);
> }
>
> AttributeList AttributeList::addDereferenceableAttr(LLVMContext &C,
> @@ -1225,20 +1196,20 @@ bool AttributeList::hasFnAttribute(Strin
>
> bool AttributeList::hasParamAttribute(unsigned ArgNo,
> Attribute::AttrKind Kind) const {
> - return hasAttribute(ArgNo + 1, Kind);
> + return hasAttribute(ArgNo + FirstArgIndex, Kind);
> }
>
> bool AttributeList::hasAttrSomewhere(Attribute::AttrKind Attr,
> unsigned *Index) const {
> if (!pImpl) return false;
>
> - for (unsigned I = 0, E = pImpl->getNumSlots(); I != E; ++I)
> - for (AttributeListImpl::iterator II = pImpl->begin(I), IE =
> pImpl->end(I);
> - II != IE; ++II)
> - if (II->hasAttribute(Attr)) {
> - if (Index) *Index = pImpl->getSlotIndex(I);
> - return true;
> - }
> + for (unsigned I = index_begin(), E = index_end(); I != E; ++I) {
> + if (hasAttribute(I, Attr)) {
> + if (Index)
> + *Index = I;
> + return true;
> + }
> + }
>
> return false;
> }
> @@ -1282,60 +1253,35 @@ std::string AttributeList::getAsString(u
> }
>
> AttributeSet AttributeList::getAttributes(unsigned Index) const {
> - if (!pImpl) return AttributeSet();
> -
> - // Loop through to find the attribute node we want.
> - for (unsigned I = 0, E = pImpl->getNumSlots(); I != E; ++I)
> - if (pImpl->getSlotIndex(I) == Index)
> - return pImpl->getSlotAttributes(I);
> -
> - return AttributeSet();
> + Index = attrIdxToArrayIdx(Index);
> + if (!pImpl || Index >= getNumAttrSets())
> + return AttributeSet();
> + return pImpl->begin()[Index];
> }
>
> -AttributeList::iterator AttributeList::begin(unsigned Slot) const {
> - if (!pImpl)
> - return ArrayRef<Attribute>().begin();
> - return pImpl->begin(Slot);
> +AttributeList::iterator AttributeList::begin() const {
> + return pImpl ? pImpl->begin() : nullptr;
> }
>
> -AttributeList::iterator AttributeList::end(unsigned Slot) const {
> - if (!pImpl)
> - return ArrayRef<Attribute>().end();
> - return pImpl->end(Slot);
> +AttributeList::iterator AttributeList::end() const {
> + return pImpl ? pImpl->end() : nullptr;
> }
>
> //===-------------------------------------------------------
> ---------------===//
> // AttributeList Introspection Methods
> //===-------------------------------------------------------
> ---------------===//
>
> -unsigned AttributeList::getNumSlots() const {
> - return pImpl ? pImpl->getNumSlots() : 0;
> -}
> -
> -unsigned AttributeList::getSlotIndex(unsigned Slot) const {
> - assert(pImpl && Slot < pImpl->getNumSlots() &&
> - "Slot # out of range!");
> - return pImpl->getSlotIndex(Slot);
> -}
> -
> -AttributeSet AttributeList::getSlotAttributes(unsigned Slot) const {
> - assert(pImpl && Slot < pImpl->getNumSlots() &&
> - "Slot # out of range!");
> - return pImpl->getSlotAttributes(Slot);
> +unsigned AttributeList::getNumAttrSets() const {
> + return pImpl ? pImpl->NumAttrSets : 0;
> }
>
> #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> LLVM_DUMP_METHOD void AttributeList::dump() const {
> dbgs() << "PAL[\n";
>
> - for (unsigned i = 0, e = getNumSlots(); i < e; ++i) {
> - uint64_t Index = getSlotIndex(i);
> - dbgs() << " { ";
> - if (Index == ~0U)
> - dbgs() << "~0U";
> - else
> - dbgs() << Index;
> - dbgs() << " => " << getAsString(Index) << " }\n";
> + for (unsigned i = index_begin(), e = index_end(); i != e; ++i) {
> + if (getAttributes(i).hasAttributes())
> + dbgs() << " { " << i << " => " << getAsString(i) << " }\n";
> }
>
> dbgs() << "]\n";
> @@ -1346,26 +1292,16 @@ LLVM_DUMP_METHOD void AttributeList::dum
> // AttrBuilder Method Implementations
> //===-------------------------------------------------------
> ---------------===//
>
> +// FIXME: Remove this ctor, use AttributeSet.
> AttrBuilder::AttrBuilder(AttributeList AL, unsigned Index) {
> - AttributeListImpl *pImpl = AL.pImpl;
> - if (!pImpl) return;
> -
> - for (unsigned I = 0, E = pImpl->getNumSlots(); I != E; ++I) {
> - if (pImpl->getSlotIndex(I) != Index) continue;
> -
> - for (AttributeListImpl::iterator II = pImpl->begin(I), IE =
> pImpl->end(I);
> - II != IE; ++II)
> - addAttribute(*II);
> -
> - break;
> - }
> + AttributeSet AS = AL.getAttributes(Index);
> + for (const Attribute &A : AS)
> + addAttribute(A);
> }
>
> AttrBuilder::AttrBuilder(AttributeSet AS) {
> - if (AS.hasAttributes()) {
> - for (const Attribute &A : AS)
> - addAttribute(A);
> - }
> + for (const Attribute &A : AS)
> + addAttribute(A);
> }
>
> void AttrBuilder::clear() {
>
> Modified: llvm/trunk/lib/IR/Instructions.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> Instructions.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/Instructions.cpp (original)
> +++ llvm/trunk/lib/IR/Instructions.cpp Tue May 23 12:01:48 2017
> @@ -454,6 +454,9 @@ bool CallInst::dataOperandHasImpliedAttr
> // question is a call argument; or be indirectly implied by the kind of
> its
> // containing operand bundle, if the operand is a bundle operand.
>
> + if (i == Attribute::ReturnIndex)
> + return hasRetAttr(Kind);
> +
> // FIXME: Avoid these i - 1 calculations and update the API to use
> zero-based
> // indices.
> if (i < (getNumArgOperands() + 1))
> @@ -779,6 +782,9 @@ bool InvokeInst::dataOperandHasImpliedAt
> // question is an invoke argument; or be indirectly implied by the kind
> of its
> // containing operand bundle, if the operand is a bundle operand.
>
> + if (i == Attribute::ReturnIndex)
> + return hasRetAttr(Kind);
> +
> // FIXME: Avoid these i - 1 calculations and update the API to use
> zero-based
> // indices.
> if (i < (getNumArgOperands() + 1))
>
> Modified: llvm/trunk/lib/IR/Verifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/
> Verifier.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/IR/Verifier.cpp (original)
> +++ llvm/trunk/lib/IR/Verifier.cpp Tue May 23 12:01:48 2017
> @@ -1736,17 +1736,9 @@ void Verifier::visitConstantExpr(const C
> }
>
> bool Verifier::verifyAttributeCount(AttributeList Attrs, unsigned
> Params) {
> - if (Attrs.getNumSlots() == 0)
> - return true;
> -
> - unsigned LastSlot = Attrs.getNumSlots() - 1;
> - unsigned LastIndex = Attrs.getSlotIndex(LastSlot);
> - if (LastIndex <= Params ||
> - (LastIndex == AttributeList::FunctionIndex &&
> - (LastSlot == 0 || Attrs.getSlotIndex(LastSlot - 1) <= Params)))
> - return true;
> -
> - return false;
> + // There shouldn't be more attribute sets than there are parameters
> plus the
> + // function and return value.
> + return Attrs.getNumAttrSets() <= Params + 2;
> }
>
> /// Verify that statepoint intrinsic is well formed.
>
> Modified: llvm/trunk/lib/Transforms/Utils/FunctionComparator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/
> FunctionComparator.cpp?rev=303654&r1=303653&r2=303654&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Utils/FunctionComparator.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/FunctionComparator.cpp Tue May 23
> 12:01:48 2017
> @@ -76,12 +76,14 @@ int FunctionComparator::cmpMem(StringRef
>
> int FunctionComparator::cmpAttrs(const AttributeList L,
> const AttributeList R) const {
> - if (int Res = cmpNumbers(L.getNumSlots(), R.getNumSlots()))
> + if (int Res = cmpNumbers(L.getNumAttrSets(), R.getNumAttrSets()))
> return Res;
>
> - for (unsigned i = 0, e = L.getNumSlots(); i != e; ++i) {
> - AttributeList::iterator LI = L.begin(i), LE = L.end(i), RI =
> R.begin(i),
> - RE = R.end(i);
> + for (unsigned i = L.index_begin(), e = L.index_end(); i != e; ++i) {
> + AttributeSet LAS = L.getAttributes(i);
> + AttributeSet RAS = R.getAttributes(i);
> + AttributeSet::iterator LI = LAS.begin(), LE = LAS.end();
> + AttributeSet::iterator RI = RAS.begin(), RE = RAS.end();
> for (; LI != LE && RI != RE; ++LI, ++RI) {
> Attribute LA = *LI;
> Attribute RA = *RI;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170523/5a53416e/attachment.html>
More information about the llvm-commits
mailing list