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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/include/llvm/IR/Attributes.h:484
+  iterator end() const;
+  bool empty() const { return pImpl == nullptr; }
+  bool size() const;
----------------
ahatanak wrote:
> Is it necessary to have two functions, empty and isEmpty, which have the same functionality?
I never actually ended up using this or size(), so I'll remove them. We can standardize on the STL names later.


================
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
----------------
ahatanak wrote:
> Is this change related to the changes in AttributeList?
I will rebase and it should go away. This was in my local master, but not origin/master, which arcanist diffs against.


================
Comment at: llvm/lib/IR/Attributes.cpp:843
+
+  unsigned MaxIndex = 0;
+  for (auto Pair : Attrs)
----------------
ahatanak wrote:
> 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?
You're right, I can.


https://reviews.llvm.org/D32819