This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ patch] Reduce vtable alignment


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]