This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ patch] Reduce vtable alignment
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>
- Date: Mon, 19 May 2014 11:10:43 +0200
- Subject: Re: [C++ patch] Reduce vtable alignment
- Authentication-results: sourceware.org; auth=none
- References: <20140516180442 dot GI20755 at kam dot mff dot cuni dot cz> <20140516191242 dot GA10833 at kam dot mff dot cuni dot cz> <CAFiYyc3S1TpPa8rM-yZn=e9ykkxj_T_kGNn5PzLQG=G-k4ZsAg at mail dot gmail dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, May 19, 2014 at 10:52:52AM +0200, Richard Biener wrote:
> On Fri, May 16, 2014 at 9:12 PM, Jan Hubicka <hubicka@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