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]

RFA: PATCH to tree optimizers for target/39179 (PE binds_local_p issue)


The PE binds_local_p returns true for many more variables than the default (ELF) definition, so it shows up problems with aggressive use of that hook. Two places in the tree optimizers assume that if TREE_STATIC is set and binds_local_p is true, we know the value of a const variable. This is not true in the C++ front end, where variables routinely have both TREE_STATIC and DECL_EXTERNAL set. Apparently this is incorrect, and C++ linkage code is overdue for an overhaul--the tricks we use to defer linkage choices until EOF can be simplified a lot now that we're always in unit-at-a-time mode--but fixing that isn't feasible for the 4.4 timeframe. Just checking !DECL_EXTERNAL fixes the bug simply.

In discussion of the PR, there was some confusion about what binds_local_p actually means, so I've clarified a couple of uses of "module" in the headers.

I tested that this fixes the bug with a i386-pc-mingw32 cross-compiler, and regression tested on x86_64-pc-linux-gnu.

OK for trunk?
2009-02-18  Jason Merrill  <jason@redhat.com>

	PR target/39179
	* tree-ssa-ccp.c (get_symbol_constant_value): Don't assume zero
	value if DECL_EXTERNAL.
	* tree-sra.c (sra_walk_gimple_assign): Likewise.
	* target.h (gcc_target::binds_local_p): Clarify "module".
	* tree.h (TREE_PUBLIC): Clarify "module".

Index: tree-sra.c
===================================================================
*** tree-sra.c	(revision 144242)
--- tree-sra.c	(working copy)
*************** sra_walk_gimple_assign (gimple stmt, gim
*** 1008,1013 ****
--- 1008,1014 ----
  	 we'd been passed the constructor directly.  Invoke INIT.  */
        else if (TREE_CODE (rhs) == VAR_DECL
  	       && TREE_STATIC (rhs)
+ 	       && !DECL_EXTERNAL (rhs)
  	       && TREE_READONLY (rhs)
  	       && targetm.binds_local_p (rhs))
  	fns->init (lhs_elt, DECL_INITIAL (rhs), gsi);
Index: tree-ssa-ccp.c
===================================================================
*** tree-ssa-ccp.c	(revision 144242)
--- tree-ssa-ccp.c	(working copy)
*************** get_symbol_constant_value (tree sym)
*** 287,296 ****
  	 have zero as the initializer if they may not be
  	 overridden at link or run time.  */
        if (!val
  	  && targetm.binds_local_p (sym)
            && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
  	       || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
!         return fold_convert (TREE_TYPE (sym), integer_zero_node);
      }
  
    return NULL_TREE;
--- 287,297 ----
  	 have zero as the initializer if they may not be
  	 overridden at link or run time.  */
        if (!val
+ 	  && !DECL_EXTERNAL (sym)
  	  && targetm.binds_local_p (sym)
            && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
  	       || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
! 	return fold_convert (TREE_TYPE (sym), integer_zero_node);
      }
  
    return NULL_TREE;
Index: testsuite/g++.dg/opt/const6.C
===================================================================
*** testsuite/g++.dg/opt/const6.C	(revision 0)
--- testsuite/g++.dg/opt/const6.C	(revision 0)
***************
*** 0 ****
--- 1,14 ----
+ // PR target/39179
+ // Make sure that we don't optimize away the load from K::k.
+ // { dg-options "-O2" }
+ // { dg-final { scan-assembler _ZN1K1kE } }
+ 
+ struct K {
+     static const unsigned k;
+ };
+ extern "C" void abort (void);
+ int main() {
+     if ( K::k != 1 )
+       abort ();
+     return 1;
+ }
Index: target.h
===================================================================
*** target.h	(revision 144242)
--- target.h	(working copy)
*************** struct gcc_target
*** 626,632 ****
    bool (* in_small_data_p) (const_tree);
  
    /* True if EXP names an object for which name resolution must resolve
!      to the current module.  */
    bool (* binds_local_p) (const_tree);
  
    /* Modify and return the identifier of a DECL's external name,
--- 626,632 ----
    bool (* in_small_data_p) (const_tree);
  
    /* True if EXP names an object for which name resolution must resolve
!      to the current executable or shared library.  */
    bool (* binds_local_p) (const_tree);
  
    /* Modify and return the identifier of a DECL's external name,
Index: tree.h
===================================================================
*** tree.h	(revision 144242)
--- tree.h	(working copy)
*************** extern void omp_clause_range_check_faile
*** 1195,1203 ****
   (CONSTANT_CLASS_P (EXPR) && TREE_OVERFLOW (EXPR))
  
  /* In a VAR_DECL, FUNCTION_DECL, NAMESPACE_DECL or TYPE_DECL,
!    nonzero means name is to be accessible from outside this module.
     In an IDENTIFIER_NODE, nonzero means an external declaration
!    accessible from outside this module was previously seen
     for this name in an inner scope.  */
  #define TREE_PUBLIC(NODE) ((NODE)->base.public_flag)
  
--- 1195,1203 ----
   (CONSTANT_CLASS_P (EXPR) && TREE_OVERFLOW (EXPR))
  
  /* In a VAR_DECL, FUNCTION_DECL, NAMESPACE_DECL or TYPE_DECL,
!    nonzero means name is to be accessible from outside this translation unit.
     In an IDENTIFIER_NODE, nonzero means an external declaration
!    accessible from outside this translation unit was previously seen
     for this name in an inner scope.  */
  #define TREE_PUBLIC(NODE) ((NODE)->base.public_flag)
  

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