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

[Andrew via Phabricator via cfe-commits]

browneee added a comment.

In D77621#1968437 <https://reviews.llvm.org/D77621#1968437>, @dexonsmith wrote:

> This is thanks to a commit of mine that shaved a word off of `SmallVector`.  Some options to consider:
>
> 1. Revert to word-size integers (`size_t`? `uintptr_t`?) for Size and Capacity for small-enough types.  Could be just if `sizeof(T)==1`.  Or maybe just for `char` and `unsigned char`.
> 2. Revert my patch entirely and go back to words (these used to be `void*`).
> 3. (Your patch, stop using `SmallVector<char>`.)
>
>   I think I would prefer some variation of (1) over (3).


Hi Duncan, thanks for raising these alternatives.

Links to your prior commit for context: Review <https://reviews.llvm.org/D48518>, Commit <https://github.com/llvm-mirror/llvm/commit/604b45ddcfdc90ff66d39bc4fb7cc88f62f26499>

I agree any of these options would solve the issue I'm experiencing.

Option 1:

- I think SmallVectorBase would need to become templated.
- The size related code would need support to two sets of edge cases.
- The varying capacity may be surprising, and adds another variation to both both small mode and heap mode.

Option 3:

- This patch is somewhat widespread. A more localized fix may be desirable.
- Some inconvenience of an API change for downstream.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621