STRICT_ALIGNMENT is too strict for m68k

STRICT_ALIGNMENT is too strict for m68k

[Greg McGary]

m68k.h has STRICT_ALIGNMENT unconditionally defined as 1, although a
few host-specific includes (linux.h. ccur-GAS.h, plexus.h) override it
as 0.  My experience is only with the 68302 (68000 core) and 68360
(a.k.a. cpu32, mostly 68020 core), so I can't make generalizations,
but I know that the 68020 cores (and probably later) do not require
strict alignment, whereas the 68000 (and 68010?) do require it.  The
fact that STRICT_ALIGNMENT is on unconditionally causes gcc to
generate byte-at-a-time memory access for packed structures which is
very inefficient in both time & space.

It would be a very nice improvement if STRICT_ALIGNMENT were made to
be conditional on the CPU implementation.

Would people with broader knowledge of the m68k line please tell me
which CPU cores allow unaligned word access?

Re: STRICT_ALIGNMENT is too strict for m68k

[Rask Ingemann Lambertsen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

Den 15-Maj-98 00:38:01 skrev Greg McGary følgende om "STRICT_ALIGNMENT is too strict for m68k":

> Would people with broader knowledge of the m68k line please tell me
> which CPU cores allow unaligned word access?

The 68000 and the 68010 do _not_ allow unaligned word access. The 68020,
68030, 68040 and 68060 _do_ allow unaligned word access.

Regards,

/¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯T¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯\
| Rask Ingemann Lambertsen       | E-mail: mailto:rask@kampsax.k-net.dk  |
| Registered Phase5 developer    | WWW: http://www.gbar.dtu.dk/~c948374/ |
| A4000, 775 kkeys/s (RC5-64)    | "ThrustMe" on XPilot, ARCnet and IRC  |
|   Shell to DOS... Shell to DOS... DOS, do you copy? Shell to DOS...    |

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	It would be a very nice improvement if STRICT_ALIGNMENT were made to
	be conditional on the CPU implementation.

Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
it will affect structure layout.  It is generally unwise to have -mCPU
options change the ABI, as most users won't want that.  It could
perhaps be done for a particular target instead of all of them.  Or it
could perhaps be done only if enabled by an option.  I think doing it
for all m68k targets by default would be a bad idea.

Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Greg McGary]

Jim Wilson <wilson@cygnus.com> writes:

> 	It would be a very nice improvement if STRICT_ALIGNMENT were made to
> 	be conditional on the CPU implementation.
> 
> Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
> it will affect structure layout.  It is generally unwise to have -mCPU
> options change the ABI, as most users won't want that.  It could
> perhaps be done for a particular target instead of all of them.  Or it
> could perhaps be done only if enabled by an option.  I think doing it
> for all m68k targets by default would be a bad idea.

Obviously I wasn't thinking about ABI.  I was thinking about a
memory-starved embedded system.  Turning off strict alignment shrinks
code size by 5%.  A better way to fix that system is to relax the
unnecessary uses of ((aligned (1), packed)), but this is legacy code
and doing that breaks other ports.  It's probably possible to debug
all ports so that structs can be unpacked, or packed at a 2-byte
boundary, but all of that takes time, and in the mean time, I'd like
to have the memory savings by flipping a switch.  I now have
`-m(no-)strict-align' implemented now as an Ascend-specific
SUBTARGET_SWITCHES.  Would you be willing to put `-m(no-)strict-align'
in TARGET_SWITCHES, issuing a warning when it is turned off on a CPU
that doesn't support unaligned access?

Re: STRICT_ALIGNMENT is too strict for m68k

[Kamil Iskra]

On Fri, 15 May 1998, Jim Wilson wrote:

> 	It would be a very nice improvement if STRICT_ALIGNMENT were made to
> 	be conditional on the CPU implementation.
> Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
> it will affect structure layout.

Is that really so? According to tm.texi:


@item STRICT_ALIGNMENT
Define this macro to be the value 1 if instructions will fail to work
if given data not on the nominal alignment.  If instructions will merely
go slower in that case, define this macro as 0.


Clearly, there is no mention of any influence on the structure alignment,
but only on the choice of instructions to generate for unaligned access.

I just installed the following patch in my local 1.0.2 tree:


Sat May 16 21:12:39 1998  Kamil Iskra  <iskra@student.uci.agh.edu.pl>

	* m68k.h (STRICT_ALIGNMENT): Depend on TARGET_68020.


--- egcs/gcc/config/m68k/m68k.h.orig	Tue Mar 17 16:06:24 1998
+++ egcs/gcc/config/m68k/m68k.h	Sat May 16 20:36:38 1998
@@ -289,7 +289,7 @@
 
 /* Set this nonzero if move instructions will actually fail to work
    when given unaligned data.  */
-#define STRICT_ALIGNMENT 1
+#define STRICT_ALIGNMENT (! TARGET_68020)
 
 /* Maximum power of 2 that code can be aligned to.  */
 #define MAX_CODE_ALIGN	2			/* 4 byte alignment */



I compiled the following testcase: 

#include <stddef.h>
#include <stdio.h>

struct A
{
    char a;
    short b;
    int c;
} str;

char a;
short b;
int c;

int main(void)
{
    printf("sizeof(struct A): %d\n", sizeof(struct A));

    printf("offsetof(struct A, a): %d\n", offsetof(struct A, a));
    printf("offsetof(struct A, b): %d\n", offsetof(struct A, b));
    printf("offsetof(struct A, c): %d\n", offsetof(struct A, c));

    str.a=a;
    str.b=b;
    str.c=c;
}

The results generated were the same for executables generated with the
compiler with unmodified STRICT_ALIGNMENT and with the patched one. The
case of -fpack-struct was also tested, and the patched compiler generated
better code. Therefore, I think that the patch given above is correct and
useful, and as such should be applied.

/ Kamil Iskra    AmigaOS  Linux/i386  Linux/m68k               \
| GeekGadgets GCC maintainer   UNIX system administrator       |
| iskra@student.uci.agh.edu.pl  kiskra@ernie.icslab.agh.edu.pl |
\ kamil@dwd.interkom.pl   http://student.uci.agh.edu.pl/~iskra /

Re: STRICT_ALIGNMENT is too strict for m68k

[Richard Henderson]

On Fri, May 15, 1998 at 02:35:18PM -0700, Greg McGary wrote:
> > Changing the value of STRICT_ALIGNMENT changes the ABI.

Seems to me that there should be a separate macro for controling
the alignment of members in structures, leaving STRICT_ALIGNMENT
for indicating if unaligned accesses to memory will trap.

Perhaps define a STRICT_ALIGNMENT_FOR_STRUCT, and default it to
STRICT_ALIGNMENT in stor-layout.c?

Gary, I don't think this would be a particularly large patch if
you'd like to have a go at it.


r~

Re: STRICT_ALIGNMENT is too strict for m68k

[Richard Henderson]

On Sat, May 16, 1998 at 09:18:04PM +0200, Kamil Iskra wrote:
> On Fri, 15 May 1998, Jim Wilson wrote:
> 
> > 	It would be a very nice improvement if STRICT_ALIGNMENT were made to
> > 	be conditional on the CPU implementation.
> > Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
> > it will affect structure layout.
> 
> Is that really so? According to tm.texi:

Yes.  See how it affects the alignment (and thus the size) of a 
structure in layout_record.  One might argue, given its description
in tm.texi, that this was wrong.  Alternately, tm.texi was simply
not updated properly.


r~

Re: STRICT_ALIGNMENT is too strict for m68k

[Kamil Iskra]

On Sat, 16 May 1998, Richard Henderson wrote:

> > > Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
> > > it will affect structure layout.
> > Is that really so? According to tm.texi:
> Yes.  See how it affects the alignment (and thus the size) of a 
> structure in layout_record.

I see. Thanks for the explanation.

However, I spent some time today on this part of stor-layout.c trying to
understand it and build a testcase that would prove my patch wrong and so
far I failed.

We are talking about the following code from stor-layout.c:layout_type,
right?


  /* Normally, use the alignment corresponding to the mode chosen.
     However, where strict alignment is not required, avoid
     over-aligning structures, since most compilers do not do this
     alignment.  */

  if (TYPE_MODE (type) != BLKmode && TYPE_MODE (type) != VOIDmode
      && (STRICT_ALIGNMENT
	  || (TREE_CODE (type) != RECORD_TYPE && TREE_CODE (type) != UNION_TYPE
	      && TREE_CODE (type) != QUAL_UNION_TYPE
	      && TREE_CODE (type) != ARRAY_TYPE)))
    TYPE_ALIGN (type) = GET_MODE_ALIGNMENT (TYPE_MODE (type));


I don't think this code applies to over-aligning structures, at least not
on m68k.

Many m68k targets define STRUCTURE_SIZE_BOUNDARY to be equal to
BIGGEST_ALIGNMENT. This means that structures are always maximally
aligned, thus the above code cannot over-align them.

Even if STRUCTURE_SIZE_BOUNDARY is not defined, the following code from
layout_type, case RECORD_TYPE takes care of things:


	  /* If structure's known alignment is less than
	     what the scalar mode would need, and it matters,
	     then stick with BLKmode.  */
	  if (STRICT_ALIGNMENT
	      && ! (TYPE_ALIGN (type) >= BIGGEST_ALIGNMENT
		    || (TYPE_ALIGN (type)
			>= TREE_INT_CST_LOW (TYPE_SIZE (type)))))
	    {
	      if (TYPE_MODE (type) != BLKmode)
		/* If this is the only reason this type is BLKmode,
		   then don't force containing types to be BLKmode.  */
		TYPE_NO_FORCE_BLK (type) = 1;
	      TYPE_MODE (type) = BLKmode;
	    }


Combined with the first code fragment shown, it prevents structures from
over-aligning.

I begin to wonder if the second code fragment doesn't make the first one
redundant, at least in the case of RECORD_TYPE. But then, some targets
might have some unusual alignment requirements, where alignment does not
correspond to the type size or sth. Still, this is not the case with m68k,
where chars are 8-bit aligned and everything else is 16-bit (or, on some
subtargets, 32-bit for types >= 32bits). Therefore, I still think that my
patch is OK.

> One might argue, given its description
> in tm.texi, that this was wrong.  Alternately, tm.texi was simply
> not updated properly.

Yeah, tm.texi clearly does not correspond to the actual behavior of
STRICT_ALIGNMENT and is misleading. I guess we are too late by at least a
few years to change the behavior of STRICT_ALIGNMENT, so I do agree with
another mail of yours that a new macro, equal to STRICT_ALIGNMENT by
default, ought to be defined.

/ Kamil Iskra    AmigaOS  Linux/i386  Linux/m68k               \
| GeekGadgets GCC maintainer   UNIX system administrator       |
| iskra@student.uci.agh.edu.pl  kiskra@ernie.icslab.agh.edu.pl |
\ kamil@dwd.interkom.pl   http://student.uci.agh.edu.pl/~iskra /

Re: STRICT_ALIGNMENT is too strict for m68k

[Jeffrey A Law]

  In message < Pine.LNX.3.96.980517174658.6643A-100000@jinks.home >you write:
  > > Yes.  See how it affects the alignment (and thus the size) of a 
  > > structure in layout_record.
  > 
  > I see. Thanks for the explanation.
It also effects how parameters are passed in registers for some
targets (see calls.c).

  > However, I spent some time today on this part of stor-layout.c trying to
  > understand it and build a testcase that would prove my patch wrong and so
  > far I failed.
Not worth the trouble to write a testcase for this.  We know the
change is ABI unsafe.

jeff

Re: STRICT_ALIGNMENT is too strict for m68k

[Andreas Schwab]

Jeffrey A Law <law@cygnus.com> writes:

|>   In message < Pine.LNX.3.96.980517174658.6643A-100000@jinks.home >you write:
|>   > > Yes.  See how it affects the alignment (and thus the size) of a 
|>   > > structure in layout_record.
|>   > 
|>   > I see. Thanks for the explanation.
|> It also effects how parameters are passed in registers for some
|> targets (see calls.c).

The m68k does not pass parameters in registers.

|>   > However, I spent some time today on this part of stor-layout.c trying to
|>   > understand it and build a testcase that would prove my patch wrong and so
|>   > far I failed.
|> Not worth the trouble to write a testcase for this.  We know the
|> change is ABI unsafe.

I have changed STRICT_ALIGNMENT to 0 some time ago in m68k-linux, and no
structure layout has ever changed.  I'm not convinced that this change
ever effects it, only the way unaligned accesses are handled.

-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	I now have
	`-m(no-)strict-align' implemented now as an Ascend-specific
	SUBTARGET_SWITCHES.  Would you be willing to put `-m(no-)strict-align'
	in TARGET_SWITCHES, issuing a warning when it is turned off on a CPU
	that doesn't support unaligned access?

This sounds reasonable.  I would add that the invoke.texi documentation should
say something about affecting the ABI, and hence resulting in incompatible
code.  Also, I think an error instead of a warning for bad option combinations
would be better.  If the code isn't going to work, then there should be a
compile time error, not a run time error.

Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	Seems to me that there should be a separate macro for controling
	the alignment of members in structures, leaving STRICT_ALIGNMENT
	for indicating if unaligned accesses to memory will trap.

Actually, there is already a second macro in use by expmed.c/expr.c called
SLOW_UNALIGNED_ACCESS, which defaults to STRICT_ALIGNMENT.  He could perhaps
make m68k.h define this macro, and modify other places if necessary to
use SLOW_UNALIGNED_ACCESS instead of STRICT_ALIGNMENT.  That would avoid
the ABI issue without creating a new macro.

I am not certain whether this actually solve the original problem though,
as he may have been relying on other affects of STRICT_ALIGNMENT.

Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	> Changing the value of STRICT_ALIGNMENT changes the ABI.  In particular,
	> it will affect structure layout.

	Is that really so? According to tm.texi:

STRICT_ALIGNMENT will have a number of effects that may not be obvious from
a cursory reading of the documentation.

I am certain that it will affect structure layout for some targets, but I do
not have time to construct an example.  It may not affect all m68k targets,
and it may not affect all structures, but it only needs to break one
structure for one target to make it an unacceptable change.

	 Therefore, I think that the patch given above is correct and
	useful, and as such should be applied.

The patch is not correct, because as I have already mentioned, it will cause
unacceptable ABI changes.

Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	The m68k does not pass parameters in registers.

The m68k does return values in registers, though, and the same reasoning
applies here.

	I have changed STRICT_ALIGNMENT to 0 some time ago in m68k-linux, and no
	structure layout has ever changed.  I'm not convinced that this change
	ever effects it, only the way unaligned accesses are handled.

Changing STRICT_ALIGNMENT in m68k/linux.h would be OK, since the affect will
be limited to one system, and people can either prove that it is harmless,
or decide that it is correct for that system.

However, changing STRICT_ALIGNMENT in m68k/m68k.h is risky, because we have
to prove that none of the m68k ports will be affected.  Simply arguing that
you can't find a testcase isn't really good enough.  However, to help shorten
the discussion a bit, I went ahead and worked up a testcase.

For the m68k-coff configuration, I get different code for the following
testcase when STRICT_ALIGNMENT changes.  If STRICT_ALIGNMENT, the structure
is BLKmode, and is returned in memory (see RETURN_IN_MEMORY in expr.h).
        move.b ga,(%a1)
        move.b ga+1,1(%a1)
        move.l %a1,%d0
If !STRICT_ALIGNMENT, the structure is HImode, and is returned in a register.
        move.w ga,%d0

struct foo
{
  char c;
  char d;
} ga;

struct foo
sub ()
{
  return ga;
}


Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Jeffrey A Law]

  In message < mszpgj43qh.fsf@tucson-net-82.eng.ascend.com >you write:
  > to have the memory savings by flipping a switch.  I now have
  > `-m(no-)strict-align' implemented now as an Ascend-specific
  > SUBTARGET_SWITCHES.  Would you be willing to put `-m(no-)strict-align'
  > in TARGET_SWITCHES, issuing a warning when it is turned off on a CPU
  > that doesn't support unaligned access?
I believe Jim has already said he's willing to accept such a change
in principle.  So am I.  He just got his mail out before I did :-)

So whip it up and submit it :-)

Cheers,
jeff

Re: STRICT_ALIGNMENT is too strict for m68k

[Kamil Iskra]

On Mon, 18 May 1998, Jim Wilson wrote:

> 	Seems to me that there should be a separate macro for controling
> 	the alignment of members in structures, leaving STRICT_ALIGNMENT
> 	for indicating if unaligned accesses to memory will trap.
> Actually, there is already a second macro in use by expmed.c/expr.c called
> SLOW_UNALIGNED_ACCESS, which defaults to STRICT_ALIGNMENT.  He could perhaps
> make m68k.h define this macro, and modify other places if necessary to
> use SLOW_UNALIGNED_ACCESS instead of STRICT_ALIGNMENT.  That would avoid
> the ABI issue without creating a new macro.

According to tm.texi:

@item SLOW_UNALIGNED_ACCESS
Define this macro to be the value 1 if unaligned accesses have a cost
many times greater than aligned accesses, for example if they are
emulated in a trap handler.

Please, let's not repeat the STRICT_ALIGNMENT problem by modifying
functionality of yet another simple macro in a confusing way.

IMHO, there is little use for SLOW_UNALIGNED_ACCESS, the way it is
currently defined, since:

1. with -m68000, unaligned access will simply crash.
2. with -m68020, unaligned access will not crash and will not be emulated
by a trap handler: it will merely run slightly slower.

The macro that SHOULD apply in this case is STRICT_ALIGNMENT. As pointed
out by the Cygnus people (thanks for the testcase, Jim: I observed the
BLKmode <-> ?Imode differences, but I wasn't aware of their consequences),
it can't be used, since (contrary to the way it is currently documented)
it is ABI unsafe.

Therefore, I think that what we need is a new macro, like
USE_STRICT_ALIGNMENT_ABI or sth, that, when defined and non-zero simulates
the ABI changes imposed by STRICT_ALIGNMENT (mode, alignment, register
arguments), but does not force unaligned accesses to be made scattered
into byte-accesses.

If done carefully, this would allow us to alter m68k.h in ABI-compatible
manner.

Does this sound OK for you guys? I am willing to contribute the necessary
changes.

/ Kamil Iskra    AmigaOS  Linux/i386  Linux/m68k               \
| GeekGadgets GCC maintainer   UNIX system administrator       |
| iskra@student.uci.agh.edu.pl  kiskra@ernie.icslab.agh.edu.pl |
\ kamil@dwd.interkom.pl   http://student.uci.agh.edu.pl/~iskra /

Re: STRICT_ALIGNMENT is too strict for m68k

[Jim Wilson]

	@item SLOW_UNALIGNED_ACCESS
	Define this macro to be the value 1 if unaligned accesses have a cost
	many times greater than aligned accesses, for example if they are
	emulated in a trap handler.

	Please, let's not repeat the STRICT_ALIGNMENT problem by modifying
	functionality of yet another simple macro in a confusing way.

I think the only thing wrong here is that the documentation for
SLOW_UNALIGNED_ACCESS was poorly written.  It already does what you want
as far as I can tell.  There is no need to change the meaning of it; we
need only correct ambiguities in the documentation for it.

SLOW_UNALIGNED_ACCESS was originally created for targets that did not have
STRICT_ALIGNMENT set, but where we didn't want to emit unaligned loads because
they were too slow.  The documentation was written with this bias.

However, SLOW_UNALIGNED_ACCESS can also be used for targets that do have
STRICT_ALIGNMENT set, in which case defining SLOW_UNALIGNED_ACCESS to zero
will cause gcc to emit unaligned loads, resulting in slower but smaller code.
This case was not considered when the documentation was written, but the
implementation already does this.

Jim

Re: STRICT_ALIGNMENT is too strict for m68k

[Rask Ingemann Lambertsen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

[cut long cc: list]

Den 18-Maj-98 11:04:23 skrev Andreas Schwab følgende om "Re: STRICT_ALIGNMENT is too strict for m68k":
> Jeffrey A Law <law@cygnus.com> writes:

> The m68k does not pass parameters in registers.

At least on the m68k-amigaos target, there's an option for this:

-mregparm=number_of_registers

where number_of_registers defaults to 2.

Regards,

/¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯T¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯\
| Rask Ingemann Lambertsen       | E-mail: mailto:rask@kampsax.k-net.dk  |
| Registered Phase5 developer    | WWW: http://www.gbar.dtu.dk/~c948374/ |
| A4000, 775 kkeys/s (RC5-64)    | "ThrustMe" on XPilot, ARCnet and IRC  |
| Never underestimate the bandwidth of a CD-ROM flying through the lab.  |

Re: STRICT_ALIGNMENT is too strict for m68k

[Andreas Schwab]

Jim Wilson <wilson@cygnus.com> writes:

|> 	The m68k does not pass parameters in registers.

|> The m68k does return values in registers, though, and the same reasoning
|> applies here.

Ok, you're right, and the test case was really convincing. :-)

Andreas.

-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org