[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