Should all PPC BSPs build with -mstrict-align?

[Michael Davidsaver]

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?

[Gedare Bloom]

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

It should also be back-ported to open branches.

[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.

[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.

[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.

[Joel Sherrill]

Given that this needs to be addressed on a per-BSP basis and was filed against the Beatnik, shouldn't the Beatnik have this option added?

Other BSPs would require investigation to know whether it is needed to add this option.

[Joel Sherrill]

I hesitate to close this since I believe that at least the Beatnik likely needs this added. And the options for every BSP need to be evaluated.

As a result, I am moving this to a new milestone. I think the example has to be compiled against the CPU CFLAGS used by every BSP. I tried to do this programmatically but the use of include's makes this a bit more difficult than a simple grep.

This depends on the version of gcc and the CPU flags used. This will require hand testing across all BSPs.

I am moving the milestone to 6.1 to keep it from being "won't fix" and hoping someone will perform the review.

[Sebastian Huber]

I think we should a test case for this so that everyone who cares to run the test suite will notice the issue.

[Chris Johns]

In the new build system?

[Michael Davidsaver]

I think a test case would be an excellent idea.

From looking into this a bit more, the '-mstrict-align' flag is overly conservative, and doesn't capture the granularity of the instructions which have alignment restrictions on a particular target.

eg. from the mpc7450 docs on the possible causes of an alignment exception

• A floating-point load/store, stmw, stwcx., lmw, lwarx, eciwx, or ecowx instruction operand is not word-aligned.
• A multiple/string load/store operation is attempted in little-endian mode
• An operand of a dcbz instruction is on a page that is write-through or cache-inhibited for a virtual mode access.
• An attempt to execute a dcbz instruction occurs when the cache is disabled or locked.

which I read as meaning plain 'stw' would work.

[Joel Sherrill]

This hasn't had an update in 2 years. Any thoughts? New milestone?

[Uchenna Ezeobi <uchenna.ezeobi3@…>]

In ae534d10/rtems:

spec: Add -mstrict-align to mvme2100 default build

Update #3767