[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D77621#1968647 <https://reviews.llvm.org/D77621#1968647>, @browneee wrote:

> Do we want to increase the complexity of SmallVector somewhat? or do we want to keep the limit and affirm SmallVector is for small things?


I don't think we should limit `SmallVector` to small things.  Most `std::string` implementations also have the small storage optimization, but they're not limited to small things.  Note that even `SmallVector<T,0>` has a number of conveniences for LLVM over `std::vector` (such as extra API, ability to use `SmallVectorImpl` APIs, and no pessimizations from exception handling).

Personally, I'm fine with splitting SmallVectorBase into `SmallVectorBase<uintptr_t>` and `SmallVectorBase<uint32_t>` (on 32-bit architectures, there's actually no split there).  There aren't APIs that take a `SmallVectorBase` so there's no downside there.  It doesn't seem too bad to me to do something like:

  template <class SizeT> SmallVectorBase {
    typedef SizeT size_type;
    // ...
  };
  template <class T>
  using SmallVectorSizeType =
      std::conditional<sizeof(T) < 4, uintptr_t, uint32_t>;
  template <class T> SmallVectorImpl :
      SmallVectorBase<SmallVectorSizeType<T>> { ... };

If the complexity is too much, I would personally prefer to have my patch reverted (option 2 above) over making SmallVector stop working with large byte arrays.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621