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

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

dexonsmith added a comment.

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

> Thanks for the revert explanation and notes, nikic.
>
> @dexonsmith what is your current thinking on going back to the original std::vector approach?


SmallVector has only been limited to UINT32_MAX size for about a year and I think it’s a pretty major regression that I broke using it for arbitrary char buffers.  I don’t think that’s acceptable really.  Note that there was pushback when I shrank SmallVector at all for aesthetic reasons.

Note that breaking `SmallVector<char>` also breaks `SmallString` and `raw_svector_ostream` for buffers that are sometimes large.  This was certainly not the goal of my original commit and I think it’s the wrong result.

One thing to try I suppose is specializing just when `sizeof(T)==1`.  But even if there’s still a compile time hit, I think making SmallVector functional is more critical.  Use cases that really want something tiny can use `TinyPtrVector`; or if that’s not appropriate we can introduce a `TinyVector` that works for other types (could make it 8 bytes with small storage for 1 element if the type is 4 bytes or smaller).

This might be worth a thread on llvm-dev.  Maybe no one else thinks LLVM should use `SmallVectorImpl` pervasively in APIs anymore.

(AFAICT `MCRelaxableFragment` has a `SmallVector<MCFixup>` and would not have been affected by the reverted commit since `sizeof(MCFixup)` is quite large, not sure why that was brought up.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621