[PATCH] D49163: ADT: Shrink SmallVector size 0 to 16B on 64-bit platforms

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

dexonsmith added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:90
+        reinterpret_cast<const char *>(this) +
+        offsetof(SmallVectorAlignmentAndSize<T>, FirstEl)));
+  }
----------------
dexonsmith wrote:
> rnk wrote:
> > This assumes that base classes are laid out more or less the same as fields, but we were already assuming that there wouldn't be padding between FirstEl and InlineElts, so this seems like an improvement.
> What about this?
> ```
> struct SmallVectorAlignmentAndSizeBase {
>   AlignedCharArrayUnion<SmallVectorBase> Base;
> };
> template <class T>
> struct SmallVectorAlignmentAndSize : SmallVectorAlignmentAndSizeBase {
>   AlignedCharArrayUnion<T> FirstEl;
> };
> 
> // still doing this later:
>   void *getFirstEl() const {
>     return const_cast<void *>(reinterpret_cast<const void *>(
>         reinterpret_cast<const char *>(this) +
>         offsetof(SmallVectorAlignmentAndSize<T>, FirstEl)));
>   }
> ```
> we were already assuming that there wouldn't be padding between FirstEl and InlineElts

BTW, I don't think we had to assuming that there wouldn't be padding.  As long as there's enough otherwise-unused storage after `FirstEl`, we won't overflow the allocation.


https://reviews.llvm.org/D49163