On Fri, 6 Jun 2014, Richard Sandiford wrote: > Richard Biener <rguent...@suse.de> writes: > > On Thu, 5 Jun 2014, Sandra Loosemore wrote: > > > >> On 06/05/2014 03:50 PM, Richard Sandiford wrote: > >> > Sandra Loosemore <san...@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.
