[PATCH] D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers.
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 13 14:40:33 PST 2020
mehdi_amini 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 *>>``.
+
----------------
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 *>>``.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90884/new/
https://reviews.llvm.org/D90884
More information about the llvm-commits
mailing list