[llvm-commits] [PATCH] Fix overalignment of SmallVector

[Richard Smith]

On Tue, Aug 21, 2012 at 3:51 PM, Chris Lattner <clattner at apple.com> wrote:

>
> On Aug 21, 2012, at 3:19 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> > Hi,
> >
> > SmallVector currently always has 16-byte alignment (on x86_64), no
> matter what type is contained with it, because SmallVectorBase has a long
> double member.  This causes problems: for instance, ExtractValueInst has 16
> byte alignment due to its Indices member (of type SmallVector<unsigned,
> 4>), but is only allocated with 8 byte alignment.
> >
> > The attached patch fixes this by removing the long double union hack
> from SmallVectorBase. Instead, SmallVector's inline storage is represented
> as a sequence of  llvm::AlignedCharArrayUnion<T> objects, with the first
> such object living in SmallVectorTemplateCommon<T> rather than in
> SmallVectorBase.
> >
> > The downside of this is that SmallVector<T> now always requires T to be
> a complete type. However, it turns out that no code in LLVM or Clang was
> deliberately depending on SmallVector<T, 0>'s promise that T is not
> required to be complete. This did however expose a problem: ArrayRef<T>
> copy-construction was implicitly instantiating SmallVectorTemplateCommon<T>
> (and std::vector<T>) during overload resolution. To avoid this, I've added
> a dummy template parameter to SmallVectorTemplateCommon, so now the
> ArrayRef(SmallVector) constructor will only be considered if the argument
> is already of type SmallVector (and not merely if it converts to a
> SmallVector). I've applied the same change to the ArrayRef(std::vector)
> constructor, where the same problem exists, but our current standard
> library implementations happen to allow us to get away with it.
>
> LGTM, thanks!
>

Thanks, r162331.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120821/688bcd18/attachment.html>