Re: Revised mips alignment patches.

[Richard Henderson <rth@cygnus.com>]

On Tue, Aug 31, 1999 at 04:40:08PM +1000, Geoff Keating wrote:
> Also, all the implementations share a common bug: FUNCTION_BOUNDARY is
> not just a request for an alignment, it's also an assertion that
> function pointers have certain low bits clear (see
> get_pointer_alignment in builtins.c and c_alignof in c-typeck.c).

Good eyes.

> I can easily make it -falign-functions=<n> if people think
> that's an improvement, but we already have -finline-limit-<n>.

I'd much rather the `=' syntax, as it's more clear that the n is an
argument.  I don't like -finline-limit-n or -fschedule-verbose-n either.

> What do people think?

I think the patch is generally good.

> +  /* Set up the align_*_log variables, defaulting them to 1 if they
> +     were still unset.  */
> +  if (align_loops <= 0) align_loops = 1;
> +  align_loops_log = floor_log2 (align_loops);
> +  if (align_jumps <= 0) align_jumps = 1;
> +  align_jumps_log = floor_log2 (align_jumps);
> +  if (align_labels <= 0) align_labels = 1;
> +  align_labels_log = floor_log2 (align_labels);
> +  if (align_functions <= 0) align_functions = 1;
> +  align_functions_log = floor_log2 (align_functions);

We should force align_functions to FUNCTION_BOUNDARY here, just
so the back end's optimization_options doesn't have to, and ...

> +  /* Handle a user-specified function alignment.
> +     Note that we still need to align to FUNCTION_BOUNDARY, as above,
> +     because ASM_OUTPUT_MAX_SKIP_ALIGN might not do any alignment at all.  */

... we don't have to dual case this.

> 	* config/mips/mips.c (override_options):  On 64-bit targets,
> 	try to align things to 64-bit boundaries.
> 	(print_operand): New substitution, %~,
> 	which aligns labels to align_labels_log.
> 	* config/mips/mips.md (div_trap_normal): Use %~.
> 	(div_trap_mips16): Likewise.
> 	(abssi): Likewise.
> 	(absdi2): Likewise.
> 	(ffssi2): Likewise.
> 	(ffsdi2): Likewise.
> 	(ashldi3_internal): Likewise.
> 	(ashrdi3_internal): Likewise.
> 	(lshrdi3_internal): Likewise.
> 	(casesi_internal): Likewise.

Gavin should approve the mips stuff.

> 	(sparc_override_options): Default align_functions on ultrasparc.
> 	Provide backwards compatibility for -malign-loops, -malign-jumps,
> 	-malign-functions.

I don't see any point in providing backward compatibility on
Sparc.  They aren't that heavily used except on x86, since they
appear in the Linux kernel makefiles.

The Sparc bits are fine, otherwise.


r~