[PATCH] D91467: ADT: Take small enough, trivially copyable T by value in SmallVector

[Sean Silva via Phabricator via llvm-commits]

silvas added a comment.

Do you have any performance and binary size results? I suspect that these are frequently just inlined + sroa'd so this won't make much of a difference in the end.



================
Comment at: llvm/include/llvm/ADT/SmallVector.h:321
+  using ValueParamT =
+      typename std::conditional<sizeof(T) <= 16, T, const T &>::type;
+
----------------
njames93 wrote:
> In relation to my comment about insert, it would be a lot easier to implement if this condition was extracted out into a static constexpr variable
> ```lang=c++
> static constexpr bool IsPushByVal = sizeof(T) <= 16;
> ```
> 
> A second point is maybe this should be based loosely on the target. Though I'm not super sure on that, WDYT?
> ```lang=c++
> sizeof(T) <= 2 * sizeof(void*)
> ```
> 
+1, I think that basing it on sizeof(void*) probably makes sense.


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

https://reviews.llvm.org/D91467