#3767 new defect

Should all PPC BSPs build with -mstrict-align?

Reported by: Michael Davidsaver Owned by:
Priority: normal Milestone: 5.1
Component: arch/powerpc Version: 5
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

As requested by Gedare Bloom.

https://lists.rtems.org/pipermail/users/2019-July/033400.html

Based on the description in the GCC documentation, I would expect
this to be needed unless the target handles alignment exceptions
to fixup unaligned access (which RTEMS doesn't).

https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options

It looks like some, but not all, RTEMS BSPs provide this flag.
With relevant exceptions being mvme5500 and beatnik.

https://github.com/RTEMS/rtems/search?q=-mstrict-align&type=Code

We're seeing a fault from a memcpy() when a copy of a double into an
unaligned byte buffer is optimized to the builtin.

char *buf = ...;
double value = ...;
memcpy(buf, &value, sizeof(value));

In the absence of -mstrict-align this memcpy() is sometimes ("-mcpu=7400 -O3")
optimized to a single "stfd" which faults because 'buf' is not 8 byte aligned.

I feel like there is a bug here. My question is whether this is the
builtin making an overzealous assumption about destination alignment,
or our usage of memcpy() being incorrect somehow. Should this be reported as a GCC bug as well?

Change History (4)

comment:1 Changed on Jul 17, 2019 at 6:20:12 PM by Gedare Bloom

Milestone: 5.1
Version: 5

I think this option likely should be enabled for all powerpc BSPs.

It should also be back-ported to open branches.

comment:2 Changed on Jul 18, 2019 at 6:04:23 AM by Sebastian Huber

I don't think this should be enabled for all BSPs. For example the e6500 based QorIQ variants don't need it. It needs to be added case by case.

comment:3 Changed on Jul 18, 2019 at 7:00:23 PM by Michael Davidsaver

I forgot to link to our analysis https://github.com/epics-base/epics-base/issues/29

The effect can be seen by compiling:

#include <string.h>
void test_double(char *dest, double src)
{
   memcpy(dest, &src, sizeof(src));
}

void test_int64(char *dest, long long src)
{
   memcpy(dest, &src, sizeof(src));
}

eg.

powerpc-rtems4.10-gcc -mcpu=7400 -g -O3 -o copyme.o -c copyme.c  && powerpc-rtems4.10-objdump -dS copyme.o

Then add -mstrict-align and see the difference.

I should also note that -mcpu=8540 (mvme3100) doesn't seem to need -mstrict-align. Not sure if this is implied for this target.

comment:4 Changed on Jul 19, 2019 at 5:27:56 AM by Sebastian Huber

The GCC -mcpu option changes the default values of other options such as -mstrict-align. You can have a look at the GCC sources: gcc/config/rs6000/rs6000-cpus.def

If you have problems with an alignment exception on a -mcpu=7400 based BSP, then simply add the -mstrict-align to this BSP. Other BSPs should remain as is.

Note: See TracTickets for help on using tickets.