[PATCH] D32819: [IR] Switch AttributeList to use an array for O(1) access

[Akira Hatanaka via Phabricator via llvm-commits]

ahatanak added inline comments.


================
Comment at: llvm/include/llvm/IR/Attributes.h:484
+  iterator end() const;
+  bool empty() const { return pImpl == nullptr; }
+  bool size() const;
----------------
Is it necessary to have two functions, empty and isEmpty, which have the same functionality?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4959
       if (!EmitFuncArgumentDbgValue(Address, Variable, Expression, dl, 0, true,
                                     N)) {
-        // If variable is pinned by a alloca in dominating bb then
----------------
Is this change related to the changes in AttributeList?


================
Comment at: llvm/lib/IR/Attributes.cpp:843
+
+  unsigned MaxIndex = 0;
+  for (auto Pair : Attrs)
----------------
Since Attrs is already sorted when it's passed to this function, can you just look at the last element to find the max index?


================
Comment at: llvm/lib/IR/Attributes.cpp:895
     return AttributeList();
-  AttributeSet AS = AttributeSet::get(C, B);
-  std::pair<unsigned, AttributeSet> Arr[1] = {{Index, AS}};
-  return getImpl(C, Arr);
+  SmallVector<AttributeSet, 5> AttrSets(Index+1);
+  AttrSets[Index] = AttributeSet::get(C, B);
----------------
Space before and after +.


https://reviews.llvm.org/D32819