This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- From: Richard Biener <rguenther at suse dot de>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Sandra Loosemore <sandra at codesourcery dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 6 Jun 2014 09:44:00 +0200 (CEST)
- Subject: Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Authentication-results: sourceware.org; auth=none
- References: <537A5DA9 dot 9010905 at codesourcery dot com> <87y4xd4xn1 dot fsf at talisman dot default> <alpine dot LSU dot 2 dot 11 dot 1406041417110 dot 2632 at zhemvz dot fhfr dot qr> <538F3EFA dot 9030008 at codesourcery dot com> <alpine dot LSU dot 2 dot 11 dot 1406050933510 dot 2632 at zhemvz dot fhfr dot qr> <5390CCD4 dot 6010007 at codesourcery dot com> <87ppin81qt dot fsf at talisman dot default> <53911A8E dot 9000004 at codesourcery dot com> <alpine dot LSU dot 2 dot 11 dot 1406060915150 dot 2632 at zhemvz dot fhfr dot qr> <87d2em8p71 dot fsf at talisman dot default>
On Fri, 6 Jun 2014, Richard Sandiford wrote:
> Richard Biener <rguenther@suse.de> writes:
> > On Thu, 5 Jun 2014, Sandra Loosemore wrote:
> >
> >> On 06/05/2014 03:50 PM, Richard Sandiford wrote:
> >> > Sandra Loosemore <sandra@codesourcery.com> writes:
> >> > > On 06/05/2014 01:39 AM, Richard Biener wrote:
> >> > > >
> >> > > > [snip]
> >> > > >
> >> > > > Ok, we definitely need to preserve that (documented) behavior. I
> >> > > > suppose
> >> > > > it also sets DECL_USER_ALIGN. -falign-functions is probably another
> >> > > > setter of DECL_ALIGN here.
> >> > > >
> >> > > > If we add a target hook that may adjust function alignment then it
> >> > > > has to honor any user set alignment, then -falign-functions and
> >> > > > then it may only increase alignment over the default FUNCTION_BOUNDARY.
> >> > > >
> >> > > > The point to adjust alignment with the hook may still be output time,
> >> > > > but as we figured it can't simply ignore DECL_ALIGN.
> >> > >
> >> > > Hmmmm. The MIPS tweak we want here is to decrease the alignment for
> >> > > certain functions, so I suppose this means we'd have to go about it
> >> > > backwards by making FUNCTION_BOUNDARY 16 rather than 32 (at least for
> >> > > -Os) and then later increasing it back to 32 for all non-microMIPS
> >> > > functions.
> >> > >
> >> > > Richard S., WDYT? Shall I hack up another patch that does it that way,
> >> > > or do you think that's still the wrong way to go about it?
> >> >
> >> > Well, it sounds like the idea is that FUNCTION_BOUNDARY should basically
> >> > be ignored as a DECL_ALIGN. The only cases where DECL_ALIGN should matter
> >> > are those where the user has set the alignment via command-line options
> >> > or attributes. If so, I'd rather just leave it as it is and make the
> >> > hook pick something smaller.
> >> >
> >> > Just to be sure: I'd imagined this working by having varasm.c detect
> >> > when DECL_ALIGN needs to be honoured and having a hook to call when
> >> > DECL_ALIGN is irrelevant. The default version would then assert
> >> > that DECL_ALIGN is FUNCTION_BOUNDARY and would return that alignment.
> >> > The MIPS hook would override it for the special microMIPS case,
> >> > but would call into the default otherwise.
> >> >
> >> > (Hmm, maybe for that level of detail I should just have written a patch.
> >> > Sorry about that...)
> >
> > Well, the question is what you initialize DECL_ALIGN to initially and how
> > you ensure that docs are followed for aligned attribute (you can't
> > decrease function alignment by it). Adjusting a pre-existing DECL_ALIGN
> > to sth smaller at any point sounds dangerous. So why not go with
> > Sandras idea instead? I'd transition FUNCTION_BOUNDARY to a target hook
> > like
> >
> > unsigned function_boundary (tree decl);
> >
> > and for decl == NULL return the "default" (the default hook implementation
> > would just return MAX (FUNCTION_BOUNDARY, DECL_ALIGN)). We'd call
> > function_boundary (NULL) from tree.c and later from varasm.c query
> > it again, but this time with the decl specified.
> >
> > And yes, you'd lower your default function_boundary.
>
> What I don't like about this is the function_boundary (NULL) bit.
> If having function_boundary (NULL) lower than function_boundary (decl)
> is dangerous, that implies that we're using the function_boundary (NULL)
> somewhere, which in itself seems bad.
The other way around - function_boundary (NULL) should not be larger
than function_boundary (decl).
> How about initialising the DECL_ALIGN to:
>
> (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
> ? 2 * BITS_PER_UNIT : BITS_PER_UNIT)
>
> instead of function_boundary (NULL)? That way we might even be able to
> rely on DECL_ALIGN in get_object_alignment_2.
Not sure about that, but initializing that way certainly looks
reasonable.
Btw, maybe we should call function_boundary to properly initialize
DECL_ALIGN earlier than output time, for example in
cgraph_finalize_function. There might be an IPA pass sorting
functions after their alignment to squeeze some more bits
by reducing possible padding.
Richard.
- References:
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
- Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os