This is the mail archive of the gcc@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 Re: [ast-optimizer-branch] gcc.dg/struct-alias-1.c failure


>>>>> "Diego" == Diego Novillo <dnovillo@redhat.com> writes:

> We are now failing struct-alias-1.c because we don't do constant
> propagation on trees.  The simplifier is turning:

>   s.x = 0;
>   if (s.x != 0)
>     link_error ();

> into

>   s.x = 0;
>   T.2 = s.x;
>   if (T.2 != 0)
>     link_error ();

> which confuses the RTL constant propagator.

The test isn't that simple; there's another store in the middle.  So,

  s.x = 0;
  s.a[i] = 1;
  if (s.x != 0)
    link_error ();

becomes

  s.x = 0;
  T.1 = &s.a;
  (*T.1)[i] = 1;
  T.2 = s.x;
  if (T.2 != 0)
    link_error ();

Also, it's not cprop that's getting confused, it's alias analysis.  In the
unsimplified code, the backend can see that the second store is to a
different member of s, so it won't conflict with the store we already saw;
in the simplified code, the struct member reference has been factored out,
so the alias analysis code doesn't see it.

This is exactly what rth was talking about; he argues that we shouldn't
separate the array and member references.  Until alias analysis improves, I
think I agree.  I've attached a patch.  In this patch I've commented out
the code for handling arrays and structs separately, as we might want to
switch back once alias analysis is more clever.

In the array simplification function, you made an effort to simplify the
bounds in left-to-right order.  Why?  It would seem more efficient to
simplify them in right-to-left order, as that way the first one used in
calculation will be the one most recently calculated.  It's also simpler to
code that way.

OK?

Jason

2002-06-12  Jason Merrill  <jason@redhat.com>

	* tree-simple.c (is_simple_compound_lval): New fn.
	(is_simple_varname): Call it instead of is_simple_arrayref and
	is_simple_compref.
	(is_simple_arrayref, is_simple_compref): Comment out.
	* tree-simple.h: Declare it.
	* c-simplify.c (simplify_expr_either): Comment out.
	(simplify_compound_lval): New fn.
	(simplify_array_ref, simplify_component_ref): Just call it.

*** tree-simple.c.~1~	Mon Jun 10 13:05:15 2002
--- tree-simple.c	Wed Jun 12 01:22:15 2002
*************** Boston, MA 02111-1307, USA.  */
*** 199,206 ****
  
       ----------------------------------------------------------------------  */
  
- static bool is_union_based_ref		PARAMS ((tree));
- 
  /* Validation of SIMPLE statements.  */
  
  /** {{{ is_simple_stmt ()
--- 199,204 ----
*************** is_simple_varname (t)
*** 699,709 ****
    if (t == NULL_TREE)
      return 1;
  
!   return (is_simple_id (t) || is_simple_arrayref (t) || is_simple_compref (t));
  }
  
  /* }}} */
  
  /** {{{ is_simple_const ()
  
      Return nonzero if T is a constant.  */
--- 697,751 ----
    if (t == NULL_TREE)
      return 1;
  
!   return (is_simple_id (t)
! #if 0
! 	  || is_simple_arrayref (t) || is_simple_compref (t)
! #else
! 	  || is_simple_compound_lval (t)
! #endif
! 	  );
  }
  
  /* }}} */
  
+ /* Returns nonzero if T is an array or member reference of the form:
+ 
+       compound_lval
+       	      : min_lval '[' val ']'
+ 	      | min_lval '.' ID
+ 	      | compound_lval '[' val ']'
+ 	      | compound_lval '.' ID
+ 
+    This is not part of the original SIMPLE definition, which separates
+    array and member references, but it seems reasonable to handle them
+    together.  Also, this way we don't run into problems with union
+    aliasing; gcc requires that for accesses through a union to alias, the
+    union reference must be explicit, which was not always the case when we
+    were splitting up array and member refs.  */
+ 
+ int
+ is_simple_compound_lval (t)
+      tree t;
+ {
+   /* Allow arrays of complex types.  */
+   if (TREE_CODE (t) == REALPART_EXPR
+       || TREE_CODE (t) == IMAGPART_EXPR)
+     t = TREE_OPERAND (t, 0);
+ 
+   if (TREE_CODE (t) != ARRAY_REF && TREE_CODE (t) != COMPONENT_REF)
+     return 0;
+ 
+   for (; TREE_CODE (t) == COMPONENT_REF || TREE_CODE (t) == ARRAY_REF;
+        t = TREE_OPERAND (t, 0))
+     {
+       if (TREE_CODE (t) == ARRAY_REF
+ 	  && !is_simple_val (TREE_OPERAND (t, 1)))
+ 	return 0;
+     }
+ 
+   return is_simple_min_lval (t);
+ }
+ 
  /** {{{ is_simple_const ()
  
      Return nonzero if T is a constant.  */
*************** is_simple_min_lval (t)
*** 803,838 ****
  
    return (is_simple_id (t)
  	  || (TREE_CODE (t) == INDIRECT_REF
! 	      && is_simple_id (TREE_OPERAND (t, 0)))
! 	  || is_union_based_ref (t));
! }
! 
! /* }}} */
! 
! /** {{{ is_union_based_ref ()
! 
!     Returns true iff T is a compound lvalue expression involving a union.
!     We currently don't simplify such expressions because it confuses alias
!     analysis.  FIXME alias analysis should be smarter, and this should go
!     away.  gcc.c-torture/execute/990413-2.c breaks without this.  */
! 
! static bool
! is_union_based_ref (t)
!      tree t;
! {
!   for (; TREE_CODE (t) == COMPONENT_REF || TREE_CODE (t) == ARRAY_REF;
!        t = TREE_OPERAND (t, 0))
!     {
!       if (TREE_CODE (t) == COMPONENT_REF
! 	  && TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 0))) == UNION_TYPE)
! 	return 1;
!     }
! 
!   return 0;
  }
  
  /* }}} */
  
  /** {{{ is_simple_arrayref ()
  
      Return nonzero if T is an array reference of the form:
--- 845,856 ----
  
    return (is_simple_id (t)
  	  || (TREE_CODE (t) == INDIRECT_REF
! 	      && is_simple_id (TREE_OPERAND (t, 0))));
  }
  
  /* }}} */
  
+ #if 0
  /** {{{ is_simple_arrayref ()
  
      Return nonzero if T is an array reference of the form:
*************** is_simple_compref (t)
*** 900,905 ****
--- 918,924 ----
  }
  
  /* }}} */
+ #endif
  
  /** {{{ is_simple_cast ()
  
*** tree-simple.h.~1~	Mon Jun 10 08:25:47 2002
--- tree-simple.h	Tue Jun 11 20:44:00 2002
*************** int is_simple_id                       P
*** 53,58 ****
--- 53,59 ----
  int is_simple_varname                  PARAMS ((tree));
  int is_simple_val                      PARAMS ((tree));
  int is_simple_min_lval                 PARAMS ((tree));
+ int is_simple_compound_lval            PARAMS ((tree));
  int is_simple_arrayref                 PARAMS ((tree));
  int is_simple_compref                  PARAMS ((tree));
  int is_simple_cast                     PARAMS ((tree));
*** c-simplify.c.~1~	Tue Jun 11 13:18:23 2002
--- c-simplify.c	Tue Jun 11 20:44:00 2002
*************** static void simplify_expr_common     PAR
*** 66,74 ****
                                                int (*) PARAMS ((tree)), tree, int));
  static void simplify_expr            PARAMS ((tree *, tree *, tree *,
                                                int (*) PARAMS ((tree)), tree));
- static void simplify_expr_either     PARAMS ((tree *, tree *, tree *,
-                                               int (*) PARAMS ((tree)), tree));
  static void simplify_array_ref       PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_self_mod_expr   PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_component_ref   PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_call_expr       PARAMS ((tree *, tree *, tree *, tree));
--- 66,73 ----
                                                int (*) PARAMS ((tree)), tree, int));
  static void simplify_expr            PARAMS ((tree *, tree *, tree *,
                                                int (*) PARAMS ((tree)), tree));
  static void simplify_array_ref       PARAMS ((tree *, tree *, tree *, tree));
+ static void simplify_compound_lval   PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_self_mod_expr   PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_component_ref   PARAMS ((tree *, tree *, tree *, tree));
  static void simplify_call_expr       PARAMS ((tree *, tree *, tree *, tree));
*************** simplify_lvalue_expr (expr_p, pre_p, pos
*** 1140,1145 ****
--- 1139,1147 ----
    simplify_expr_common (expr_p, pre_p, post_p, simple_test_f, stmt, 2);
  }
  
+ #if 0
+ static void simplify_expr_either     PARAMS ((tree *, tree *, tree *,
+                                               int (*) PARAMS ((tree)), tree));
  static void
  simplify_expr_either (expr_p, pre_p, post_p, simple_test_f, stmt)
       tree *expr_p;
*************** simplify_expr_either (expr_p, pre_p, pos
*** 1150,1155 ****
--- 1152,1158 ----
  {
    simplify_expr_common (expr_p, pre_p, post_p, simple_test_f, stmt, 1|2);
  }
+ #endif
  
  /* }}} */
  
*************** simplify_array_ref (expr_p, pre_p, post_
*** 1202,1207 ****
--- 1205,1214 ----
       tree *post_p;
       tree stmt;
  {
+ #if 1
+   /* Handle array and member refs together for now.  */
+   simplify_compound_lval (expr_p, pre_p, post_p, stmt);
+ #else
    tree *p;
    varray_type dim_stack;
  
*************** simplify_array_ref (expr_p, pre_p, post_
*** 1231,1240 ****
--- 1238,1289 ----
      }
  
    VARRAY_FREE (dim_stack);
+ #endif
  }
  
  /* }}} */
  
+ /* Simplify the COMPONENT_REF or ARRAY_REF node pointed by EXPR_P.
+ 
+    PRE_P points to the list where side effects that must happen before
+      *EXPR_P should be stored.
+ 
+    POST_P points to the list where side effects that must happen after
+      *EXPR_P should be stored.
+ 
+    STMT is the statement tree that contains EXPR.  It's used in cases
+      where simplifying an expression requires creating new statement
+      trees.  */
+ 
+ static void
+ simplify_compound_lval (expr_p, pre_p, post_p, stmt)
+      tree *expr_p;
+      tree *pre_p;
+      tree *post_p;
+      tree stmt;
+ {
+   tree *p;
+   enum tree_code code;
+ 
+   if (TREE_CODE (*expr_p) != ARRAY_REF && TREE_CODE (*expr_p) != COMPONENT_REF)
+     abort ();
+ 
+   for (p = expr_p;
+        TREE_CODE (*p) == ARRAY_REF || TREE_CODE (*p) == COMPONENT_REF;
+        p = &TREE_OPERAND (*p, 0))
+     {
+       code = TREE_CODE (*p);
+       if (code == ARRAY_REF)
+ 	simplify_expr (&TREE_OPERAND (*p, 1), pre_p, post_p,
+ 		       is_simple_val, stmt);
+     }
+ 
+   /* Now we're down to the first bit that isn't an ARRAY_REF or
+      COMPONENT_REF.  */
+   simplify_expr_common (p, pre_p, post_p, is_simple_min_lval, stmt,
+ 			code == COMPONENT_REF ? 3 : 2);
+ }
+ 
  /** {{{ simplify_self_mod_expr ()
  
      Simplify the self modifying expression pointed by EXPR_P (++, --, +=, -=).
*************** simplify_component_ref (expr_p, pre_p, p
*** 1322,1327 ****
--- 1371,1380 ----
       tree *post_p;
       tree stmt;
  {
+ #if 1
+   /* Handle array and member refs together for now.  */
+   simplify_compound_lval (expr_p, pre_p, post_p, stmt);
+ #else
    tree *p;
  
    if (TREE_CODE (*expr_p) != COMPONENT_REF)
*************** simplify_component_ref (expr_p, pre_p, p
*** 1334,1339 ****
--- 1387,1393 ----
  
    /* Now we're down to the first bit that isn't a COMPONENT_REF.  */
    simplify_expr_either (p, pre_p, post_p, is_simple_min_lval, stmt);
+ #endif
  }
  
  /* }}} */

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