Patchwork Fix PR52571

login
register
mail settings
Submitter Richard Guenther
Date March 14, 2012, 12:59 p.m.
Message ID <alpine.LNX.2.00.1203141359120.27160@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/146606/
State New
Headers show

Comments

Richard Guenther - March 14, 2012, 12:59 p.m.
On Wed, 14 Mar 2012, Richard Guenther wrote:

> 
> This fixes PR52571, we should not align DECL_COMMON variables as
> they might be pre-empted by a definition with lower alignment.
> 
> At LTO/WPA level we might recover from missed optimizations by
> promoting DECL_COMMON variables to non-common.  Not sure if we
> do that already.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Checked in with some testcase adjustments.

Richard.

2012-03-14  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/52571
	* tree-vect-data-refs.c (vect_compute_data_ref_alignment): Move
	flag_section_anchors check ...
	(vect_can_force_dr_alignment_p): ... here.  Do not re-align
	DECL_COMMON variables.

	* gcc.dg/vect/vect-2.c: Initialize arrays.
	* gcc.dg/vect/no-section-anchors-vect-34.c: Likewise.
	* gcc.target/i386/recip-vec-divf.c: Use -fno-common.
	* gcc.target/i386/recip-vec-sqrtf.c: Likewise.

Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c	(revision 185379)
--- gcc/tree-vect-data-refs.c	(working copy)
*************** vect_compute_data_ref_alignment (struct
*** 872,881 ****
  
    if (!base_aligned)
      {
!       /* Do not change the alignment of global variables if
! 	 flag_section_anchors is enabled.  */
!       if (!vect_can_force_dr_alignment_p (base, TYPE_ALIGN (vectype))
! 	  || (TREE_STATIC (base) && flag_section_anchors))
  	{
  	  if (vect_print_dump_info (REPORT_DETAILS))
  	    {
--- 872,878 ----
  
    if (!base_aligned)
      {
!       if (!vect_can_force_dr_alignment_p (base, TYPE_ALIGN (vectype)))
  	{
  	  if (vect_print_dump_info (REPORT_DETAILS))
  	    {
*************** vect_can_force_dr_alignment_p (const_tre
*** 4546,4557 ****
    if (TREE_CODE (decl) != VAR_DECL)
      return false;
  
!   if (DECL_EXTERNAL (decl))
      return false;
  
    if (TREE_ASM_WRITTEN (decl))
      return false;
  
    if (TREE_STATIC (decl))
      return (alignment <= MAX_OFILE_ALIGNMENT);
    else
--- 4543,4564 ----
    if (TREE_CODE (decl) != VAR_DECL)
      return false;
  
!   /* We cannot change alignment of common or external symbols as another
!      translation unit may contain a definition with lower alignment.  
!      The rules of common symbol linking mean that the definition
!      will override the common symbol.  */
!   if (DECL_EXTERNAL (decl)
!       || DECL_COMMON (decl))
      return false;
  
    if (TREE_ASM_WRITTEN (decl))
      return false;
  
+   /* Do not change the alignment of global variables if flag_section_anchors
+      is enabled.  */
+   if (TREE_STATIC (decl) && flag_section_anchors)
+     return false;
+ 
    if (TREE_STATIC (decl))
      return (alignment <= MAX_OFILE_ALIGNMENT);
    else
Mike Stump - April 9, 2012, 11:54 p.m.
On Mar 14, 2012, at 5:59 AM, Richard Guenther wrote:
>> This fixes PR52571, we should not align DECL_COMMON variables as
>> they might be pre-empted by a definition with lower alignment.

I added this to the PR:

Ah, I had another thought.  COMDAT and LINKONCE things I don't think can be realigned for all the same reasons that one cannot align COMMON.  I've not thought about this long and hard, so, could be wrong, so, would be good to have a C++ or a vectorizer person review the idea.  The idea is, if you compile one translation unit with a vectorizor on, and another with it off, we wind up with two instantiations, each with different alignment, and the one picked at the end need not be either of them, but rather an explicit instantiation.   This seems identical to what happens to common to me.

Patch

Index: gcc/testsuite/gcc.dg/vect/vect-2.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/vect-2.c	(revision 185379)
+++ gcc/testsuite/gcc.dg/vect/vect-2.c	(working copy)
@@ -6,7 +6,7 @@ 
 #define N 16
 
 char cb[N] = {0,3,6,9,12,15,18,21,24,27,30,33,36,39,42,45};
-char ca[N];
+char ca[N] = {};
 
 __attribute__ ((noinline)) 
 int main1 ()
Index: gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-34.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-34.c	(revision 185379)
+++ gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-34.c	(working copy)
@@ -7,7 +7,7 @@ 
  
 struct {
   char ca[N];
-} s;
+} s = {};
 char cb[N] = {0,3,6,9,12,15,18,21,24,27,30,33,36,39,42,45};
 
 __attribute__ ((noinline))
Index: gcc/testsuite/gcc.target/i386/recip-vec-divf.c
===================================================================
--- gcc/testsuite/gcc.target/i386/recip-vec-divf.c	(revision 185379)
+++ gcc/testsuite/gcc.target/i386/recip-vec-divf.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -ffast-math -ftree-vectorize -msse -mfpmath=sse -mrecip" } */
+/* { dg-options "-O2 -ffast-math -ftree-vectorize -msse -mfpmath=sse -mrecip -fno-common" } */
 
 float a[4];
 float b[4];
Index: gcc/testsuite/gcc.target/i386/recip-vec-sqrtf.c
===================================================================
--- gcc/testsuite/gcc.target/i386/recip-vec-sqrtf.c	(revision 185379)
+++ gcc/testsuite/gcc.target/i386/recip-vec-sqrtf.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -ffast-math -ftree-vectorize -msse -mfpmath=sse -mrecip" } */
+/* { dg-options "-O2 -ffast-math -ftree-vectorize -msse -mfpmath=sse -mrecip -fno-common" } */
 
 float a[4];
 float b[4];