Subject: Re: new patch

From: Matthew Wilcox <[email-masked]>
Date: Mon, 5 Jan 1998 18:25:11 +0000 (GMT)

> 
> BTW, for those of you using the new GCC, something that has been discovered
> by Dave Gilbert:
> 
>  When compiling the kernel, turn off some of the optimisations, specifically
>  the one that allows GCC/EGCS to assume that all structures are word aligned.
>  GCC will try to load a short using a LDR with a shift.  EG, to access bytes
>  12 and 13 only of a structure, GCC will use by default:
>    ldr r0, [r1, #14]
>    mov r0, r0, lsr #16
> 
>  This means that if r1 isn't word aligned (it certainly isn't for IP ethernet
>  packets in this case), the LDR picks up the wrong data.
> 
> Does anyone know if this is a legal optimisation that the compiler is making?

All you need to add is -mshort-load-bytes to the command line.  Phil told
me about this one already.

> BTW, don't say that it should be word aligned.  If that structure was word
> aligned, then the IP structures won't be, which will be even worse.  Blame
> Xerox for that one!

Cuh, those Xerox guys, hey?  Fancy not optimising a network protocol for
a 32 bit CPU due to be designed in 20 years time.. ;-)

unsubscribe: body of `unsubscribe linux-arm' to [email-masked]

---

From: Philip Blundell <[email-masked]>
Date: Mon, 05 Jan 1998 22:36:31 +0000

>me about this one already.

Actually, that won't fix it in all cases (though I think it will for the 
specific one of the Ethernet code).  According to Richard Earnshaw it can 
still go wrong.  

In the short term we need to either use -mshort-load-bytes if it works, and 
fix the kernel if not.  In the longer term we might be able to persuade gcc to 
generate the right code automatically.

p.

---

From: Philip Blundell <[email-masked]>
Date: Mon, 05 Jan 1998 22:50:22 +0000

>Does anyone know if this is a legal optimisation that the compiler is making?

The consensus among the gcc people is that it is, because the ARM ABI is 
allowed to define a minimum alignment (and has done so, to 32).  The 
optimisation does produce noticeably better code in the vast majority of cases.

Changing this alignment now would not only cause a performance hit for all 
binaries, but it would make gcc incompatible with armcc, and with previous 
versions of gcc (in other words, all libraries would need recompiling). 

A way to signal to gcc that a structure is unaligned in this way is on the 
cards in the future, but we don't have it yet.  In the short term we need to 
find a workaround such as using -mshort-load-bytes on the offending source 
files, or fixing them to use explicit byte ops.

p.

---

From: Matthew Wilcox <[email-masked]>
Date: Mon, 5 Jan 1998 22:28:29 +0000 (GMT)

>> EG, to access bytes

Can I ask for a specific example here?  Are we talking about something along
the lines of:

struct foo
{
  int a;
  short b;
  short c;
  char d;
  short e;
  short f;
}

ie which of these will be wrongly accessed?  And what alignment will GCC
give them?

I don't think that -mshort-load-bytes is the correct way to differentiate
the two cases, I think -munaligned-loads versus -mno-unaligned-loads is the
correct name for the switch - simply because I don't think shorts should
be loaded as bytes!  It's /at least/ as efficient to load them as words
and then shift them twice as it is to load as bytes and orr them together.

---

From: Alan Cox <[email-masked]>
Date: Mon, 5 Jan 1998 23:08:53 +0000 (GMT)

> generate the right code automatically.

Fixing the kernel is a non option for every case I think. What I've
suggested to Russell is this for later ARM chips

o	Compile with gcc 2.8 without -mshort-load-bytes
o	In kernel space take exceptions on unaligned load/store (since there
	should be almost none)
o	Instrument the ones we get and write little unaligned load
	macros for the ones that show a lot (ie ether_type_trans)

Fixing gcc is hard - it means fixing the __attribute(packed)) bug in the ARM so
that it works properly and potentially adding an __attribute(unaligned) 
to the compiler.

By taking alignment traps now gcc wont generate them all the time its
realistic to make AX.25 and IPX work on the ARM

---

From: David Alan Gilbert <[email-masked]>
Date: Tue, 06 Jan 1998 09:51:43 +0000

> suggested to Russell is this for later ARM chips

But what of the older ARMs.

Dave

-- 
----------------------------------------------------------------------
- Dr. David Alan Gilbert - WARNING! This is a beta release .signature-
- Work:    dg @ cogency.co.uk        -    0161-428-9444              -
- Home:    gro.gilbert @ treblig.org -                               -
----------------------------------------------------------------------

---

From: Alan Cox <[email-masked]>
Date: Tue, 6 Jan 1998 10:03:33 +0000 (GMT)

> But what of the older ARMs.

You would appear to end up with the same situation as before. Those cases
explicitly handled work, the others dont. The code (especially networking)
was never designed to cope with a CPU as weak as the early arm designs
are at handling arbitary alignments. You could argue thats a design weakness
and I wouldnt argue that hard but I will take clean patches that dont impact
other platforms..

Alan

---

From: David Alan Gilbert <[email-masked]>
Date: Tue, 06 Jan 1998 10:02:28 +0000

> ie which of these will be wrongly accessed?  And what alignment will GCC

OK - your case above is fine - it will work OK. But here is the case which
started this whole ball rolling:

struct ethhdr {
  char addr1[6];
  char addr2[6];
  short proto;
};

This is the header on an ethernet packet; when it comes in off the wire its
put into a buffer with this structure starting on a 2 byte alginment
address; thats done so that the data which follows (probably a lump
of Tcp (or is it IP?)) will be word aligned.

In the depths of the ethernet code there is something which accesses the
'proto' field from this 2 byte aligned pointer.  Unfortunatly the ARM GCC
assumes that ALL structures are word aligned and generates code to access
the short using a LDR which relies on the word alignment - it thus gets
a dud value for 'proto' and the incoming packet is thrown away because
it can't figure out what type of packet it is.

Why doesn't this cause a problem for other Linux's? All other Linux GCC's
assume that struct's are algined to the size of the largest component within
the struct (upto word size?) and thus they would only assume this was short
aligned and generate appropriate code. (I, like most C programmers I suspect,
had presumed that this was what was ment to happen in C but the GCC guys have
been telling us that the standard actually allows the C implementation to
specify this alignment).

Why hasn't this caused problems before for ARM Linux? Earlier versions of GCC
didn't happen to have optimisations which excercised the problem. Russ had
to do some hacks to the isofs stuff to make it work because.....

Structures get padded:

struct x {
  short a;
};

sizeof(x)=4

and thus if you happen to have a datastructure described using collections
of small structures you get padding all over the place.

> be loaded as bytes!

That isn't what's happening.  The -mshort-load-bytes is turning off the new
optimisation effectivly and causing GCC to emit code which doesn't rely
on the word alignment IN THIS CASE. Its not a general soloution to the
unalignment.

Dave

---

From: Alan Cox <[email-masked]>
Date: Tue, 6 Jan 1998 10:28:26 +0000 (GMT)

> of Tcp (or is it IP?)) will be word aligned.

Actually dont rely on this. Some cards cannot do non 32bit aligned DMA and
those cards will land the packet otherwise aligned.

> aligned and generate appropriate code. (I, like most C programmers I suspect,

Not always - on byte aligned packets - eg IP over AX.25 the alpha and sparc 
will sometimes fault and emulate the faulting instruction. This is faster
than filling the generic cases with byte loads

> been telling us that the standard actually allows the C implementation to

GCC is correct in chosing to align its fields. Its broken in that it doesnt
honour "packed" attributes on the ARM.

Alan

---

From: Philip Blundell <[email-masked]>
Date: Tue, 06 Jan 1998 11:08:53 +0000

>ie which of these will be wrongly accessed?  And what alignment will GCC

In that case, all will be well.  The structure must have 32-bit alignment by 
anybody's standards because of the `int' member.

>and then shift them twice as it is to load as bytes and orr them together.

The problem arises because you don't know what will happen if you load a word 
at arbitrary alignment.

p.

---

From: Philip Blundell <[email-masked]>
Date: Tue, 06 Jan 1998 10:51:03 +0000

>o	In kernel space take exceptions on unaligned load/store (since there

That's no good for ARM3 though. 

>	macros for the ones that show a lot (ie ether_type_trans)

If you don't use -mshort-load-bytes GCC is basically being told that the MMU 
will not trap unaligned accesses.  I believe that without that flag it can 
deliberately misalign loads in order to take advantage of the slightly
idiomatic ARM behaviour. 

p.

---

From: Russell King - ARM Linux Admin <[email-masked]>
Date: Mon, 5 Jan 1998 23:15:04 +0000 (GMT)

> optimisation does produce noticeably better code in the vast majority of cases.

Someone ought to look into the __packed__ attribute in GCC - according to Alan
Cox, it is supposed to tell the compiler to ignore any of these stupid alignments,
for instance, have a look at the changes in fs/isofs/rock.[ch], which originally
was something like (this is from memory):

struct times {
 char time[7];
};

struct RR_TF {
 char c1;
 char c2;
 struct times[3];
}

Now, on the disk, the layout is:

Byte	Contents
+0	c1
+1	c2
+2	times[0].time[0]
...
+8	times[0].time[6]
+9	times[1].time[0]
...
and so on.

However, if left to ARM GCC, then times[0].time[0] will be at +4, and times[1].time[0]
at +12.  Alan claims that if you use __packed__ on these, it should tell GCC that you
want everything to be as small as possible, and that would be correct.  However, ARM
GCC appears to ignore it.

Can someone confirm both the claim, and my observed action of GCC, and confirm if
ARM GCC is correct/wrong.  (And I hope that this doesn't cause another war-of-the
compilers).
   _____
  |_____| ------------------------------------------------- ---+---+-
  |   |        Russell King        [email-masked]        --- ---
  | | | |    http://www.arm.uk.linux.org/~rmk/home.html      /  /  |
  | +-+-+                                                     --- -+-
  /   |               THE developer of ARM Linux              |+| /|\
 /  | | |                                                     ---  |
    +-+-+ -------------------------------------------------  /\\\  |

---

From: "Neil A. Carson" <[email-masked]>
Date: Tue, 06 Jan 1998 19:04:14 +0000

> of small structures you get padding all over the place.

Dunnow if it's of any relevance or not; we hit this with netbsd/arm a
while ago, and made some compiler changes which are now part of the gcc
three, but recognised as not being for general use, I think. Could Linux
not adopt this approach too whih I think would solve any problems? Or
maybe I'm getting the wrong end of the stick, adn the problems are
future ones not present ones.

	N.

---

From: David Alan Gilbert <[email-masked]>
Date: Wed, 07 Jan 1998 09:50:37 +0000

> future ones not present ones.

Which version of the compiler has the mods (or is it a patch you have)?

Dave