[PATCH] D78859: [IR] Use map for string attributes (NFC)

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/IR/AttributeImpl.h:185
 
+  DenseMap<StringRef, Attribute> StringAttrs;
+
----------------
nikic wrote:
> arsenm wrote:
> > Is StringMap any better?
> I believe StringMap is only useful if the map needs to own the string. In this case the string is already owned by the Attribute. (StringMap allocates each map entry separately, as strings are variable-length.)
> 
> There's also CachedHashStringRef that can be used here, but I don't think that's useful either, because the map is immutable, so we do not need to worry about rehashing overhead.
Won't this store the Attributes twice (once in the TrailingObjects base class and once in the DenseMap)? Would it be better to just use a StringMap here to subsume the TrailingObjects and not duplicate the Attributes? Or store a pointer to the const * value returned by the iterator?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78859