Re: [PATCH] MS Bitfields for Interix

[Joern Rennecke <joern dot rennecke at superh dot com>]

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
Mon Sep  9 13:09:05 2002  J"orn Rennecke <joern.rennecke@superh.com>

	* stor-layout.c (palce_field) Honour DECL_PACKED in
	MS bitfield code.

*** stor-layout.c	Mon Sep  9 12:25:49 2002
--- stor-layout.c-0906	Fri Sep  6 13:28:30 2002
*************** place_field (rli, field)
*** 810,819 ****
  	 applies if there was an immediately prior, non-zero-size
  	 bitfield.  (That's the way it is, experimentally.) */
        if (! integer_zerop (DECL_SIZE (field))
! 	  ? ! DECL_PACKED (field)
! 	  : (rli->prev_field
! 	     && DECL_BIT_FIELD_TYPE (rli->prev_field)
! 	     && ! integer_zerop (DECL_SIZE (rli->prev_field))))
  	{
  	  unsigned int type_align = TYPE_ALIGN (type);
  	  type_align = MAX (type_align, desired_align);
--- 810,818 ----
  	 applies if there was an immediately prior, non-zero-size
  	 bitfield.  (That's the way it is, experimentally.) */
        if (! integer_zerop (DECL_SIZE (field))
! 	  || (rli->prev_field
! 	      && DECL_BIT_FIELD_TYPE (rli->prev_field)
! 	      && ! integer_zerop (DECL_SIZE (rli->prev_field))))
  	{
  	  unsigned int type_align = TYPE_ALIGN (type);
  	  type_align = MAX (type_align, desired_align);
*************** place_field (rli, field)
*** 1018,1025 ****
       to determine alignment, since that matches the documentation */
  
    if ((* targetm.ms_bitfield_layout_p) (rli->t)
!       && ((DECL_BIT_FIELD_TYPE (field) && ! DECL_PACKED (field))
! 	  || (rli->prev_field && ! DECL_PACKED (field))))
      {
        /* At this point, either the prior or current are bitfields,
  	 (possibly both), and we're dealing with MS packing. */
--- 1017,1024 ----
       to determine alignment, since that matches the documentation */
  
    if ((* targetm.ms_bitfield_layout_p) (rli->t)
!       && (DECL_BIT_FIELD_TYPE(field)
! 	 || rli->prev_field))
      {
        /* At this point, either the prior or current are bitfields,
  	 (possibly both), and we're dealing with MS packing. */