cvs commit: src/include ar.h

[Ruslan Ermilov]

On Mon, Nov 13, 2006 at 10:19:29PM +1100, Bruce Evans wrote:
> On Mon, 13 Nov 2006, Stefan Farfeleder wrote:
[...]
> >You seem to have missed the discussion on -current. GCC pads "struct foo
> >{ char c; };" to 4 bytes on ARM.  I agree on __packed not being
> >portable.
> 
> I don't read -current.  Peharps I shouldn't comment on it.
> 
> I think it is a bug for ARM to do that.  If __packed actually gives a size
> of 1, then the padding must be only an optimization for time.
> 
I thought so as well.  Please see this email and emails surrounding it:

http://lists.freebsd.org/pipermail/freebsd-arm/2006-November/000198.html

> struct ar's natural size is 60, so __packed is unnecessary for ARM too
> (unless there is internal padding which would cause more problems).
> 
Adding __packed didn't change sizeof(struct ar) on FreeBSD/arm but removed
the 4-byte alignment requirement for it, which is what we were attacking.
It now generates less optimal assembly (byte loads instead of 32-bit word
loads), but a discussion on -arm suggested that it's ok for this struct
to be packed.

ISO C99 is clear that padding (except at the beginning) is compiler/machine
dependent:

: Within a structure object, the non-bit-field members and the units
: in which bit-fields reside have addresses that increase in the order
: in which they are declared. A pointer to a structure object, suitably
: converted, points to its initial member (or if that member is a
: bit-field, then to the unit in which it resides), and vice versa.
: There may be unnamed padding within a structure object, but not at
: its beginning.

So there doesn't seem to be a portable way to "lay" structures on
externally supplied data, e.g., data read from files.  It means that
we'd have to use bytestrings loads that Poul-Henning mentioned to be
portable, but I think a common sense should win between imaginary
portability and practical performance -- it's often not desirable to
copy a data again, fully or in part, just for the sake of portability
(e.g. in network code) if we know how our beloved compiler packs the
data.  And if we ever switch to a compiler with different packing rules
(packing rules in GCC seems to be "natural" in your speak), a LOT of
our programs will break.  Again, we added __packed here only for its
(documented) side effect to lower the alignment requirement, not
packing.

In practice, adding __packed on this structure (which is "all chars")
and is already "naturally packed", doesn't have any effect on
assembly on all platforms that don't REQUIRE an unnatural alignment
(i.e., anything except ARM).  And by using bytestring loading instead
we'd loose performance on all architectures.  This OTOH penalizes
code on ARM, but I think that IN THIS PARTICULAR CASE of a structure
of all chars it's okay.

I agree that -Wpacked should probably be used to identify harmful
instances of __packed.


Cheers,
-- 
Ruslan Ermilov
ru at FreeBSD.org
FreeBSD committer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-all/attachments/20061113/f9086725/attachment.pgp