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]

Re: [PATCH][C++] Fix PR37742


On Fri, 31 Oct 2008, Jason Merrill wrote:

> Richard Guenther wrote:
> > Ping!  The C++ parts need approval.
> 
> OK.

Re-testing the patch uncovered one optimization regression.  The
following patch fixes that by ensuring that we still propagate
addresses of a decl even if we lose restrict qualification in this case.
This is a fair trade-off and should make sure we do not run into
wrong-code problems.

Bootstrapped and tested on x86_64-unknown-linux-gnu, I'll apply this
variant tomorrow.

Richard.

2008-10-14  Richard Guenther  <rguenther@suse.de>

	PR middle-end/37742
	* tree-ssa.c (useless_type_conversion_p_1): Check different restrict
	qualified pointer conversion before stripping qualifiers.
	* gimplify.c (create_tmp_from_val): Use correctly qualified type.
	* tree-flow.h (may_propagate_address_into_dereference): Declare.
	* tree-ssa-ccp.c (may_propagate_address_into_dereference): New function.
	(ccp_fold): Use it.
	* tree-ssa-forwprop.c (rhs_to_tree): Remove useless conversions,
	properly canonicalize binary ops.
	(forward_propagate_addr_expr_1): Use
	may_propagate_address_into_dereference.

	cp/
	* decl.c (start_preparsed_function): Use the correct type for
	building the RESULT_DECL.

	* gcc.c-torture/compile/pr37742.c: New testcase.
	* g++.dg/pr37742.C: Likewise.
	* gcc.dg/tree-ssa/forwprop-7.c: Check for two volatile loads.

Index: gcc/tree-ssa.c
===================================================================
*** gcc/tree-ssa.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/tree-ssa.c	2008-11-02 16:26:12.000000000 +0100
*************** delete_tree_ssa (void)
*** 1071,1077 ****
  static bool
  useless_type_conversion_p_1 (tree outer_type, tree inner_type)
  {
!   /* Qualifiers on value types do not matter.  */
    inner_type = TYPE_MAIN_VARIANT (inner_type);
    outer_type = TYPE_MAIN_VARIANT (outer_type);
  
--- 1071,1088 ----
  static bool
  useless_type_conversion_p_1 (tree outer_type, tree inner_type)
  {
!   /* Do the following before stripping toplevel qualifiers.  */
!   if (POINTER_TYPE_P (inner_type)
!       && POINTER_TYPE_P (outer_type))
!     {
!       /* Do not lose casts to restrict qualified pointers.  */
!       if ((TYPE_RESTRICT (outer_type)
! 	   != TYPE_RESTRICT (inner_type))
! 	  && TYPE_RESTRICT (outer_type))
! 	return false;
!     }
! 
!   /* From now on qualifiers on value types do not matter.  */
    inner_type = TYPE_MAIN_VARIANT (inner_type);
    outer_type = TYPE_MAIN_VARIANT (outer_type);
  
*************** useless_type_conversion_p_1 (tree outer_
*** 1147,1158 ****
        /* We do not care for const qualification of the pointed-to types
  	 as const qualification has no semantic value to the middle-end.  */
  
-       /* Do not lose casts to restrict qualified pointers.  */
-       if ((TYPE_RESTRICT (outer_type)
- 	   != TYPE_RESTRICT (inner_type))
- 	  && TYPE_RESTRICT (outer_type))
- 	return false;
- 
        /* Otherwise pointers/references are equivalent if their pointed
  	 to types are effectively the same.  We can strip qualifiers
  	 on pointed-to types for further comparison, which is done in
--- 1158,1163 ----
Index: gcc/testsuite/gcc.c-torture/compile/pr37742.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.c-torture/compile/pr37742.c	2008-11-02 16:26:12.000000000 +0100
***************
*** 0 ****
--- 1,21 ----
+ void foo(int* __restrict__ p, int* q, int* p1, int *q1)
+ {
+   int i;
+ 
+   p = p1;
+   q = q1;
+ 
+   for (i = 0; i < 4; ++i)
+     *++q = *++p + 1;
+ }
+ 
+ void bar(int* p, int* __restrict__ q, int* p1, int *q1)
+ {
+   int i;
+ 
+   p = p1;
+   q = q1;
+ 
+   for (i = 0; i < 4; ++i)
+     *++q = *++p + 1;
+ }
Index: gcc/gimplify.c
===================================================================
*** gcc/gimplify.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/gimplify.c	2008-11-02 16:26:12.000000000 +0100
*************** create_tmp_var (tree type, const char *p
*** 565,571 ****
  static inline tree
  create_tmp_from_val (tree val)
  {
!   return create_tmp_var (TYPE_MAIN_VARIANT (TREE_TYPE (val)), get_name (val));
  }
  
  /* Create a temporary to hold the value of VAL.  If IS_FORMAL, try to reuse
--- 565,571 ----
  static inline tree
  create_tmp_from_val (tree val)
  {
!   return create_tmp_var (TREE_TYPE (val), get_name (val));
  }
  
  /* Create a temporary to hold the value of VAL.  If IS_FORMAL, try to reuse
Index: gcc/cp/decl.c
===================================================================
*** gcc/cp/decl.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/cp/decl.c	2008-11-02 16:26:12.000000000 +0100
*************** start_preparsed_function (tree decl1, tr
*** 11492,11498 ****
      {
        tree resdecl;
  
!       resdecl = build_decl (RESULT_DECL, 0, TYPE_MAIN_VARIANT (restype));
        DECL_ARTIFICIAL (resdecl) = 1;
        DECL_IGNORED_P (resdecl) = 1;
        DECL_RESULT (decl1) = resdecl;
--- 11492,11498 ----
      {
        tree resdecl;
  
!       resdecl = build_decl (RESULT_DECL, 0, restype);
        DECL_ARTIFICIAL (resdecl) = 1;
        DECL_IGNORED_P (resdecl) = 1;
        DECL_RESULT (decl1) = resdecl;
Index: gcc/testsuite/g++.dg/pr37742.C
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/g++.dg/pr37742.C	2008-11-02 16:26:12.000000000 +0100
***************
*** 0 ****
--- 1,10 ----
+ /* { dg-do compile } */
+ 
+ typedef long unsigned int size_t;
+ void*   __valarray_get_memory(size_t __n);
+ int*__restrict__
+ __valarray_get_storage(size_t __n)
+ {
+   return static_cast<int* __restrict__>(__valarray_get_memory(__n));
+ }
+ 
Index: gcc/testsuite/gcc.dg/tree-ssa/forwprop-7.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/forwprop-7.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/testsuite/gcc.dg/tree-ssa/forwprop-7.c	2008-11-02 16:26:12.000000000 +0100
*************** int foo(void)
*** 8,14 ****
    return *p + *p;
  }
  
! /* We should not convert the cast to a VCE in forwprop1 as we have a volatile reference.  */
  /* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 0 "forwprop1"} } */
! /* { dg-final { scan-tree-dump-times "volatile int" 2 "forwprop1"} } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
--- 8,16 ----
    return *p + *p;
  }
  
! /* We should not convert the cast to a VCE in forwprop1 as we have a
!    volatile reference.  */
! 
  /* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 0 "forwprop1"} } */
! /* { dg-final { scan-tree-dump-times "={v}" 2 "forwprop1"} } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
Index: gcc/tree-flow.h
===================================================================
*** gcc/tree-flow.h.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/tree-flow.h	2008-11-02 16:26:12.000000000 +0100
*************** bool fold_stmt (gimple_stmt_iterator *);
*** 905,910 ****
--- 905,912 ----
  bool fold_stmt_inplace (gimple);
  tree get_symbol_constant_value (tree);
  tree fold_const_aggregate_ref (tree);
+ bool may_propagate_address_into_dereference (tree, tree);
+ 
  
  /* In tree-vrp.c  */
  tree vrp_evaluate_conditional (enum tree_code, tree, tree, gimple);
Index: gcc/tree-ssa-ccp.c
===================================================================
*** gcc/tree-ssa-ccp.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/tree-ssa-ccp.c	2008-11-02 16:26:12.000000000 +0100
*************** ccp_visit_phi_node (gimple phi)
*** 851,856 ****
--- 851,881 ----
      return SSA_PROP_NOT_INTERESTING;
  }
  
+ /* Return true if we may propagate the address expression ADDR into the 
+    dereference DEREF and cancel them.  */
+ 
+ bool
+ may_propagate_address_into_dereference (tree addr, tree deref)
+ {
+   gcc_assert (INDIRECT_REF_P (deref)
+ 	      && TREE_CODE (addr) == ADDR_EXPR);
+ 
+   /* If the address is invariant then we do not need to preserve restrict
+      qualifications.  But we do need to preserve volatile qualifiers until
+      we can annotate the folded dereference itself properly.  */
+   if (is_gimple_min_invariant (addr)
+       && (!TREE_THIS_VOLATILE (deref)
+ 	  || TYPE_VOLATILE (TREE_TYPE (addr))))
+     return useless_type_conversion_p (TREE_TYPE (deref),
+ 				      TREE_TYPE (TREE_OPERAND (addr, 0)));
+ 
+   /* Else both the address substitution and the folding must result in
+      a valid useless type conversion sequence.  */
+   return (useless_type_conversion_p (TREE_TYPE (TREE_OPERAND (deref, 0)),
+ 				     TREE_TYPE (addr))
+ 	  && useless_type_conversion_p (TREE_TYPE (deref),
+ 					TREE_TYPE (TREE_OPERAND (addr, 0))));
+ }
  
  /* CCP specific front-end to the non-destructive constant folding
     routines.
*************** ccp_fold (gimple stmt)
*** 897,908 ****
  		      prop_value_t *val = get_value (TREE_OPERAND (*base, 0));
  		      if (val->lattice_val == CONSTANT
  			  && TREE_CODE (val->value) == ADDR_EXPR
! 			  && useless_type_conversion_p
! 			  (TREE_TYPE (TREE_OPERAND (*base, 0)),
! 			   TREE_TYPE (val->value))
! 			  && useless_type_conversion_p
! 			  (TREE_TYPE (*base),
! 			   TREE_TYPE (TREE_OPERAND (val->value, 0))))
  			{
  			  /* We need to return a new tree, not modify the IL
  			     or share parts of it.  So play some tricks to
--- 922,929 ----
  		      prop_value_t *val = get_value (TREE_OPERAND (*base, 0));
  		      if (val->lattice_val == CONSTANT
  			  && TREE_CODE (val->value) == ADDR_EXPR
! 			  && may_propagate_address_into_dereference
! 			       (val->value, *base))
  			{
  			  /* We need to return a new tree, not modify the IL
  			     or share parts of it.  So play some tricks to
Index: gcc/tree-ssa-forwprop.c
===================================================================
*** gcc/tree-ssa-forwprop.c.orig	2008-11-02 16:23:56.000000000 +0100
--- gcc/tree-ssa-forwprop.c	2008-11-02 16:55:17.000000000 +0100
*************** rhs_to_tree (tree type, gimple stmt)
*** 334,343 ****
  {
    enum tree_code code = gimple_assign_rhs_code (stmt);
    if (get_gimple_rhs_class (code) == GIMPLE_BINARY_RHS)
!     return fold_convert (type, build2 (code, type, gimple_assign_rhs1 (stmt),
!                          gimple_assign_rhs2 (stmt)));
    else if (get_gimple_rhs_class (code) == GIMPLE_UNARY_RHS)
!     return fold_convert (type, build1 (code, type, gimple_assign_rhs1 (stmt)));
    else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
      return gimple_assign_rhs1 (stmt);
    else
--- 334,343 ----
  {
    enum tree_code code = gimple_assign_rhs_code (stmt);
    if (get_gimple_rhs_class (code) == GIMPLE_BINARY_RHS)
!     return fold_build2 (code, type, gimple_assign_rhs1 (stmt),
! 			gimple_assign_rhs2 (stmt));
    else if (get_gimple_rhs_class (code) == GIMPLE_UNARY_RHS)
!     return build1 (code, type, gimple_assign_rhs1 (stmt));
    else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
      return gimple_assign_rhs1 (stmt);
    else
*************** forward_propagate_addr_expr_1 (tree name
*** 719,730 ****
       propagate the ADDR_EXPR into the use of NAME and fold the result.  */
    if (TREE_CODE (lhs) == INDIRECT_REF
        && TREE_OPERAND (lhs, 0) == name
!       && useless_type_conversion_p (TREE_TYPE (TREE_OPERAND (lhs, 0)),
! 				    TREE_TYPE (def_rhs))
!       /* ???  This looks redundant, but is required for bogus types
! 	 that can sometimes occur.  */
!       && useless_type_conversion_p (TREE_TYPE (lhs),
! 				    TREE_TYPE (TREE_OPERAND (def_rhs, 0))))
      {
        *lhsp = unshare_expr (TREE_OPERAND (def_rhs, 0));
        fold_stmt_inplace (use_stmt);
--- 719,725 ----
       propagate the ADDR_EXPR into the use of NAME and fold the result.  */
    if (TREE_CODE (lhs) == INDIRECT_REF
        && TREE_OPERAND (lhs, 0) == name
!       && may_propagate_address_into_dereference (def_rhs, lhs))
      {
        *lhsp = unshare_expr (TREE_OPERAND (def_rhs, 0));
        fold_stmt_inplace (use_stmt);
*************** forward_propagate_addr_expr_1 (tree name
*** 747,758 ****
       propagate the ADDR_EXPR into the use of NAME and fold the result.  */
    if (TREE_CODE (rhs) == INDIRECT_REF
        && TREE_OPERAND (rhs, 0) == name
!       && useless_type_conversion_p (TREE_TYPE (TREE_OPERAND (rhs, 0)),
! 				    TREE_TYPE (def_rhs))
!       /* ???  This looks redundant, but is required for bogus types
! 	 that can sometimes occur.  */
!       && useless_type_conversion_p (TREE_TYPE (rhs),
! 				    TREE_TYPE (TREE_OPERAND (def_rhs, 0))))
      {
        *rhsp = unshare_expr (TREE_OPERAND (def_rhs, 0));
        fold_stmt_inplace (use_stmt);
--- 742,748 ----
       propagate the ADDR_EXPR into the use of NAME and fold the result.  */
    if (TREE_CODE (rhs) == INDIRECT_REF
        && TREE_OPERAND (rhs, 0) == name
!       && may_propagate_address_into_dereference (def_rhs, rhs))
      {
        *rhsp = unshare_expr (TREE_OPERAND (def_rhs, 0));
        fold_stmt_inplace (use_stmt);


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