[PATCH] D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers.

[Sean Silva via Phabricator via llvm-commits]

silvas added inline comments.


================
Comment at: llvm/docs/ProgrammersManual.rst:1517
+* ``Vec<T>`` should be used for vectors on the heap. This includes inside other
+  data structures, such as ``DenseMap<Value *, Vec<Value *>>``.
+
----------------
mehdi_amini wrote:
> silvas wrote:
> > mehdi_amini wrote:
> > > I don't necessarily subscribe to the stack/heap difference.
> > > 
> > > If the SVec/Vec is part of a heap allocation, they retain their properties: `Vect` would always an **extra** heap allocation for the data while SVec keeps inlined storage in the original heap allocation (with all the benefits of SmallVector).
> > > 
> > > I see it similarly to trailing allocation in heap allocated object somehow.
> > > In general, heap or stack, SmallVector and Vec behaves differently in "move" vs "copy".
> > This is definitely a simplification, and these are just "guidelines". There are definitely good reasons to use SVec on the heap or Vec on the stack. I think this is the safest default advice we can give, especially since large SmallVector on the heap seems to be one of the main patterns that has been historically problematic.
> Can you phrase this in a less prescriptive way? "should" comes a bit strong to me. I think it'd be enough to just say:
> 
> ```
> * ``SVec<T>`` should be used for vectors where a "small" number
>   of elements are expected. (the "S" in the name stands for "Small" or
>   "on the Stack").
> * ``Vec<T>`` should be used for cases where inline storage is too expensive. For example inside other
>   data structures, such as ``DenseMap<Value *, Vec<Value *>>``.
> ```
softened "should" to "recommend".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884