On Mon, May 19, 2014 at 10:52:52AM +0200, Richard Biener wrote:
> On Fri, May 16, 2014 at 9:12 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> > this patch makes also the rtti type info for A in the testcase:
> >
> > struct A
> > {
> > virtual void foo(void) {};
> > virtual void foo2(void) {};
> > virtual void foo3(void) {};
> > virtual void foo4(void) {};
> > virtual void foo5(void) {};
> > } a;
> >
> > aligned only to the ABI requirement (8) instead of being bumped up to 16
> > bytes
> > by the following code in i386.c:
> > /* x86-64 ABI requires arrays greater than 16 bytes to be aligned
> > to 16byte boundary. */
> > if (TARGET_64BIT)
> > {
> > if ((opt ? AGGREGATE_TYPE_P (type) : TREE_CODE (type) == ARRAY_TYPE)
> > && TYPE_SIZE (type)
> > && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
> > && wi::geu_p (TYPE_SIZE (type), 128)
> > && align < 128)
> > return 128;
> > }
> >
> > Here the variable is first run through align_variable and that decides to
> > add
> > optional alignment. We really want only ABI required alignment here.
> > Does the following patch look resonable?
>
> Hmm, but if the optimizers or the target can rely on DATA_ABI_ALIGNMENT
> then we can't really lower it. Because we can make the vtable escape
> to another unit that sees it as just an array of pointers?
Sure, they can rely on DATA_ABI_ALIGNMENT (if that macro is defined), but
anything beyond that (such as what DATA_ALIGNMENT returns) is optimization
only. So, Honza's patch looks good for me.
> So this looks unsafe to me. (same may apply to the idea of
> having TARGET_VTABLE_ENTRY_ALIGN at all, if that possibly
> conflicts with ABI alignment requirements present otherwise).
Right now the intersection of targets overriding TARGET_VTABLE_ENTRY_ALIGN and
targets defining DATA_ABI_ALIGNMENT is empty. In any case, even in that
case one should (if DATA_ABI_ALIGNMENT is defined) apply DATA_ABI_ALIGNMENT
(on top of TARGET_VTABLE_ENTRY_ALIGN and/or TYPE_ALIGN, dunno how those two
exactly mix together) and not DATA_ALIGNMENT. But this patch is about
tinfo, not vtable.
> > * rtti.c: Include tm_p.h
> > (emit_tinfo_decl): Align type infos only as required by the target
> > ABI.
> >
> > Index: rtti.c
> > ===================================================================
> > --- rtti.c (revision 210521)
> > +++ rtti.c (working copy)
> > @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.
> > #include "coretypes.h"
> > #include "tm.h"
> > #include "tree.h"
> > +#include "tm_p.h"
> > #include "stringpool.h"
> > #include "stor-layout.h"
> > #include "cp-tree.h"
> > @@ -1596,6 +1597,12 @@ emit_tinfo_decl (tree decl)
> > DECL_INITIAL (decl) = init;
> > mark_used (decl);
> > cp_finish_decl (decl, init, false, NULL_TREE, 0);
> > + /* Avoid targets optionally bumping up the alignment to improve
> > + vector instruction accesses, tinfo are never accessed this way. */
> > +#ifdef DATA_ABI_ALIGNMENT
> > + DECL_ALIGN (decl) = DATA_ABI_ALIGNMENT (decl, TYPE_ALIGN (TREE_TYPE
> > (decl)));
> > + DECL_USER_ALIGN (decl) = true;
> > +#endif
> > return true;
> > }
> > else
Jakub
