Re: [PATCH] PR opt/6627: New target macro MINIMUM_FUNCTION_BOUNDARY

[Roger Sayle <roger at eyesopen dot com>]

On Sun, 29 Sep 2002, Richard Henderson wrote:
> On Sun, Sep 29, 2002 at 12:51:55PM -0600, Roger Sayle wrote:
> > The problem is that TARGET_PTRMEMFUNC_VBIT_LOCATION is defined
> > in terms of FUNCTION_BOUNDARY [line 472 of defaults.h].
>
> Yes, but that's ifndef.  One would force the ABI one
> desired in the target header.

Ahh, OK.  I see the solution you're considering.  Avoid the new
target macro, keep FUNCTION_BOUNDARY as the _known_ minimum
alignment, i.e. return it back to 1 byte on IA-32, and then
force TARGET_PTRMEMFUNC_VBIT_LOCATION in ptrmemfunc_vbit_in_pfn
in i386.h, and modify the C++ and Java front-ends to always
increase the user-specified to atleast two bytes when
TARGET_PTRMEMFUNC_VBIT_LOCATION requires it.

The only difficulty is that the code in varasm.c currently
views user alignment as a hint, and only guarantees atleast
FUNCTION_BOUNDARY alignment.  See the code in assemble_start_function
that mentions ASM_OUTPUT_MAX_SKIP_ALIGN might not do anything and
the cfun->function_frequency != FUNCTION_FREQUENCY_UNLIKELY_EXECUTED
My patch guaranteed FUNCTION_BOUNDARY alignment in align_functions
was >= FUNCTION_BOUNDARY, and MINIMUM_FUNCTION_BOUNDARY alignment
otherwise.

Of course, this is easily resolved and simplifies the code by
always honoring the user alignment.  It'll both simplify the
logic and decrease the size of the .s files.

> > Infact this is the only other use of FUNCTION_BOUNDARY in CVS,
> > so this would contradict your claim that this is used in
> > optimizations.
>
> Untrue.
>
> builtins.c:211:  align = FUNCTION_BOUNDARY;
> c-common.c:3061:  value = size_int (FUNCTION_BOUNDARY / BITS_PER_UNIT);
> tree.c:1277:  return FUNCTION_BOUNDARY;

I must be going insane.  Writing the patch yesterday I'm sure I'd
grepped for all the uses of FUNCTION_BOUNDARY and align_functions.
Feel free to call me an idiot.

And indeed all of these uses are infact bugs with the current
value of FUNCTION_BOUNDARY as defined by i386.h.  Worse, they
aren't fixed by my patch as currently written.


As penance, let me know if you'd like me to produce a patch
along the lines of my interpretation of your comments above.

Roger
--
Ok, 16 byte alignment makes sense for IA-64.  How about i960?