This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Take 2: Fix PR37216 [cygming] Invalid alignment for SSE store to .comm data generated with -O3


    Hi,

  This is the revised version of the patch to fix PR32716, with all
considerations from the threads earlier today about tests and zero-length
commons(*) taken into account.  Now going for a full bootstrap and test,
results in a couple of days.

gcc/ChangeLog:

	PR target/37216

	* configure.ac (HAVE_GAS_ALIGNED_COMM):  Add autoconf test and
	macro definition for support of three-operand format aligned
	.comm directive in assembler on cygwin/pe/mingw target OS.
	* configure:  Regenerate.
	* config.h:  Regenerate.

	* config/i386/winnt.c (i386_pe_asm_output_aligned_decl_common):  Use
	aligned form of .comm directive if -mpe-aligned-commons is in effect.
	* config/i386/cygming.opt (-mpe-aligned-commons):  Add new option.

	* doc/invoke.texi (-mpe-aligned-commons):  Document new target option.
	* doc/tm.texi (ASM_OUTPUT_COMMON):  Document zero size commons.

gcc/testsuite/ChangeLog:

	PR target/37216

	* lib/target-supports.exp (check_effective_target_pe_aligned_commons):
	New function.
	* gcc.target/i386/pr37216.c:  New test source file.
	* gcc.dg/compat/struct-layout-1_generate.c (dg_options[]):  No longer
	use -fno-common for testing Cygwin and MinGW targets.

  OK if the whole thing completes without regressions?

    cheers,
      DaveK
-- 
(*) - http://gcc.gnu.org/ml/gcc-patches/2009-05/threads.html#01528
      http://gcc.gnu.org/ml/gcc/2009-05/threads.html#00581
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 147710)
+++ gcc/configure.ac	(working copy)
@@ -2954,6 +2954,15 @@ changequote(,)dnl
 changequote([,])dnl
     case $target_os in
       cygwin* | pe | mingw32*)
+	# Recent binutils allows the three-operand form of ".comm" on PE.  This
+	# definition is used unconditionally to initialise the default state of
+	# the target option variable that governs usage of the feature.
+	gcc_GAS_CHECK_FEATURE([.comm with alignment], gcc_cv_as_comm_has_align,
+	 [2,19,52],,[.comm foo,1,32])
+	AC_DEFINE_UNQUOTED(HAVE_GAS_ALIGNED_COMM,
+	  [`if test $gcc_cv_as_comm_has_align = yes; then echo 1; else echo 0; fi`],
+	  [Define if your assembler supports specifying the alignment
+	   of objects allocated using the GAS .comm command.])
 	# Used for DWARF 2 in PE
 	gcc_GAS_CHECK_FEATURE([.secrel32 relocs],
 	  gcc_cv_as_ix86_pe_secrel32,
Index: gcc/config/i386/winnt.c
===================================================================
--- gcc/config/i386/winnt.c	(revision 147710)
+++ gcc/config/i386/winnt.c	(working copy)
@@ -499,8 +499,11 @@ i386_pe_asm_output_aligned_decl_common (FILE *stre
 {
   HOST_WIDE_INT rounded;
 
-  /* Compute as in assemble_noswitch_variable, since we don't actually
-     support aligned common.  */
+  /* Compute as in assemble_noswitch_variable, since we don't have
+     support for aligned common on older binutils.  We must also
+     avoid emitting a common symbol of size zero, as this is the
+     overloaded representation that indicates an undefined external
+     symbol in the PE object file format.  */
   rounded = size ? size : 1;
   rounded += (BIGGEST_ALIGNMENT / BITS_PER_UNIT) - 1;
   rounded = (rounded / (BIGGEST_ALIGNMENT / BITS_PER_UNIT)
@@ -510,9 +513,13 @@ i386_pe_asm_output_aligned_decl_common (FILE *stre
 
   fprintf (stream, "\t.comm\t");
   assemble_name (stream, name);
-  fprintf (stream, ", " HOST_WIDE_INT_PRINT_DEC "\t" ASM_COMMENT_START
-	   " " HOST_WIDE_INT_PRINT_DEC "\n",
-	   rounded, size);
+  if (use_pe_aligned_common)
+    fprintf (stream, ", " HOST_WIDE_INT_PRINT_DEC ", %d\n",
+	   size ? size : (HOST_WIDE_INT) 1,
+	   exact_log2 (align) - exact_log2 (CHAR_BIT));
+  else
+    fprintf (stream, ", " HOST_WIDE_INT_PRINT_DEC "\t" ASM_COMMENT_START
+	   " " HOST_WIDE_INT_PRINT_DEC "\n", rounded, size);
 }
 
 /* The Microsoft linker requires that every function be marked as
Index: gcc/config/i386/cygming.opt
===================================================================
--- gcc/config/i386/cygming.opt	(revision 147710)
+++ gcc/config/i386/cygming.opt	(working copy)
@@ -45,3 +45,7 @@ Set Windows defines
 mwindows
 Target
 Create GUI application
+
+mpe-aligned-commons
+Target Var(use_pe_aligned_common) Init(HAVE_GAS_ALIGNED_COMM)
+Use the GNU extension to the PE format for aligned common data
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 147710)
+++ gcc/doc/invoke.texi	(working copy)
@@ -15675,6 +15675,15 @@ This option is available for Cygwin and MinGW targ
 specifies that a GUI application is to be generated by
 instructing the linker to set the PE header subsystem type
 appropriately.
+
+@item -mpe-aligned-commons
+@opindex mpe-aligned-commons
+This option is available for Cygwin and MinGW targets.  It
+specifies that the GNU extension to the PE file format that 
+permits the correct alignment of COMMON variables should be
+used when generating code.  It will be enabled by default if
+GCC detects that the target assembler found during configuration
+supports the feature.
 @end table
 
 See also under @ref{i386 and x86-64 Options} for standard options.
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 147710)
+++ gcc/doc/tm.texi	(working copy)
@@ -7384,7 +7384,14 @@ outputting a single uninitialized variable.
 A C statement (sans semicolon) to output to the stdio stream
 @var{stream} the assembler definition of a common-label named
 @var{name} whose size is @var{size} bytes.  The variable @var{rounded}
-is the size rounded up to whatever alignment the caller wants.
+is the size rounded up to whatever alignment the caller wants.  It is
+possible that @var{size} may be zero, for instance if a struct with no
+other member than a zero-length array is defined.  In this case, the
+backend must output a symbol definition that allocates at least one
+byte, both so that the address of the resulting object does not compare
+equal to any other, and because some object formats cannot even express
+the concept of a zero-sized common symbol, as that is how they represent
+an ordinary undefined external.
 
 Use the expression @code{assemble_name (@var{stream}, @var{name})} to
 output the name itself; before and after that, output the additional
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 147710)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -611,6 +611,18 @@ proc check_effective_target_pthread {} {
     } "-pthread"]
 }
 
+# Return 1 if compilation with -mpe-aligned-commons is error-free
+# for trivial code, 0 otherwise.
+
+proc check_effective_target_pe_aligned_commons {} {
+    if { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
+	return [check_no_compiler_messages pe_aligned_commons object {
+	    int foo;
+	} "-mpe-aligned-commons"]
+    }
+    return 0
+}
+
 # Return 1 if the target supports -fstack-protector
 proc check_effective_target_fstack_protector {} {
     return [check_runtime fstack_protector {
Index: gcc/testsuite/gcc.target/i386/pr37216.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr37216.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr37216.c	(revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -msse2" } */
+/* { dg-options "-O3 -msse2 -mpe-aligned-commons" { target pe_aligned_commons } } */
+
+#include "sse2-check.h"
+
+int iarr[64];
+int iint = 0;
+
+void
+sse2_test (void)
+{
+  int i;
+
+  for (i = 0; i < 64; i++)
+    iarr[i] = -2;
+}
Index: gcc/testsuite/gcc.dg/compat/struct-layout-1_generate.c
===================================================================
--- gcc/testsuite/gcc.dg/compat/struct-layout-1_generate.c	(revision 147710)
+++ gcc/testsuite/gcc.dg/compat/struct-layout-1_generate.c	(working copy)
@@ -46,7 +46,7 @@ const char *dg_options[] = {
 "/* { dg-options \"%s-I%s\" } */\n",
 "/* { dg-options \"%s-I%s -Wno-abi\" } */\n",
 "/* { dg-options \"%s-I%s -mno-mmx -Wno-abi\" { target i?86-*-* x86_64-*-* } } */\n",
-"/* { dg-options \"%s-I%s -fno-common\" { target hppa*-*-hpux* powerpc*-*-darwin* *-*-mingw32* *-*-cygwin* } } */\n",
+"/* { dg-options \"%s-I%s -fno-common\" { target hppa*-*-hpux* powerpc*-*-darwin* } } */\n",
 "/* { dg-options \"%s-I%s -mno-mmx -fno-common -Wno-abi\" { target i?86-*-darwin* x86_64-*-darwin* } } */\n",
 "/* { dg-options \"%s-I%s -mno-base-addresses\" { target mmix-*-* } } */\n",
 "/* { dg-options \"%s-I%s -mlongcalls -mtext-section-literals\" { target xtensa*-*-* } } */\n"

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]