RE: [PATCH] MS Bitfields for Interix

["Donn Terry" <donnte at microsoft dot com>]

I was able to look at the patch.

1) It appears to be reversed.  I just took that into account.

2) I may have missed a subtlety (and that's going to require running the
complete
regressions to eliminate), but it appears to me that near each of the
changes there's
an explicit test for it being MS bitfield packing.  If we're making the
policy decision
that __attribute__((packed)) always wins, then it would be clearer (and
more likely
to actually implement that policy) to have both places (and any other
places that
apply) read: 
    (* targetm.ms_bitfield_layout_p) (rli->t) && ! DECL_PACKED (field))

This still leaves a loose end of "you get what you get": if a single
field
is labeled packed then the prior rules may have been different in the
two 
different sets of packing rules.  (If the whole structure is
__attribute__((packed)),
then there's no problem.)

If you concur with the policy decision and that #2 will express that
well, then
we'll go ahead and try things that way.

Donn


-----Original Message-----
From: Joern Rennecke [mailto:joern.rennecke@superh.com] 
Sent: Monday, September 09, 2002 5:25 AM
To: Donn Terry
Cc: Douglas B Rupp; gcc-patches@gcc.gnu.org; aoliva@redhat.com
Subject: Re: [PATCH] MS Bitfields for Interix


Donn Terry wrote:
> The intent of the patch (and my belief about the way it's implemented)

> is that it does the right thing for code which uses MS bitfields and 
> optionally #pragma
> pack.  If the user uses __attribute__((packed)) alone, he gets what
the
> standard
> gcc definition of that is. If the user codes __attribute__((packed))
and
> the MS
> bitfields option, he gets what he gets.  (I suspect that in fact he'd
> get something
> more-or-less reasonable, but it seems worthless to try to specify
> exactly what that
> is, because mixing the options is of no particular value to anyone
that
> cares about
> specific bitfield compatability.)
> 
> (Even as of today, that's the case, when both #pragma pack and
> __attribute__((packed))
> are specified the packing rules take both into account.)
> 
> The intent of the statement in the documentation was simply trying to 
> say "there be dragons here if you do this" without being quite that 
> blunt.  Alternate phrasing
> is solicited if you don't like what's there.  (The proper, but
possibly
> politcally
> incorrect statement is "Don't mix the two, all bets are off if you
> try".)

That is not acceptable.  For the sh64-elf tool chain, MS bitfield layout
is part of the ABI.  And the packed attribute is a gcc feature that is
used quite a lot, and it should do the same target independently. With
your patch, when I recompile libgcc, I get a lot of regressions, because
the floating point emulation code (which is used e.g. for operating
systems that don't want to have the overhead of saving floating point
registers before entering kernel code) uses the packed attribute to
describe the layout of IEEE numbers.

I have attached a patch that goes on top of yours that re-instates the
packed atribute.

It does not, however, fix the compilation of files with structures with
variable sized members.  I don't know what the MS rules are for these
(or do they not exist there?). 
	
-- 
--------------------------
SuperH (UK) Ltd.
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330