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] Fix PR33604 and another place we'd lose volatile accesses


This removes one missed forward propagation opportunity regarding CV
changes of pointed-to memory like

  int i;
  const int *p = (const int *)&i;
  return *i;

where all required semantics are verified and annotated at the memory
access site.  This happens quite often in C++ code with constant class
members appearantly.

Fixed by allowing to propagate addresses into dereferences if the 
pointed-to memory is trivially convertible.  Now, this exposes a bug
in fold_stmt_r where we lost volatile tree annotations for the resulting
tree.

Fixed thusly.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to mainline.

2007-11-08  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/33604
	* tree-ssa-forwprop.c (tree_ssa_forward_propagate_single_use_vars):
	Disregard changes in CV qualifiers of pointed to types for
	forward propagating ADDR_EXPRs.
	* tree-ssa-ccp.c (fold_stmt_r): Preserve volatileness of the original
	expression.

	* g++.dg/tree-ssa/pr33604.C: New testcase.
	* gcc.dg/pr32721.c: Adjust pattern.

Index: testsuite/g++.dg/tree-ssa/pr33604.C
===================================================================
*** testsuite/g++.dg/tree-ssa/pr33604.C	(revision 0)
--- testsuite/g++.dg/tree-ssa/pr33604.C	(revision 0)
***************
*** 0 ****
--- 1,46 ----
+ /* { dg-do run } */
+ /* { dg-options "-O -fdump-tree-forwprop1" } */
+ 
+ struct Value
+ {
+   double value;
+   Value(double value_) : value (value_) {}
+   operator double() const { return value; }
+   Value& operator=(double other) { value = other; }
+ };
+ 
+ struct Ref
+ {
+   const Value& m;
+   Ref(const Value& m_) : m(m_) {}
+   operator double() const { return m; }
+ };
+ 
+ struct Diff
+ {
+   const Ref lhs, rhs;
+   Diff(const Value& lhs_, const Value& rhs_) : lhs(lhs_), rhs(rhs_) {}
+   operator double() const { return lhs - rhs; }
+ };
+ 
+ extern "C" void abort (void);
+ int main(int argc, char *argv[])
+ {
+   Value I(1), m(4);
+   for(int a = 0; a < 1000; a++)
+     m = Diff (I, m);
+ 
+   if (!(m / 4 == I))
+     abort ();
+   return 0;
+ }
+ 
+ /* Check that we forward propagated
+      D.2182_13 = (struct Ref *) &D.2137.lhs;
+    to
+      D.2182_13->lhs.m ={v} &I;
+    yielding
+      D.2137.lhs.m ={v} &I;  */
+ 
+ /* { dg-final { scan-tree-dump-times "D\\\.....\\\..hs\\\.m =" 2 "forwprop1" } } */
+ /* { dg-final { cleanup-tree-dump "forwprop1" } } */
Index: tree-ssa-forwprop.c
===================================================================
*** tree-ssa-forwprop.c	(revision 129897)
--- tree-ssa-forwprop.c	(working copy)
*************** tree_ssa_forward_propagate_single_use_va
*** 952,958 ****
  		  continue;
  		}
  
! 	      if (TREE_CODE (rhs) == ADDR_EXPR)
  		{
  		  if (forward_propagate_addr_expr (lhs, rhs))
  		    {
--- 952,966 ----
  		  continue;
  		}
  
! 	      if (TREE_CODE (rhs) == ADDR_EXPR
! 		  /* We can also disregard changes in CV qualifiers for
! 		     the dereferenced value.  */
! 		  || ((TREE_CODE (rhs) == NOP_EXPR
! 		       || TREE_CODE (rhs) == CONVERT_EXPR)
! 		      && TREE_CODE (TREE_OPERAND (rhs, 0)) == ADDR_EXPR
! 		      && POINTER_TYPE_P (TREE_TYPE (rhs))
! 		      && useless_type_conversion_p (TREE_TYPE (TREE_TYPE (rhs)),
! 						    TREE_TYPE (TREE_TYPE (TREE_OPERAND (rhs, 0))))))
  		{
  		  if (forward_propagate_addr_expr (lhs, rhs))
  		    {
Index: tree-ssa-ccp.c
===================================================================
*** tree-ssa-ccp.c	(revision 130038)
--- tree-ssa-ccp.c	(working copy)
*************** fold_stmt_r (tree *expr_p, int *walk_sub
*** 2034,2039 ****
--- 2034,2040 ----
    bool *inside_addr_expr_p = fold_stmt_r_data->inside_addr_expr_p;
    bool *changed_p = fold_stmt_r_data->changed_p;
    tree expr = *expr_p, t;
+   bool volatile_p = TREE_THIS_VOLATILE (expr);
  
    /* ??? It'd be nice if walk_tree had a pre-order option.  */
    switch (TREE_CODE (expr))
*************** fold_stmt_r (tree *expr_p, int *walk_sub
*** 2159,2164 ****
--- 2160,2167 ----
  
    if (t)
      {
+       /* Preserve volatileness of the original expression.  */
+       TREE_THIS_VOLATILE (t) = volatile_p;
        *expr_p = t;
        *changed_p = true;
      }
Index: testsuite/gcc.dg/pr32721.c
===================================================================
*** testsuite/gcc.dg/pr32721.c	(revision 130038)
--- testsuite/gcc.dg/pr32721.c	(working copy)
*************** spinlock1 = &spinlock[1];
*** 14,18 ****
   while (*spinlock0);
  }
  
! /* { dg-final { scan-tree-dump "={v} \\*spinlock0" "optimized" } } */
  /* { dg-final { cleanup-tree-dump "optimized" } } */
--- 14,19 ----
   while (*spinlock0);
  }
  
! /* { dg-final { scan-tree-dump "={v} .*spinlock" "optimized" } } */
! /* { dg-final { scan-tree-dump "spinlock.* ={v}" "optimized" } } */
  /* { dg-final { cleanup-tree-dump "optimized" } } */


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