public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: make #pragma pack() implementation consistent with other compilers (PR c/7054)
@ 2004-07-07 13:38 Jan Beulich
  2004-07-07 14:00 ` Paul Brook
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Jan Beulich @ 2004-07-07 13:38 UTC (permalink / raw)
  To: jsm; +Cc: gcc-patches

For the documentation requirements (I'm still learning doing 'open
source', and previously I was used to there being someone else taking
care of documentation stuff, after all I'm not a technical writer but a
programmer): Can you suggest an obvious to use editor for the .texi
files (not a plain text one, so I won't have to twiddle the directives
in there a hundred times)?

For the unmentioned test case changes: Without them these tests fail if
you have a target that requires less than word alignment as the default
setting (which is what DEFAULT_PACK_STRUCT is used for). Thus for such
targets the packing value has to be set high enough (and thus the value
8 to accomodate all targets up to 64 bits). It seemed inappropriate to
add options for individual targets here when adding them globally does
not do any harm), but it could of course be done.

I know what your answer will be to the following, but it's really
becoming a pain to try to help solve issues with the compiler when all
you get back is complaints that this and that and a third rule isn't
being fulfilled. I got complaints before stating that I didn't run the
testsuites. I'm running them now. I got complaints that I just built
(not bootstrapped) the compiler. I'm bootstrapping it now. I got
complaints that I'm not diff-ing against the mainline. I'm doing so now.
I got complaints that I didn't write ChangeLog entries. I'm writing them
now. I maybe got other complaints I don't even recall at the moment (I
tried to avoid one by now also building at least all default languages).
No-one seems to care that all these rules require an awful lot of time
(bootstrapping the compiler takes hours, the testsuite runs take hours,
using the mainline is wasted time to me and my employer since we
obviously can't/don't want to use this for general consumption, so
importance to us have only changes on top of a released compiler), and
that instead of doing useful work I'm spending days on fulfilling just
these rules (this might be less extreme if I worked on just a single fix
or a couple of them, but over the last two or three years, prior to
having an FSF copyright assignment, a couple of dozen changes have piled
up and I'd really like to get them out rather than dropping them and
later running into the same problem again). As said above (and as got to
hear previously), everyone's (supposedly) playing by these rules.
Whether everyone's pleased by them doesn't matter. Whether this severly
limits productivity doesn't matter either.

Jan

>>> "Joseph S. Myers" <jsm@polyomino.org.uk> 07.07.04 14:07:45 >>>
On Wed, 7 Jul 2004, Jan Beulich wrote:

> 2004-07-07 Jan Beulich <jbeulich@novell.com>
> 
> 	PR c/7054
> 	* tree.h (initial_max_fld_align): Declare
> 	* stor-layout.c (initial_max_fld_align): Define and initialize.
> 	(maximum_field_alignment): Initialize to the same value.
> 	* common.opt: Add -fpack-struct= variant of switch.
> 	* opts.c: Handle -fpack-struct= variant of switch.
> 	* c-pragma.c: Change #pragma pack() handling so that is becomes
> 	compatible to other compilers: accept individual 'push'
> argument,
> 	make final pop restore (command line) default, correct
> interaction
> 	of push/pop and sole specification of a new alignment (so that
> the
> 	sequence #pragma pack(push) - #pragma pack(<n>) becomes
> identical
> 	to #pragma pack(push, <n>).
> 	* testsuite/gcc.dg/pack-test-2.c: Adjust to permit and check
> 	#pragma pack(push).

We can't consider patches without proper documentation.  This needs (a)
 
documentation for the new command-line option syntax, (b)
documentation
for the pragma changes (and it seems for the pragma itself, which as
you
evidently understand what it's meant to do at present you're best
placed
to write), (c) for what seems to be a new target macro
DEFAULT_PACK_STRUCT.

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08
> 04:32:11.000000000 +0100
> +++
> 2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27
> 10:57:09.000000000 +0200

Not mentioned in the ChangeLog entry.  Why is this testcase being
changed?

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10
> 01:04:50.000000000 +0200
> +++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26
> 15:38:24.000000000 +0200

Likewise.

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17
> 16:48:28.000000000 +0200
> +++
>
2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26
> 17:19:15.000000000 +0200

Likewise.  It seems clearly inappropriate for standards testcases to
have
any special options like you're adding.  (But this testcase is expected
to
become obsolete, as the draft C99 TC2 (N1060) would remove the
defective
requirement from the standard.)

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/ 
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: make #pragma pack() implementation consistent with other compilers (PR c/7054)
@ 2004-07-12  8:17 Jan Beulich
  2004-07-12  9:12 ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2004-07-12  8:17 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]

Second try, (hopefully) with all complaints addressed. Bootstrapped and
tested on i686-pc-linux-gnu. Jan

>>> "Joseph S. Myers" <jsm@polyomino.org.uk> 07.07.04 14:07:45 >>>
On Wed, 7 Jul 2004, Jan Beulich wrote:

> 2004-07-07 Jan Beulich <jbeulich@novell.com>
> 
> 	PR c/7054
> 	* tree.h (initial_max_fld_align): Declare
> 	* stor-layout.c (initial_max_fld_align): Define and initialize.
> 	(maximum_field_alignment): Initialize to the same value.
> 	* common.opt: Add -fpack-struct= variant of switch.
> 	* opts.c: Handle -fpack-struct= variant of switch.
> 	* c-pragma.c: Change #pragma pack() handling so that is becomes
> 	compatible to other compilers: accept individual 'push'
> argument,
> 	make final pop restore (command line) default, correct
> interaction
> 	of push/pop and sole specification of a new alignment (so that
> the
> 	sequence #pragma pack(push) - #pragma pack(<n>) becomes
> identical
> 	to #pragma pack(push, <n>).
> 	* testsuite/gcc.dg/pack-test-2.c: Adjust to permit and check
> 	#pragma pack(push).

We can't consider patches without proper documentation.  This needs (a)
 
documentation for the new command-line option syntax, (b)
documentation
for the pragma changes (and it seems for the pragma itself, which as
you
evidently understand what it's meant to do at present you're best
placed
to write), (c) for what seems to be a new target macro
DEFAULT_PACK_STRUCT.

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08
> 04:32:11.000000000 +0100
> +++
> 2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27
> 10:57:09.000000000 +0200

Not mentioned in the ChangeLog entry.  Why is this testcase being
changed?

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10
> 01:04:50.000000000 +0200
> +++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26
> 15:38:24.000000000 +0200

Likewise.

>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17
> 16:48:28.000000000 +0200
> +++
>
2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26
> 17:19:15.000000000 +0200

Likewise.  It seems clearly inappropriate for standards testcases to
have
any special options like you're adding.  (But this testcase is expected
to
become obsolete, as the draft C99 TC2 (N1060) would remove the
defective
requirement from the standard.)

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/ 
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

[-- Attachment #2: gcc-mainline-pragma-pack.patch --]
[-- Type: application/octet-stream, Size: 18224 bytes --]

2004-07-07 Jan Beulich <jbeulich@novell.com>

	PR c/7054
	* defaults.h (TARGET_DEFAULT_PACK_STRUCT): Provide default.
	* tree.h (initial_max_fld_align): Declare
	* stor-layout.c (initial_max_fld_align): Define and initialize.
	(maximum_field_alignment): Initialize to the same value.
	* common.opt: Add -fpack-struct= variant of switch.
	* opts.c: Handle -fpack-struct= variant of switch.
	* c-pragma.c: Change #pragma pack() handling so that it becomes
	compatible to other compilers: accept individual 'push' argument,
	make final pop restore (command line) default, correct interaction
	of push/pop and sole specification of a new alignment (so that the
	sequence #pragma pack(push) - #pragma pack(<n>) becomes identical
	to #pragma pack(push, <n>).
	* doc/extend.texi: New node "Structure-Packing Pragmas" under
	"Pragmas", describing #pragma pack.
	* doc/invoke.texi: Document -fpack-struct=<n> variant of switch.
	* doc/tm.texi: Adjust description for HANDLE_PRAGMA_PACK_PUSH_POP.
	Document new TARGET_DEFAULT_PACK_STRUCT.

testsuite:
2004-07-07 Jan Beulich <jbeulich@novell.com>
	* gcc.dg/c99-flex-array-4.c: Add -fpack-struct=8 to provide a
	deterministic starting point for the alignment of structure fields,
	as this test may otherwise xpass.
	* gcc.dg/pack-test-2.c: Adjust to permit and check #pragma pack(push).
	* gcc.dg/Wpadded.c: Add -fpack-struct=8 to provide a deterministic
	starting point for the alignment of structure fields, as this test may
	otherwise fail.
	* g++.dg/abi/vbase10.C: Dito.

diff -Naur 2004-07-09.12.23/gcc/common.opt pragma-pack/gcc/common.opt
--- 2004-07-09.12.23/gcc/common.opt	2004-07-02 15:13:19.000000000 +0200
+++ pragma-pack/gcc/common.opt	2004-07-06 08:44:39.000000000 +0200
@@ -527,6 +527,10 @@
 Common Report Var(flag_pack_struct)
 Pack structure members together without holes
 
+fpack-struct=
+Common RejectNegative Joined UInteger
+-fpack-struct=<number>	Set initial maximum structure member alignment
+
 fpcc-struct-return
 Common Report Var(flag_pcc_struct_return,1) VarExists
 Return small aggregates in memory, not registers
diff -Naur 2004-07-09.12.23/gcc/c-pragma.c pragma-pack/gcc/c-pragma.c
--- 2004-07-09.12.23/gcc/c-pragma.c	2004-07-02 15:13:15.000000000 +0200
+++ pragma-pack/gcc/c-pragma.c	2004-07-06 08:44:39.000000000 +0200
@@ -42,7 +42,6 @@
 typedef struct align_stack GTY(())
 {
   int                  alignment;
-  unsigned int         num_pushes;
   tree                 id;
   struct align_stack * prev;
 } align_stack;
@@ -59,8 +58,9 @@
    happens, we restore the value to this, not to a value of 0 for
    maximum_field_alignment.  Value is in bits.  */
 static int default_alignment;
-#define SET_GLOBAL_ALIGNMENT(ALIGN) \
-  (default_alignment = maximum_field_alignment = (ALIGN))
+#define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = *(alignment_stack == NULL \
+	? &default_alignment \
+	: &alignment_stack->alignment) = (ALIGN))
 
 static void push_alignment (int, tree);
 static void pop_alignment (tree);
@@ -69,31 +69,23 @@
 static void
 push_alignment (int alignment, tree id)
 {
-  if (alignment_stack == NULL
-      || alignment_stack->alignment != alignment
-      || id != NULL_TREE)
-    {
-      align_stack * entry;
-
-      entry = ggc_alloc (sizeof (* entry));
-
-      entry->alignment  = alignment;
-      entry->num_pushes = 1;
-      entry->id         = id;
-      entry->prev       = alignment_stack;
-      
-      /* The current value of maximum_field_alignment is not necessarily 
-	 0 since there may be a #pragma pack(<n>) in effect; remember it 
-	 so that we can restore it after the final #pragma pop().  */
-      if (alignment_stack == NULL)
-	default_alignment = maximum_field_alignment;
-      
-      alignment_stack = entry;
+  align_stack * entry;
 
-      maximum_field_alignment = alignment;
-    }
-  else
-    alignment_stack->num_pushes ++;
+  entry = ggc_alloc (sizeof (* entry));
+
+  entry->alignment  = alignment;
+  entry->id         = id;
+  entry->prev       = alignment_stack;
+       
+  /* The current value of maximum_field_alignment is not necessarily 
+     0 since there may be a #pragma pack(<n>) in effect; remember it 
+     so that we can restore it after the final #pragma pop().  */
+  if (alignment_stack == NULL)
+    default_alignment = maximum_field_alignment;
+ 
+  alignment_stack = entry;
+
+  maximum_field_alignment = alignment;
 }
 
 /* Undo a push of an alignment onto the stack.  */
@@ -103,12 +95,7 @@
   align_stack * entry;
       
   if (alignment_stack == NULL)
-    {
-      warning ("\
-#pragma pack (pop) encountered without matching #pragma pack (push, <n>)"
-	       );
-      return;
-    }
+    GCC_BAD("#pragma pack (pop) encountered without matching #pragma pack (push)");
 
   /* If we got an identifier, strip away everything above the target
      entry so that the next step will restore the state just below it.  */
@@ -117,27 +104,20 @@
       for (entry = alignment_stack; entry; entry = entry->prev)
 	if (entry->id == id)
 	  {
-	    entry->num_pushes = 1;
 	    alignment_stack = entry;
 	    break;
 	  }
       if (entry == NULL)
 	warning ("\
-#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s, <n>)"
+#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s)"
 		 , IDENTIFIER_POINTER (id), IDENTIFIER_POINTER (id));
     }
 
-  if (-- alignment_stack->num_pushes == 0)
-    {
-      entry = alignment_stack->prev;
+  entry = alignment_stack->prev;
 
-      if (entry == NULL)
-	maximum_field_alignment = default_alignment;
-      else
-	maximum_field_alignment = entry->alignment;
+  maximum_field_alignment = entry ? entry->alignment : default_alignment;
 
-      alignment_stack = entry;
-    }
+  alignment_stack = entry;
 }
 #else  /* not HANDLE_PRAGMA_PACK_PUSH_POP */
 #define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = (ALIGN))
@@ -150,7 +130,9 @@
 /* #pragma pack ()
    #pragma pack (N)
    
+   #pragma pack (push)
    #pragma pack (push, N)
+   #pragma pack (push, ID)
    #pragma pack (push, ID, N)
    #pragma pack (pop)
    #pragma pack (pop, ID) */
@@ -169,7 +151,7 @@
   if (token == CPP_CLOSE_PAREN)
     {
       action = set;
-      align = 0;
+      align = initial_max_fld_align;
     }
   else if (token == CPP_NUMBER)
     {
@@ -180,8 +162,8 @@
     }
   else if (token == CPP_NAME)
     {
-#define GCC_BAD_ACTION do { if (action == push) \
-	  GCC_BAD ("malformed '#pragma pack(push[, id], <n>)' - ignored"); \
+#define GCC_BAD_ACTION do { if (action != pop) \
+	  GCC_BAD ("malformed '#pragma pack(push[, id][, <n>])' - ignored"); \
 	else \
 	  GCC_BAD ("malformed '#pragma pack(pop[, id])' - ignored"); \
 	} while (0)
@@ -194,31 +176,21 @@
       else
 	GCC_BAD2 ("unknown action '%s' for '#pragma pack' - ignored", op);
 
-      token = c_lex (&x);
-      if (token != CPP_COMMA && action == push)
-	GCC_BAD_ACTION;
-
-      if (token == CPP_COMMA)
+      while ((token = c_lex (&x)) == CPP_COMMA)
 	{
 	  token = c_lex (&x);
-	  if (token == CPP_NAME)
+	  if (token == CPP_NAME && id == 0)
 	    {
 	      id = x;
-	      if (action == push && c_lex (&x) != CPP_COMMA)
-		GCC_BAD_ACTION;
-	      token = c_lex (&x);
 	    }
-
-	  if (action == push)
+	  else if (token == CPP_NUMBER && action == push && align == -1)
 	    {
-	      if (token == CPP_NUMBER)
-		{
-		  align = TREE_INT_CST_LOW (x);
-		  token = c_lex (&x);
-		}
-	      else
-		GCC_BAD_ACTION;
+	      align = TREE_INT_CST_LOW (x);
+	      if (align == -1)
+		action = set;
 	    }
+	  else
+	    GCC_BAD_ACTION;
 	}
 
       if (token != CPP_CLOSE_PAREN)
@@ -231,6 +203,9 @@
   if (c_lex (&x) != CPP_EOF)
     warning ("junk at end of '#pragma pack'");
 
+  if (flag_pack_struct)
+    GCC_BAD ("#pragma pack has no effect with -fpack-struct - ignored");
+
   if (action != pop)
     switch (align)
       {
@@ -242,6 +217,12 @@
       case 16:
 	align *= BITS_PER_UNIT;
 	break;
+      case -1:
+	if (action == push)
+	  {
+	    align = maximum_field_alignment;
+	    break;
+	  }
       default:
 	GCC_BAD2 ("alignment must be a small power of two, not %d", align);
       }
diff -Naur 2004-07-09.12.23/gcc/defaults.h pragma-pack/gcc/defaults.h
--- 2004-07-09.12.23/gcc/defaults.h	2004-06-07 11:55:52.000000000 +0200
+++ pragma-pack/gcc/defaults.h	2004-07-09 14:53:15.638875688 +0200
@@ -461,6 +461,10 @@
 #define PREFERRED_STACK_BOUNDARY STACK_BOUNDARY
 #endif
 
+#ifndef TARGET_DEFAULT_PACK_STRUCT
+#define TARGET_DEFAULT_PACK_STRUCT 0
+#endif
+
 /* By default, the C++ compiler will use function addresses in the
    vtable entries.  Setting this nonzero tells the compiler to use
    function descriptors instead.  The value of this macro says how
diff -Naur 2004-07-09.12.23/gcc/doc/extend.texi pragma-pack/gcc/doc/extend.texi
--- 2004-07-09.12.23/gcc/doc/extend.texi	2004-07-09 12:17:18.000000000 +0200
+++ pragma-pack/gcc/doc/extend.texi	2004-07-09 12:45:45.850820240 +0200
@@ -7210,6 +7210,7 @@
 * RS/6000 and PowerPC Pragmas::
 * Darwin Pragmas::
 * Symbol-Renaming Pragmas::
+* Structure-Packing Pragmas::
 @end menu
 
 @node ARM Pragmas
@@ -7355,6 +7356,30 @@
 way of knowing that that happened.)
 @end enumerate
 
+@node Structure-Packing Pragmas
+@subsection Structure-Packing Pragmas
+
+For compatibility with Win32, GCC supports as set of @code{#pragma}
+directives which change the maximum alignment of members of structures,
+unions, and classes subsequently defined.  The @var{n} value below always
+is required to be a small power of two and specifies the new alignment
+in bytes.
+
+@enumerate
+@item @code{#pragma pack(@var{n})} simply sets the new alignment.
+@item @code{#pragma pack()} sets the alignment to the one that was in
+effect when compilation started (see also command line option
+@option{-fpack-struct[=<n>]} @pxref{Code Gen Options}).
+@item @code{#pragma pack(push[,@var{n}])} pushes the current alignment
+setting on an internal stack and then optionally sets the new alignment.
+@item @code{#pragma pack(pop)} restores the alignment setting to the one
+saved at the top of the internal stack (and removes that stack entry).
+Note that @code{#pragma pack([@var{n}])} does not influence this internal
+stack; thus it is possible to have @code{#pragma pack(push)} followed by
+multiple @code{#pragma pack(@var{n})} instances and finalized by a single
+@code{#pragma pack(pop)}.
+@end enumerate
+
 @node Unnamed Fields
 @section Unnamed struct/union fields within structs/unions.
 @cindex struct
diff -Naur 2004-07-09.12.23/gcc/doc/invoke.texi pragma-pack/gcc/doc/invoke.texi
--- 2004-07-09.12.23/gcc/doc/invoke.texi	2004-07-09 12:17:18.000000000 +0200
+++ pragma-pack/gcc/doc/invoke.texi	2004-07-09 12:45:45.930808080 +0200
@@ -689,7 +689,7 @@
 -fpcc-struct-return  -fpic  -fPIC -fpie -fPIE @gol
 -freg-struct-return  -fshared-data  -fshort-enums @gol
 -fshort-double  -fshort-wchar @gol
--fverbose-asm  -fpack-struct  -fstack-check @gol
+-fverbose-asm  -fpack-struct[=@var{n}]  -fstack-check @gol
 -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym} @gol
 -fargument-alias  -fargument-noalias @gol
 -fargument-noalias-global  -fleading-underscore @gol
@@ -11476,9 +11476,13 @@
 This flag does not have a negative form, because it specifies a
 three-way choice.
 
-@item -fpack-struct
+@item -fpack-struct[=@var{n}]
 @opindex fpack-struct
-Pack all structure members together without holes.
+Without a value specified, pack all structure members together without
+holes. When a value is specified (which must be a small power of two), pack
+structure members according to this value, representing the maximum
+alignment (that is, objects with default alignment requirements larger than
+this will be output potentially unaligned at the next fitting location.
 
 @strong{Warning:} the @option{-fpack-struct} switch causes GCC to generate
 code that is not binary compatible with code generated without that switch.
diff -Naur 2004-07-09.12.23/gcc/doc/tm.texi pragma-pack/gcc/doc/tm.texi
--- 2004-07-09.12.23/gcc/doc/tm.texi	2004-07-07 15:59:38.000000000 +0200
+++ pragma-pack/gcc/doc/tm.texi	2004-07-08 16:51:18.000000000 +0200
@@ -8974,16 +8974,23 @@
 @findex pragma
 @defmac HANDLE_PRAGMA_PACK_PUSH_POP
 Define this macro (to a value of 1) if you want to support the Win32
-style pragmas @samp{#pragma pack(push,@var{n})} and @samp{#pragma
-pack(pop)}.  The @samp{pack(push,@var{n})} pragma specifies the maximum alignment
-(in bytes) of fields within a structure, in much the same way as the
-@samp{__aligned__} and @samp{__packed__} @code{__attribute__}s do.  A
+style pragmas @samp{#pragma pack(push[,@var{n}])} and @samp{#pragma
+pack(pop)}.  The @samp{pack(push,[@var{n}])} pragma specifies the maximum
+alignment (in bytes) of fields within a structure, in much the same way as
+the @samp{__aligned__} and @samp{__packed__} @code{__attribute__}s do.  A
 pack value of zero resets the behavior to the default.  Successive
 invocations of this pragma cause the previous values to be stacked, so
 that invocations of @samp{#pragma pack(pop)} will return to the previous
 value.
 @end defmac
 
+@defmac TARGET_DEFAULT_PACK_STRUCT
+If your target requires a structure packing default other than 0 (meaning
+the machine default), define this macro the the necessary value (in bytes).
+This must be a value that would also valid to be used with
+@samp{#pragma pack()} (that is, a small power of two).
+@end defmac
+
 @defmac DOLLARS_IN_IDENTIFIERS
 Define this macro to control use of the character @samp{$} in
 identifier names for the C family of languages.  0 means @samp{$} is
diff -Naur 2004-07-09.12.23/gcc/opts.c pragma-pack/gcc/opts.c
--- 2004-07-09.12.23/gcc/opts.c	2004-07-09 12:17:13.000000000 +0200
+++ pragma-pack/gcc/opts.c	2004-07-09 12:45:45.999797592 +0200
@@ -790,6 +790,13 @@
       pp_set_line_maximum_length (global_dc->printer, value);
       break;
 
+    case OPT_fpack_struct_:
+      if (value <= 0 || (value & (value - 1)) || value > 16)
+	error("structure alignment must be a small power of two, not %d", value);
+      else
+	maximum_field_alignment = initial_max_fld_align = value * BITS_PER_UNIT;
+      break;
+
     case OPT_fpeel_loops:
       flag_peel_loops_set = true;
       break;
diff -Naur 2004-07-09.12.23/gcc/stor-layout.c pragma-pack/gcc/stor-layout.c
--- 2004-07-09.12.23/gcc/stor-layout.c	2004-07-05 09:18:04.000000000 +0200
+++ pragma-pack/gcc/stor-layout.c	2004-07-08 13:27:05.000000000 +0200
@@ -48,7 +48,9 @@
 
 /* If nonzero, this is an upper limit on alignment of structure fields.
    The value is measured in bits.  */
-unsigned int maximum_field_alignment;
+unsigned int maximum_field_alignment = TARGET_DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
+/* ... and the original value of it, specified via -fpack-struct=<value>. */
+unsigned int initial_max_fld_align = TARGET_DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.
    May be overridden by front-ends.  */
diff -Naur 2004-07-09.12.23/gcc/testsuite/gcc.dg/c99-flex-array-4.c pragma-pack/gcc/testsuite/gcc.dg/c99-flex-array-4.c
--- 2004-07-09.12.23/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17 16:48:28.000000000 +0200
+++ pragma-pack/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26 17:19:15.000000000 +0200
@@ -5,7 +5,7 @@
    from Tony Finch <dot@dotat.at>, adapted to a testcase by Joseph Myers
    <jsm28@cam.ac.uk>.  See also WG14 reflector messages 9571-3.  */
 /* { dg-do compile } */
-/* { dg-options "-std=iso9899:1999 -pedantic-errors" } */
+/* { dg-options "-std=iso9899:1999 -fpack-struct=8 -pedantic-errors" } */
 
 #include <stddef.h>
 
diff -Naur 2004-07-09.12.23/gcc/testsuite/gcc.dg/pack-test-2.c pragma-pack/gcc/testsuite/gcc.dg/pack-test-2.c
--- 2004-07-09.12.23/gcc/testsuite/gcc.dg/pack-test-2.c	2000-12-13 21:16:33.000000000 +0100
+++ pragma-pack/gcc/testsuite/gcc.dg/pack-test-2.c	2004-05-18 16:17:53.000000000 +0200
@@ -3,9 +3,11 @@
 
 /* { dg-do compile { target *-*-linux* *-*-cygwin* powerpc*-*-eabi* } } */
 
-#pragma pack(push)              /* { dg-error "malformed" } */
 #pragma pack(pop)               /* { dg-error "without matching" } */
 
+#pragma pack(push)
+#pragma pack(pop)               /* reset */
+
 #pragma pack(push, foo, 1)
 #pragma pack(pop, foo, 1)       /* { dg-error "malformed" } (/
 #pragma pack(pop)               /* reset */
diff -Naur 2004-07-09.12.23/gcc/testsuite/gcc.dg/Wpadded.c pragma-pack/gcc/testsuite/gcc.dg/Wpadded.c
--- 2004-07-09.12.23/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10 01:04:50.000000000 +0200
+++ pragma-pack/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26 15:38:24.000000000 +0200
@@ -1,7 +1,7 @@
 /* Source: EMC.  */
 
 /* { dg-do compile } */
-/* { dg-options "-Wpadded" } */
+/* { dg-options "-Wpadded -fpack-struct=8" } */
 
 struct foo {
   char bar;
diff -Naur 2004-07-09.12.23/gcc/testsuite/g++.dg/abi/vbase10.C pragma-pack/gcc/testsuite/g++.dg/abi/vbase10.C
--- 2004-07-09.12.23/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08 04:32:11.000000000 +0100
+++ pragma-pack/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27 10:57:09.000000000 +0200
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-options "-Wabi -fabi-version=1" }
+// { dg-options "-Wabi -fabi-version=1 -fpack-struct=8" }
 // On ARM processors, the alignment of B will be 4 even though it
 // contains only a single "char".  That would avoids the situation
 // that the warning below is designed to catch.  We therefore
diff -Naur 2004-07-09.12.23/gcc/tree.h pragma-pack/gcc/tree.h
--- 2004-07-09.12.23/gcc/tree.h	2004-07-09 12:17:14.000000000 +0200
+++ pragma-pack/gcc/tree.h	2004-07-09 12:45:46.064787712 +0200
@@ -3040,8 +3040,10 @@
    + (BITS_PER_UNIT > 8) + (BITS_PER_UNIT > 16) + (BITS_PER_UNIT > 32) \
    + (BITS_PER_UNIT > 64) + (BITS_PER_UNIT > 128) + (BITS_PER_UNIT > 256))
 
-/* If nonzero, an upper limit on alignment of structure fields, in bits.  */
+/* If nonzero, an upper limit on alignment of structure fields, in bits,  */
 extern unsigned int maximum_field_alignment;
+/* and the original value of it, specified via -fpack-struct=<value>. */
+extern unsigned int initial_max_fld_align;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.  */
 extern unsigned int set_alignment;

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: make #pragma pack() implementation consistent with other compilers (PR c/7054)
@ 2004-07-09 14:21 Jan Beulich
  2004-07-09 16:10 ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2004-07-09 14:21 UTC (permalink / raw)
  To: jsm; +Cc: gcc-patches

>>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08
>> 04:32:11.000000000 +0100
>> +++
>> 2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27
>> 10:57:09.000000000 +0200
>
>Not mentioned in the ChangeLog entry.  Why is this testcase being
changed?

Because it makes assumptions about the default alignment, without
enforcing a certain default alignment.

>>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10
>> 01:04:50.000000000 +0200
>> +++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26
>> 15:38:24.000000000 +0200
>
>Likewise.

Similarly, it expects behavior that you can't expect when the structure
packing is too low. Thus I was forcing the packing higher.

>>
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17
>> 16:48:28.000000000 +0200
>> +++
>>
2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26
>> 17:19:15.000000000 +0200
>
>Likewise.  It seems clearly inappropriate for standards testcases to
have
>any special options like you're adding.  (But this testcase is
expected to
>become obsolete, as the draft C99 TC2 (N1060) would remove the
defective
>requirement from the standard.)

Same as above.

Generally, I'd like to understand the action I'm expected to take:
Should I (a) leave the changes as-is, just documenting them in the
ChangeLog, (b) conditionally pass options for *-*-netware*, or (c) xfail
these tests for *-*-netware*. I chose (a) originally because the
modifications aren't really target specific, since any target could
choose to use other than word alignment as the default alignment.

Thanks, Jan

^ permalink raw reply	[flat|nested] 16+ messages in thread
[parent not found: <s0ec276f.038@emea1-mh.id2.novell.com>]
* Re: make #pragma pack() implementation consistent with other   compilers (PR c/7054)
@ 2004-07-07 16:34 Jan Beulich
  2004-07-07 17:02 ` Diego Novillo
  2004-07-07 18:26 ` Jason Merrill
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Beulich @ 2004-07-07 16:34 UTC (permalink / raw)
  To: bonzini; +Cc: gcc-patches

One thing I simply forgot to add before: With all these nice rules, why
is it then that, in order to be sure my changes don't add regression
failures, I have to first bootstrap an unmodified compiler and run the
testsuite on it (so to identify all current failures), in order to then
run the same stuff again to finally compare the set of failures with my
changes to that without.

Note that while this is 'only' machine time, there is an implied
restriction on the number of things you can do on a single machine:
Certain tests appear to take quite long to compile (I measured 4 minutes
on an otherwise idle system), and thus when I run something else equally
computation intensive (i.e. another bootstrap or testsuite) on the same
machine (of which I have only one or at best two that I can afford doing
this sort of stuff), then such tests are prone to time out (and thus
fail). As a consequence, this has to be done serially, which in turn
collides with the intention to do all this against sufficiently recent
shnapshots of the cvs.

Jan

>>> Paolo Bonzini <bonzini@gnu.org> 07.07.04 17:20:18 >>>
> I know what your answer will be to the following, but it's really
> becoming a pain to try to help solve issues with the compiler when
all
> you get back is complaints that this and that and a third rule isn't
> being fulfilled.

All this is explained in http://gcc.gnu.org/contribute.html which is 
prominently linked in the home page.

> (bootstrapping the compiler takes hours, the testsuite runs take
hours,

You can do that at night.  Testing a pile of changes together is
usually 
not a problem, especially if well separated.  Still, once they have
been 
reviewed and approved, you should take your time to test them one at a

time; but then once you got the approval, you can take all the time you

need before actually committing the changes.

> using the mainline is wasted time to me and my employer since we
> obviously can't/don't want to use this for general consumption, so
> importance to us have only changes on top of a released compiler),

Sorry, but that's not a valid point.  I don't think you can believe
that 
GCC developers should pick up your 3.4 work and do the port to mainline

themselves.  No matter if paid or volunteering, they have more 
interesting things to do.  The best solution to this problem is to 
submit the changes just after the beginning of stage1.  I think most
GCC 
developers end stage3 with a queue of at least a dozen polished
patches.

The point is that you/your employer had done it wrong by letting these

changes pile up for two years.

> Whether everyone's pleased by them doesn't matter.
> Whether this severly limits productivity doesn't matter either.

I understand your position.  But the point is that these rules help 
maintainers reviewing your patch, they help developers understand what

others have done, and they help people lend a hand when you have done 
something wrong. Overall they help keeping GCC high quality (which is 
not always easy with the rules in place, either...); sooner or later 
you'll appreciate that at least your productivity is not being limited

by mainline being broken every other day (at least on i686-linux :-).

If you need additional clarifications, of course, replies are welcome 
either privately or on the mailing list.

Paolo

^ permalink raw reply	[flat|nested] 16+ messages in thread
* make #pragma pack() implementation consistent with other compilers (PR c/7054)
@ 2004-07-07 11:57 Jan Beulich
  2004-07-07 12:10 ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2004-07-07 11:57 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 11637 bytes --]

2004-07-07 Jan Beulich <jbeulich@novell.com>

	PR c/7054
	* tree.h (initial_max_fld_align): Declare
	* stor-layout.c (initial_max_fld_align): Define and initialize.
	(maximum_field_alignment): Initialize to the same value.
	* common.opt: Add -fpack-struct= variant of switch.
	* opts.c: Handle -fpack-struct= variant of switch.
	* c-pragma.c: Change #pragma pack() handling so that is becomes
	compatible to other compilers: accept individual 'push'
argument,
	make final pop restore (command line) default, correct
interaction
	of push/pop and sole specification of a new alignment (so that
the
	sequence #pragma pack(push) - #pragma pack(<n>) becomes
identical
	to #pragma pack(push, <n>).
	* testsuite/gcc.dg/pack-test-2.c: Adjust to permit and check
	#pragma pack(push).

---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/c-pragma.c	2004-07-02
15:13:15.000000000 +0200
+++ 2004-07-05.10.09/gcc/c-pragma.c	2004-07-06 08:44:38.943513912
+0200
@@ -42,7 +42,6 @@
 typedef struct align_stack GTY(())
 {
   int                  alignment;
-  unsigned int         num_pushes;
   tree                 id;
   struct align_stack * prev;
 } align_stack;
@@ -59,8 +58,9 @@
    happens, we restore the value to this, not to a value of 0 for
    maximum_field_alignment.  Value is in bits.  */
 static int default_alignment;
-#define SET_GLOBAL_ALIGNMENT(ALIGN) \
-  (default_alignment = maximum_field_alignment = (ALIGN))
+#define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment =
*(alignment_stack == NULL \
+	? &default_alignment \
+	: &alignment_stack->alignment) = (ALIGN))
 
 static void push_alignment (int, tree);
 static void pop_alignment (tree);
@@ -69,31 +69,23 @@
 static void
 push_alignment (int alignment, tree id)
 {
-  if (alignment_stack == NULL
-      || alignment_stack->alignment != alignment
-      || id != NULL_TREE)
-    {
-      align_stack * entry;
-
-      entry = ggc_alloc (sizeof (* entry));
-
-      entry->alignment  = alignment;
-      entry->num_pushes = 1;
-      entry->id         = id;
-      entry->prev       = alignment_stack;
-      
-      /* The current value of maximum_field_alignment is not
necessarily 
-	 0 since there may be a #pragma pack(<n>) in effect; remember it

-	 so that we can restore it after the final #pragma pop().  */
-      if (alignment_stack == NULL)
-	default_alignment = maximum_field_alignment;
-      
-      alignment_stack = entry;
+  align_stack * entry;
 
-      maximum_field_alignment = alignment;
-    }
-  else
-    alignment_stack->num_pushes ++;
+  entry = ggc_alloc (sizeof (* entry));
+
+  entry->alignment  = alignment;
+  entry->id         = id;
+  entry->prev       = alignment_stack;
+       
+  /* The current value of maximum_field_alignment is not necessarily 
+     0 since there may be a #pragma pack(<n>) in effect; remember it 
+     so that we can restore it after the final #pragma pop().  */
+  if (alignment_stack == NULL)
+    default_alignment = maximum_field_alignment;
+ 
+  alignment_stack = entry;
+
+  maximum_field_alignment = alignment;
 }
 
 /* Undo a push of an alignment onto the stack.  */
@@ -103,12 +95,7 @@
   align_stack * entry;
       
   if (alignment_stack == NULL)
-    {
-      warning ("\
-#pragma pack (pop) encountered without matching #pragma pack (push,
<n>)"
-	       );
-      return;
-    }
+    GCC_BAD("#pragma pack (pop) encountered without matching #pragma
pack (push)");
 
   /* If we got an identifier, strip away everything above the target
      entry so that the next step will restore the state just below it.
 */
@@ -117,27 +104,20 @@
       for (entry = alignment_stack; entry; entry = entry->prev)
 	if (entry->id == id)
 	  {
-	    entry->num_pushes = 1;
 	    alignment_stack = entry;
 	    break;
 	  }
       if (entry == NULL)
 	warning ("\
-#pragma pack(pop, %s) encountered without matching #pragma pack(push,
%s, <n>)"
+#pragma pack(pop, %s) encountered without matching #pragma pack(push,
%s)"
 		 , IDENTIFIER_POINTER (id), IDENTIFIER_POINTER (id));
     }
 
-  if (-- alignment_stack->num_pushes == 0)
-    {
-      entry = alignment_stack->prev;
+  entry = alignment_stack->prev;
 
-      if (entry == NULL)
-	maximum_field_alignment = default_alignment;
-      else
-	maximum_field_alignment = entry->alignment;
+  maximum_field_alignment = entry ? entry->alignment :
default_alignment;
 
-      alignment_stack = entry;
-    }
+  alignment_stack = entry;
 }
 #else  /* not HANDLE_PRAGMA_PACK_PUSH_POP */
 #define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment =
(ALIGN))
@@ -150,7 +130,9 @@
 /* #pragma pack ()
    #pragma pack (N)
    
+   #pragma pack (push)
    #pragma pack (push, N)
+   #pragma pack (push, ID)
    #pragma pack (push, ID, N)
    #pragma pack (pop)
    #pragma pack (pop, ID) */
@@ -169,7 +151,7 @@
   if (token == CPP_CLOSE_PAREN)
     {
       action = set;
-      align = 0;
+      align = initial_max_fld_align;
     }
   else if (token == CPP_NUMBER)
     {
@@ -180,8 +162,8 @@
     }
   else if (token == CPP_NAME)
     {
-#define GCC_BAD_ACTION do { if (action == push) \
-	  GCC_BAD ("malformed '#pragma pack(push[, id], <n>)' -
ignored"); \
+#define GCC_BAD_ACTION do { if (action != pop) \
+	  GCC_BAD ("malformed '#pragma pack(push[, id][, <n>])' -
ignored"); \
 	else \
 	  GCC_BAD ("malformed '#pragma pack(pop[, id])' - ignored"); \
 	} while (0)
@@ -194,31 +176,21 @@
       else
 	GCC_BAD2 ("unknown action '%s' for '#pragma pack' - ignored",
op);
 
-      token = c_lex (&x);
-      if (token != CPP_COMMA && action == push)
-	GCC_BAD_ACTION;
-
-      if (token == CPP_COMMA)
+      while ((token = c_lex (&x)) == CPP_COMMA)
 	{
 	  token = c_lex (&x);
-	  if (token == CPP_NAME)
+	  if (token == CPP_NAME && id == 0)
 	    {
 	      id = x;
-	      if (action == push && c_lex (&x) != CPP_COMMA)
-		GCC_BAD_ACTION;
-	      token = c_lex (&x);
 	    }
-
-	  if (action == push)
+	  else if (token == CPP_NUMBER && action == push && align ==
-1)
 	    {
-	      if (token == CPP_NUMBER)
-		{
-		  align = TREE_INT_CST_LOW (x);
-		  token = c_lex (&x);
-		}
-	      else
-		GCC_BAD_ACTION;
+	      align = TREE_INT_CST_LOW (x);
+	      if (align == -1)
+		action = set;
 	    }
+	  else
+	    GCC_BAD_ACTION;
 	}
 
       if (token != CPP_CLOSE_PAREN)
@@ -231,6 +203,9 @@
   if (c_lex (&x) != CPP_EOF)
     warning ("junk at end of '#pragma pack'");
 
+  if (flag_pack_struct)
+    GCC_BAD ("#pragma pack has no effect with -fpack-struct -
ignored");
+
   if (action != pop)
     switch (align)
       {
@@ -242,6 +217,12 @@
       case 16:
 	align *= BITS_PER_UNIT;
 	break;
+      case -1:
+	if (action == push)
+	  {
+	    align = maximum_field_alignment;
+	    break;
+	  }
       default:
 	GCC_BAD2 ("alignment must be a small power of two, not %d",
align);
       }
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/common.opt	2004-07-02
15:13:19.000000000 +0200
+++ 2004-07-05.10.09/gcc/common.opt	2004-07-06 08:44:38.935515128
+0200
@@ -527,6 +527,10 @@
 Common Report Var(flag_pack_struct)
 Pack structure members together without holes
 
+fpack-struct=
+Common RejectNegative Joined UInteger
+-fpack-struct=<number>	Set initial maximum structure member
alignment
+
 fpcc-struct-return
 Common Report Var(flag_pcc_struct_return,1) VarExists
 Return small aggregates in memory, not registers
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/opts.c	2004-07-02
15:13:53.000000000 +0200
+++ 2004-07-05.10.09/gcc/opts.c	2004-07-06 08:44:38.969509960
+0200
@@ -790,6 +790,13 @@
       pp_set_line_maximum_length (global_dc->printer, value);
       break;
 
+    case OPT_fpack_struct_:
+      if (value <= 0 || (value & (value - 1)) || value > 16)
+	error("structure alignment must be a small power of two, not
%d", value);
+      else
+	maximum_field_alignment = initial_max_fld_align = value *
BITS_PER_UNIT;
+      break;
+
     case OPT_fpeel_loops:
       flag_peel_loops_set = true;
       break;
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/stor-layout.c	2004-07-05
09:18:04.000000000 +0200
+++ 2004-07-05.10.09/gcc/stor-layout.c	2004-07-06 08:44:38.986507376
+0200
@@ -48,7 +48,12 @@
 
 /* If nonzero, this is an upper limit on alignment of structure
fields.
    The value is measured in bits.  */
-unsigned int maximum_field_alignment;
+#ifndef DEFAULT_PACK_STRUCT
+#define DEFAULT_PACK_STRUCT 0
+#endif
+unsigned int maximum_field_alignment = DEFAULT_PACK_STRUCT *
BITS_PER_UNIT;
+/* ... and the original value of it, specified via
-fpack-struct=<value>. */
+unsigned int initial_max_fld_align = DEFAULT_PACK_STRUCT *
BITS_PER_UNIT;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in
bits.
    May be overridden by front-ends.  */
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08
04:32:11.000000000 +0100
+++
2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27
10:57:09.000000000 +0200
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-options "-Wabi -fabi-version=1" }
+// { dg-options "-Wabi -fabi-version=1 -fpack-struct=8" }
 // On ARM processors, the alignment of B will be 4 even though it
 // contains only a single "char".  That would avoids the situation
 // that the warning below is designed to catch.  We therefore
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10
01:04:50.000000000 +0200
+++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26
15:38:24.000000000 +0200
@@ -1,7 +1,7 @@
 /* Source: EMC.  */
 
 /* { dg-do compile } */
-/* { dg-options "-Wpadded" } */
+/* { dg-options "-Wpadded -fpack-struct=8" } */
 
 struct foo {
   char bar;
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17
16:48:28.000000000 +0200
+++
2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26
17:19:15.000000000 +0200
@@ -5,7 +5,7 @@
    from Tony Finch <dot@dotat.at>, adapted to a testcase by Joseph
Myers
    <jsm28@cam.ac.uk>.  See also WG14 reflector messages 9571-3.  */
 /* { dg-do compile } */
-/* { dg-options "-std=iso9899:1999 -pedantic-errors" } */
+/* { dg-options "-std=iso9899:1999 -fpack-struct=8 -pedantic-errors" }
*/
 
 #include <stddef.h>
 
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/pack-test-2.c	2000-12-13
21:16:33.000000000 +0100
+++
2004-07-05.10.09/gcc/testsuite/gcc.dg/pack-test-2.c	2004-05-18
16:17:53.000000000 +0200
@@ -3,9 +3,11 @@
 
 /* { dg-do compile { target *-*-linux* *-*-cygwin* powerpc*-*-eabi* }
} */
 
-#pragma pack(push)              /* { dg-error "malformed" } */
 #pragma pack(pop)               /* { dg-error "without matching" } */
 
+#pragma pack(push)
+#pragma pack(pop)               /* reset */
+
 #pragma pack(push, foo, 1)
 #pragma pack(pop, foo, 1)       /* { dg-error "malformed" } (/
 #pragma pack(pop)               /* reset */
---
/home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/tree.h	2004-07-05
09:18:05.000000000 +0200
+++ 2004-07-05.10.09/gcc/tree.h	2004-07-06 08:44:39.045498408
+0200
@@ -3026,8 +3026,10 @@
    + (BITS_PER_UNIT > 8) + (BITS_PER_UNIT > 16) + (BITS_PER_UNIT > 32)
\
    + (BITS_PER_UNIT > 64) + (BITS_PER_UNIT > 128) + (BITS_PER_UNIT >
256))
 
-/* If nonzero, an upper limit on alignment of structure fields, in
bits.  */
+/* If nonzero, an upper limit on alignment of structure fields, in
bits,  */
 extern unsigned int maximum_field_alignment;
+/* and the original value of it, specified via -fpack-struct=<value>.
*/
+extern unsigned int initial_max_fld_align;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in
bits.  */
 extern unsigned int set_alignment;


[-- Attachment #2: gcc-mainline-pragma-pack.patch --]
[-- Type: application/octet-stream, Size: 11636 bytes --]

2004-07-07 Jan Beulich <jbeulich@novell.com>

	PR c/7054
	* tree.h (initial_max_fld_align): Declare
	* stor-layout.c (initial_max_fld_align): Define and initialize.
	(maximum_field_alignment): Initialize to the same value.
	* common.opt: Add -fpack-struct= variant of switch.
	* opts.c: Handle -fpack-struct= variant of switch.
	* c-pragma.c: Change #pragma pack() handling so that is becomes
	compatible to other compilers: accept individual 'push' argument,
	make final pop restore (command line) default, correct interaction
	of push/pop and sole specification of a new alignment (so that the
	sequence #pragma pack(push) - #pragma pack(<n>) becomes identical
	to #pragma pack(push, <n>).
	* testsuite/gcc.dg/pack-test-2.c: Adjust to permit and check
	#pragma pack(push).

--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/c-pragma.c	2004-07-02 15:13:15.000000000 +0200
+++ 2004-07-05.10.09/gcc/c-pragma.c	2004-07-06 08:44:38.943513912 +0200
@@ -42,7 +42,6 @@
 typedef struct align_stack GTY(())
 {
   int                  alignment;
-  unsigned int         num_pushes;
   tree                 id;
   struct align_stack * prev;
 } align_stack;
@@ -59,8 +58,9 @@
    happens, we restore the value to this, not to a value of 0 for
    maximum_field_alignment.  Value is in bits.  */
 static int default_alignment;
-#define SET_GLOBAL_ALIGNMENT(ALIGN) \
-  (default_alignment = maximum_field_alignment = (ALIGN))
+#define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = *(alignment_stack == NULL \
+	? &default_alignment \
+	: &alignment_stack->alignment) = (ALIGN))
 
 static void push_alignment (int, tree);
 static void pop_alignment (tree);
@@ -69,31 +69,23 @@
 static void
 push_alignment (int alignment, tree id)
 {
-  if (alignment_stack == NULL
-      || alignment_stack->alignment != alignment
-      || id != NULL_TREE)
-    {
-      align_stack * entry;
-
-      entry = ggc_alloc (sizeof (* entry));
-
-      entry->alignment  = alignment;
-      entry->num_pushes = 1;
-      entry->id         = id;
-      entry->prev       = alignment_stack;
-      
-      /* The current value of maximum_field_alignment is not necessarily 
-	 0 since there may be a #pragma pack(<n>) in effect; remember it 
-	 so that we can restore it after the final #pragma pop().  */
-      if (alignment_stack == NULL)
-	default_alignment = maximum_field_alignment;
-      
-      alignment_stack = entry;
+  align_stack * entry;
 
-      maximum_field_alignment = alignment;
-    }
-  else
-    alignment_stack->num_pushes ++;
+  entry = ggc_alloc (sizeof (* entry));
+
+  entry->alignment  = alignment;
+  entry->id         = id;
+  entry->prev       = alignment_stack;
+       
+  /* The current value of maximum_field_alignment is not necessarily 
+     0 since there may be a #pragma pack(<n>) in effect; remember it 
+     so that we can restore it after the final #pragma pop().  */
+  if (alignment_stack == NULL)
+    default_alignment = maximum_field_alignment;
+ 
+  alignment_stack = entry;
+
+  maximum_field_alignment = alignment;
 }
 
 /* Undo a push of an alignment onto the stack.  */
@@ -103,12 +95,7 @@
   align_stack * entry;
       
   if (alignment_stack == NULL)
-    {
-      warning ("\
-#pragma pack (pop) encountered without matching #pragma pack (push, <n>)"
-	       );
-      return;
-    }
+    GCC_BAD("#pragma pack (pop) encountered without matching #pragma pack (push)");
 
   /* If we got an identifier, strip away everything above the target
      entry so that the next step will restore the state just below it.  */
@@ -117,27 +104,20 @@
       for (entry = alignment_stack; entry; entry = entry->prev)
 	if (entry->id == id)
 	  {
-	    entry->num_pushes = 1;
 	    alignment_stack = entry;
 	    break;
 	  }
       if (entry == NULL)
 	warning ("\
-#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s, <n>)"
+#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s)"
 		 , IDENTIFIER_POINTER (id), IDENTIFIER_POINTER (id));
     }
 
-  if (-- alignment_stack->num_pushes == 0)
-    {
-      entry = alignment_stack->prev;
+  entry = alignment_stack->prev;
 
-      if (entry == NULL)
-	maximum_field_alignment = default_alignment;
-      else
-	maximum_field_alignment = entry->alignment;
+  maximum_field_alignment = entry ? entry->alignment : default_alignment;
 
-      alignment_stack = entry;
-    }
+  alignment_stack = entry;
 }
 #else  /* not HANDLE_PRAGMA_PACK_PUSH_POP */
 #define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = (ALIGN))
@@ -150,7 +130,9 @@
 /* #pragma pack ()
    #pragma pack (N)
    
+   #pragma pack (push)
    #pragma pack (push, N)
+   #pragma pack (push, ID)
    #pragma pack (push, ID, N)
    #pragma pack (pop)
    #pragma pack (pop, ID) */
@@ -169,7 +151,7 @@
   if (token == CPP_CLOSE_PAREN)
     {
       action = set;
-      align = 0;
+      align = initial_max_fld_align;
     }
   else if (token == CPP_NUMBER)
     {
@@ -180,8 +162,8 @@
     }
   else if (token == CPP_NAME)
     {
-#define GCC_BAD_ACTION do { if (action == push) \
-	  GCC_BAD ("malformed '#pragma pack(push[, id], <n>)' - ignored"); \
+#define GCC_BAD_ACTION do { if (action != pop) \
+	  GCC_BAD ("malformed '#pragma pack(push[, id][, <n>])' - ignored"); \
 	else \
 	  GCC_BAD ("malformed '#pragma pack(pop[, id])' - ignored"); \
 	} while (0)
@@ -194,31 +176,21 @@
       else
 	GCC_BAD2 ("unknown action '%s' for '#pragma pack' - ignored", op);
 
-      token = c_lex (&x);
-      if (token != CPP_COMMA && action == push)
-	GCC_BAD_ACTION;
-
-      if (token == CPP_COMMA)
+      while ((token = c_lex (&x)) == CPP_COMMA)
 	{
 	  token = c_lex (&x);
-	  if (token == CPP_NAME)
+	  if (token == CPP_NAME && id == 0)
 	    {
 	      id = x;
-	      if (action == push && c_lex (&x) != CPP_COMMA)
-		GCC_BAD_ACTION;
-	      token = c_lex (&x);
 	    }
-
-	  if (action == push)
+	  else if (token == CPP_NUMBER && action == push && align == -1)
 	    {
-	      if (token == CPP_NUMBER)
-		{
-		  align = TREE_INT_CST_LOW (x);
-		  token = c_lex (&x);
-		}
-	      else
-		GCC_BAD_ACTION;
+	      align = TREE_INT_CST_LOW (x);
+	      if (align == -1)
+		action = set;
 	    }
+	  else
+	    GCC_BAD_ACTION;
 	}
 
       if (token != CPP_CLOSE_PAREN)
@@ -231,6 +203,9 @@
   if (c_lex (&x) != CPP_EOF)
     warning ("junk at end of '#pragma pack'");
 
+  if (flag_pack_struct)
+    GCC_BAD ("#pragma pack has no effect with -fpack-struct - ignored");
+
   if (action != pop)
     switch (align)
       {
@@ -242,6 +217,12 @@
       case 16:
 	align *= BITS_PER_UNIT;
 	break;
+      case -1:
+	if (action == push)
+	  {
+	    align = maximum_field_alignment;
+	    break;
+	  }
       default:
 	GCC_BAD2 ("alignment must be a small power of two, not %d", align);
       }
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/common.opt	2004-07-02 15:13:19.000000000 +0200
+++ 2004-07-05.10.09/gcc/common.opt	2004-07-06 08:44:38.935515128 +0200
@@ -527,6 +527,10 @@
 Common Report Var(flag_pack_struct)
 Pack structure members together without holes
 
+fpack-struct=
+Common RejectNegative Joined UInteger
+-fpack-struct=<number>	Set initial maximum structure member alignment
+
 fpcc-struct-return
 Common Report Var(flag_pcc_struct_return,1) VarExists
 Return small aggregates in memory, not registers
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/opts.c	2004-07-02 15:13:53.000000000 +0200
+++ 2004-07-05.10.09/gcc/opts.c	2004-07-06 08:44:38.969509960 +0200
@@ -790,6 +790,13 @@
       pp_set_line_maximum_length (global_dc->printer, value);
       break;
 
+    case OPT_fpack_struct_:
+      if (value <= 0 || (value & (value - 1)) || value > 16)
+	error("structure alignment must be a small power of two, not %d", value);
+      else
+	maximum_field_alignment = initial_max_fld_align = value * BITS_PER_UNIT;
+      break;
+
     case OPT_fpeel_loops:
       flag_peel_loops_set = true;
       break;
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/stor-layout.c	2004-07-05 09:18:04.000000000 +0200
+++ 2004-07-05.10.09/gcc/stor-layout.c	2004-07-06 08:44:38.986507376 +0200
@@ -48,7 +48,12 @@
 
 /* If nonzero, this is an upper limit on alignment of structure fields.
    The value is measured in bits.  */
-unsigned int maximum_field_alignment;
+#ifndef DEFAULT_PACK_STRUCT
+#define DEFAULT_PACK_STRUCT 0
+#endif
+unsigned int maximum_field_alignment = DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
+/* ... and the original value of it, specified via -fpack-struct=<value>. */
+unsigned int initial_max_fld_align = DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.
    May be overridden by front-ends.  */
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-01-08 04:32:11.000000000 +0100
+++ 2004-07-05.10.09/gcc/testsuite/g++.dg/abi/vbase10.C	2004-05-27 10:57:09.000000000 +0200
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-options "-Wabi -fabi-version=1" }
+// { dg-options "-Wabi -fabi-version=1 -fpack-struct=8" }
 // On ARM processors, the alignment of B will be 4 even though it
 // contains only a single "char".  That would avoids the situation
 // that the warning below is designed to catch.  We therefore
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2003-06-10 01:04:50.000000000 +0200
+++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/Wpadded.c	2004-05-26 15:38:24.000000000 +0200
@@ -1,7 +1,7 @@
 /* Source: EMC.  */
 
 /* { dg-do compile } */
-/* { dg-options "-Wpadded" } */
+/* { dg-options "-Wpadded -fpack-struct=8" } */
 
 struct foo {
   char bar;
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2002-08-17 16:48:28.000000000 +0200
+++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/c99-flex-array-4.c	2004-05-26 17:19:15.000000000 +0200
@@ -5,7 +5,7 @@
    from Tony Finch <dot@dotat.at>, adapted to a testcase by Joseph Myers
    <jsm28@cam.ac.uk>.  See also WG14 reflector messages 9571-3.  */
 /* { dg-do compile } */
-/* { dg-options "-std=iso9899:1999 -pedantic-errors" } */
+/* { dg-options "-std=iso9899:1999 -fpack-struct=8 -pedantic-errors" } */
 
 #include <stddef.h>
 
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/testsuite/gcc.dg/pack-test-2.c	2000-12-13 21:16:33.000000000 +0100
+++ 2004-07-05.10.09/gcc/testsuite/gcc.dg/pack-test-2.c	2004-05-18 16:17:53.000000000 +0200
@@ -3,9 +3,11 @@
 
 /* { dg-do compile { target *-*-linux* *-*-cygwin* powerpc*-*-eabi* } } */
 
-#pragma pack(push)              /* { dg-error "malformed" } */
 #pragma pack(pop)               /* { dg-error "without matching" } */
 
+#pragma pack(push)
+#pragma pack(pop)               /* reset */
+
 #pragma pack(push, foo, 1)
 #pragma pack(pop, foo, 1)       /* { dg-error "malformed" } (/
 #pragma pack(pop)               /* reset */
--- /home/jbeulich/src/gcc/mainline/2004-07-05.10.09/gcc/tree.h	2004-07-05 09:18:05.000000000 +0200
+++ 2004-07-05.10.09/gcc/tree.h	2004-07-06 08:44:39.045498408 +0200
@@ -3026,8 +3026,10 @@
    + (BITS_PER_UNIT > 8) + (BITS_PER_UNIT > 16) + (BITS_PER_UNIT > 32) \
    + (BITS_PER_UNIT > 64) + (BITS_PER_UNIT > 128) + (BITS_PER_UNIT > 256))
 
-/* If nonzero, an upper limit on alignment of structure fields, in bits.  */
+/* If nonzero, an upper limit on alignment of structure fields, in bits,  */
 extern unsigned int maximum_field_alignment;
+/* and the original value of it, specified via -fpack-struct=<value>. */
+extern unsigned int initial_max_fld_align;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.  */
 extern unsigned int set_alignment;

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2004-07-12  8:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-07-07 13:38 make #pragma pack() implementation consistent with other compilers (PR c/7054) Jan Beulich
2004-07-07 14:00 ` Paul Brook
2004-07-07 15:51 ` Paolo Bonzini
2004-07-07 15:59   ` Paolo Bonzini
2004-07-07 16:30 ` Joseph S. Myers
2004-07-08 16:34 ` Roger Sayle
  -- strict thread matches above, loose matches on Subject: below --
2004-07-12  8:17 Jan Beulich
2004-07-12  9:12 ` Joseph S. Myers
2004-07-09 14:21 Jan Beulich
2004-07-09 16:10 ` Joseph S. Myers
     [not found] <s0ec276f.038@emea1-mh.id2.novell.com>
2004-07-07 17:49 ` Paolo Bonzini
2004-07-07 16:34 Jan Beulich
2004-07-07 17:02 ` Diego Novillo
2004-07-07 18:26 ` Jason Merrill
2004-07-07 11:57 Jan Beulich
2004-07-07 12:10 ` Joseph S. Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).