[RS6000] strict alignment for little-endian

On Mon, Jun 10, 2013 at 03:14:04PM -0400, Michael Meissner wrote:
> On Fri, Jun 07, 2013 at 10:54:39AM +0930, Alan Modra wrote:
> > I'd like to remove -mstrict-align for little-endian powerpc, because
> > the assumption that mis-aligned accesses are massively slow isn't true
> > for current powerpc processors.  However I also don't want to break
> > old machines, so probably should add -mstrict-align back for some set
> > of cpus.  Can anyone tell me the set?
> 
> I don't know the set.  I recall the original PPC's (604, 603, etc.) had the
> strict alignment rule in little endian rule, and the 750 relaxed it for GPRs
> (but not FPRs), but I don't know about later machines.

After some research and bothering people who ought to know, I came
up with the following:

Since modern powerpc processors that support little-endian (power6 and
later) do so properly rather than via the low bit swizzling of older
processors (603, 604, 750, 74xx) we don't take traps on all unaligned
accesses.  Even so, power6 and power7 still take more alignment traps
in little-endian mode than big-endian, so it may be advantageous to
leave -mstrict-align as the default for power7.  Anton tells me that
for power8, -mstrict-align is not necessary.  It's not desirable since
gcc easily loses track of alignment, for instance with -mstrict-align

void foo (char *p, char *q)
{
  __builtin_memcpy (p, q, 4);
}

generates

	lbz 7,0(4)
	lbz 8,1(4)
	lbz 10,2(4)
	lbz 9,3(4)
	stb 7,0(3)
	stb 8,1(3)
	stb 10,2(3)
	stb 9,3(3)
	blr

whereas -mno-strict-align gives

	lwz 9,0(4)
	stw 9,0(3)
	blr

This patch disables the default -mstrict-align on power8.  Rather than
testing values from enum processor_type, which is prone to error when
adding a new processor, I chose one of the mask bits that happens to
be set for power8, and is likely to be set for future new processors.
I also take the opportunity to update PROCESSOR_DEFAULT64 to the value
most likely appropriate for machines running ELFv2.  Bootstrapped and
regression tested powerpc64-linux.  OK to apply?

David, it also occurs to me that PROCESSOR_DEFAULT* isn't the best
choice of macro name nowadays.  The macro is only used to default
-mtune, not -mcpu.  If you think it a good idea, I'll
s/PROCESSOR_DEFAULT/TUNE_DEFAULT/ throughout config/rs6000 as a
followup commit to this one.

	* config/rs6000/sysv4.h (CC1_ENDIAN_LITTLE_SPEC): Define as empty.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Default
	to strict alignment on older processors when little-endian.
	* config/rs6000/linux64.h (PROCESSOR_DEFAULT64): Default to power8
	for ELFv2.

> It's not desirable since gcc easily loses track of alignment, for instance
> with -mstrict-align
> 
> void foo (char *p, char *q)
> {
>   __builtin_memcpy (p, q, 4);
> }
> 
> generates
> 
> 	lbz 7,0(4)
> 	lbz 8,1(4)
> 	lbz 10,2(4)
> 	lbz 9,3(4)
> 	stb 7,0(3)
> 	stb 8,1(3)
> 	stb 10,2(3)
> 	stb 9,3(3)
> 	blr
> 
> whereas -mno-strict-align gives
> 
> 	lwz 9,0(4)
> 	stw 9,0(3)
> 	blr

I presume you meant:

void foo (int *p, int *q)
{
  __builtin_memcpy (p, q, 4);
}

which will yield the same generated code.  Yes, it's an unfortunate regression 
on strict-alignment platforms, up to 4.5 the generated code was the same.

On Tue, Nov 19, 2013 at 1:59 AM, Alan Modra <amodra@gmail.com> wrote:

> This patch disables the default -mstrict-align on power8.  Rather than
> testing values from enum processor_type, which is prone to error when
> adding a new processor, I chose one of the mask bits that happens to
> be set for power8, and is likely to be set for future new processors.
> I also take the opportunity to update PROCESSOR_DEFAULT64 to the value
> most likely appropriate for machines running ELFv2.  Bootstrapped and
> regression tested powerpc64-linux.  OK to apply?
>
> David, it also occurs to me that PROCESSOR_DEFAULT* isn't the best
> choice of macro name nowadays.  The macro is only used to default
> -mtune, not -mcpu.  If you think it a good idea, I'll
> s/PROCESSOR_DEFAULT/TUNE_DEFAULT/ throughout config/rs6000 as a
> followup commit to this one.
>
>         * config/rs6000/sysv4.h (CC1_ENDIAN_LITTLE_SPEC): Define as empty.
>         * config/rs6000/rs6000.c (rs6000_option_override_internal): Default
>         to strict alignment on older processors when little-endian.
>         * config/rs6000/linux64.h (PROCESSOR_DEFAULT64): Default to power8
>         for ELFv2.

The patch is okay.

I agree that the mechanism for selecting the default tuning should be
cleaned up, but renaming PROCESSOR_DEFAULT to TUNE_DEFAULT just papers
over the issue.

Thanks, David