[llvm] r303654 - [IR] Switch AttributeList to use an array for O(1) access
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Tue May 23 14:35:42 PDT 2017
r303693
On Tue, May 23, 2017 at 1:59 PM, Galina Kistanova <gkistanova at gmail.com>
wrote:
> 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/Attrib
>> uteImpl.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/Attrib
>> utes.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<IndexAttrPa
>> ir>());
>> + std::copy(Sets.begin(), Sets.end(), getTrailingObjects<AttributeSe
>> t>());
>>
>> // 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/Instru
>> ctions.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/Verifi
>> er.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/Transform
>> s/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/47f916e0/attachment.html>
More information about the llvm-commits
mailing list