[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via llvm-commits]

serge-sans-paille added inline comments.


================
Comment at: llvm/include/llvm/IR/Attributes.h:974
+  SmallVector<Attribute> EnumAttrs;
+  SmallVector<Attribute> StringAttrs;
+  using iterator = typename SmallVector<Attribute>::iterator;
----------------
nikic wrote:
> Just wondering if storing both in one vector would make sense? Would allow reusing the normal comparison on attributes and generally simplify the code. Or is that noticeably less efficient?
Yeah, a single container is less efficient: It makes comparison more costly (as you need to check which kind of attribute we're dealing with each time), having two containers also reduces the cost of insert / remove (for low number of elements, we have higher chance to hit the container back). The lower bound alos typically takes less steps when the nature of attribute is known at compile time, which happens a lot.


================
Comment at: llvm/include/llvm/IR/Attributes.h:1125
+  }
+  std::pair<ArrayRef<Attribute>, ArrayRef<Attribute>> uniquify() const {
+    return std::make_pair(ArrayRef<Attribute>(EnumAttrs),
----------------
nikic wrote:
> What is uniquify() supposed to mean in this context?
Woops that's a leftover of a previous attempt when I was not trying to maintain the sorted invariant, I'll rename this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115798/new/

https://reviews.llvm.org/D115798