[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