On Sun, 21 Oct 2007 16:02:15 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

quoted text
> From: Pierre Ossman <drzeus@drzeus.cx>
> Date: Sun, 21 Oct 2007 23:21:13 +0200
>=20
> > Not sure that would be valid. memcpy() is defined as having void*
> > arguments, and the compiler cannot just ignore that if it chooses to
> > inline it.
>=20
> Yes it can, there are C language rules about the alignment of types
> that the compiler completely can take advantage of in those kinds of
> situations.
>=20

I'm not debating that. What I'm saying is that calling memcpy() casts your =
pointers to void* with the included semantical changes. It can't just ignor=
e that because it decides to inline the function. It would be the same thin=
g as when gcc decided to ignore the volatile qualifier on a pointer just be=
cause it could optimize away to the real object and discover it wasn't mark=
ed with volatile. Something that was considered a bug and was fixed.

quoted text
> If you don't believe me, compile something like the following
> with optimizations enabled:

gcc has had bugs in the past.

quoted text
> You will get a 64-bit load and a 64-bit store emitted by
> the compiler.  Here is what we get on sparc64:
>=20

I assume those ops cause a bus error on unaligned addresses?

quoted text
>=20
> However, instead of relying upon magic like this, let's just tell the
> compiler explicitly what it going on by using get_unaligned().
>=20

It wouldn't be magic:

memcpy(&icmp_param.data.icmph, skb_transport_header(skb), sizeof(struct icm=
phdr));

I believe platforms without alignment requirements could optimize this bett=
er than the series of assignments. Not that I think this will be a potentia=
l bottle neck, but still.

quoted text
> Next, there are redundant stores being done here since the code and
> type are explicitly overwritten in various ways.

Indeed.

Rgds
Pierre