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]

[tree-ssa] PATCH to use a temp when copying from memory to memory


When we're dealing with a MODIFY_EXPR which copies a value of renamable
type from one memory location to another, it would be useful to copy it
into a temporary along the way so we can avoid an additional load if we
want to get at that value again later in the function.  The
gimplify_modify_expr change accomplishes this.

The rest of the patch is dealing with the fallout from that small change.
It turns out that we were somehow commonizing COMPONENT_REFs with different
types, so that if we had an 8-bit int bit-field we would store a char but
later load an int.  We would then copy-prop the char temp into the uses of
the int COMPONENT_REF, which blows up in the expander.

The fix for this is to make extension on load from 8/16-bit bit-fields
explicit by always giving the COMPONENT_REFs the appropriate smaller
underlying type.  For stores this is already done in build_modify_expr.
For loads we handle it in the gimplifier.

There was already code in the gimplifier to force a COMPONENT_REF used as
an rvalue to have the same type as the field; I just changed it to use the
underlying type of a bit-field when it's a different size from the declared
type.  And split it out into a separate function.

This patch fixes gcc.dg/tree-ssa/20030708-1.c.  It also removes the three
casts from 20030714-1.c (so I'm updating the testcase to not expect them).
The new testcase for the original change is 20031021-1.c.

This patch breaks 20030922-1.c because previously we were loading the value
directly as a tree_code, whereas now we load it as a uchar and convert it
to a tree_code, and we fail to see that if T.2 == (tree_code)0, then 
T.1 == (uchar)0.  This is a general problem with value propagation through
conversions, and I think it's reasonable to leave this broken for now.

Tested athlon-pc-linux-gnu, applied to tree-ssa.  The tree.c hunks also
applied to (and tested on) trunk.

2003-10-21  Jason Merrill  <jason@redhat.com>

	* gimplify.c (gimplify_modify_expr): Require a regvar on either
	the lhs or rhs if we're dealing with a renameable type.
	(canonicalize_component_ref): New fn.
	(gimplify_compound_lval): Use it.
	(gimplify_conversion): Use it.
	(gimplify_expr): Lose redundant STRIP_MAIN_TYPE_NOPS.
	Discard conversions in void context.
	* tree.c (get_unwidened): Check TREE_UNSIGNED on the field's type.
	(get_narrower): Likewise.

*** gimplify.c.~1~	2003-10-21 10:50:39.000000000 -0400
--- gimplify.c	2003-10-21 14:30:21.000000000 -0400
*************** static void gimplify_conversion (tree *)
*** 88,93 ****
--- 88,94 ----
  static int gimplify_init_constructor (tree *, tree *, int);
  static void gimplify_minimax_expr (tree *, tree *, tree *);
  static void build_stack_save_restore (tree *, tree *);
+ static void canonicalize_component_ref (tree *);
  
  static struct gimplify_ctx
  {
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 338,346 ****
    if (*expr_p == NULL_TREE)
      return true;
  
-   /* Go ahead and strip type nops before we test our predicate.  */
-   STRIP_MAIN_TYPE_NOPS (*expr_p);
- 
    /* We used to check the predicate here and return immediately if it
       succeeds.  This is wrong; the design is for gimplification to be
       idempotent, and for the predicates to only test for valid forms, not
--- 339,344 ----
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 447,455 ****
  	  if (IS_EMPTY_STMT (*expr_p))
  	    break;
  
! 	  if (VOID_TYPE_P (TREE_TYPE (*expr_p)))
  	    {
! 	      /* Just strip a conversion to void and try again.  */
  	      *expr_p = TREE_OPERAND (*expr_p, 0);
  	      break;
  	    }
--- 445,455 ----
  	  if (IS_EMPTY_STMT (*expr_p))
  	    break;
  
! 	  if (VOID_TYPE_P (TREE_TYPE (*expr_p))
! 	      || fallback == fb_none)
  	    {
! 	      /* Just strip a conversion to void (or in void context) and
! 		 try again.  */
  	      *expr_p = TREE_OPERAND (*expr_p, 0);
  	      break;
  	    }
*************** static void
*** 1231,1247 ****
  gimplify_conversion (tree *expr_p)
  {  
    /* If a NOP conversion is changing the type of a COMPONENT_REF
!      expression, then it is safe to force the type of the
!      COMPONENT_REF to be the same as the type of the field the
!      COMPONENT_REF is accessing.
! 
!      This is a profitable thing to do as canonicalization of
!      types on COMPONENT_REFs exposes more redundant COMPONENT_REFs.  */
    if (TREE_CODE (TREE_OPERAND (*expr_p, 0)) == COMPONENT_REF)
!     {
!       TREE_TYPE (TREE_OPERAND (*expr_p, 0))
! 	= TREE_TYPE (TREE_OPERAND (TREE_OPERAND (*expr_p, 0), 1));
!     }
  
    /* Strip away as many useless type conversions as possible
       at the toplevel.  */
--- 1231,1240 ----
  gimplify_conversion (tree *expr_p)
  {  
    /* If a NOP conversion is changing the type of a COMPONENT_REF
!      expression, then canonicalize its type now in order to expose more
!      redundant conversions.  */
    if (TREE_CODE (TREE_OPERAND (*expr_p, 0)) == COMPONENT_REF)
!     canonicalize_component_ref (&TREE_OPERAND (*expr_p, 0));
  
    /* Strip away as many useless type conversions as possible
       at the toplevel.  */
*************** gimplify_array_ref_to_plus (tree *expr_p
*** 1388,1393 ****
--- 1381,1425 ----
    *expr_p = build1 (INDIRECT_REF, elttype, result);
  }
  
+ /* *EXPR_P is a COMPONENT_REF being used as an rvalue.  If its type is
+    different from its canonical type, wrap the whole thing inside a
+    NOP_EXPR and force the type of the COMPONENT_REF to be the canonical
+    type.
+ 
+    The canonical type of a COMPONENT_REF is the type of the field being
+    referenced--unless the field is a bit-field which can be read directly
+    in a smaller mode, in which case the canonical type is the
+    sign-appropriate type corresponding to that mode.  */
+ 
+ static void
+ canonicalize_component_ref (tree *expr_p)
+ {
+   tree expr = *expr_p;
+   tree type;
+ 
+   if (TREE_CODE (expr) != COMPONENT_REF)
+     abort ();
+ 
+   if (INTEGRAL_TYPE_P (TREE_TYPE (expr)))
+     type = TREE_TYPE (get_unwidened (expr, NULL_TREE));
+   else
+     type = TREE_TYPE (TREE_OPERAND (expr, 1));
+ 
+   if (TREE_TYPE (expr) != type)
+     {
+       tree old_type = TREE_TYPE (expr);
+ 
+       /* Set the type of the COMPONENT_REF to the underlying type.  */
+       TREE_TYPE (expr) = type;
+ 
+       /* And wrap the whole thing inside a NOP_EXPR.  */
+       expr = build1 (NOP_EXPR, old_type, expr);
+       recalculate_side_effects (expr);
+ 
+       *expr_p = expr;
+     }
+ }
+ 
  /* Gimplify the COMPONENT_REF, ARRAY_REF, REALPART_EXPR or IMAGPART_EXPR
     node pointed by EXPR_P.
  
*************** gimplify_compound_lval (tree *expr_p, tr
*** 1468,1492 ****
        recalculate_side_effects (t);
      }
  
!   /* Now look at the toplevel expression.  If it is a COMPONENT_REF and
!      the type of the COMPONENT_REF is different than the field being
!      referenced, then wrap the whole thing inside a NOP_EXPR and force
!      the type of the COMPONENT_REF to be the same as the field being
!      referenced.  */
    if (! want_lvalue
!       && TREE_CODE (*expr_p) == COMPONENT_REF
!       && TREE_TYPE (*expr_p) != TREE_TYPE (TREE_OPERAND (*expr_p, 1)))
!     {
!       tree type_for_nop_expr = TREE_TYPE (*expr_p);
! 
!       /* Set the type of the COMPONENT_REF to the type of the field
! 	 being referenced.  */
!       TREE_TYPE (*expr_p) = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
! 
!       /* And wrap the whole thing inside a NOP_EXPR.  */
!       *expr_p = build1 (NOP_EXPR, type_for_nop_expr, *expr_p);
!       recalculate_side_effects (*expr_p);
!     }
  }
  
  /*  Gimplify the self modifying expression pointed by EXPR_P (++, --, +=, -=).
--- 1500,1510 ----
        recalculate_side_effects (t);
      }
  
!   /* If the outermost expression is a COMPONENT_REF, canonicalize its
!      type.  */
    if (! want_lvalue
!       && TREE_CODE (*expr_p) == COMPONENT_REF)
!     canonicalize_component_ref (expr_p);
  }
  
  /*  Gimplify the self modifying expression pointed by EXPR_P (++, --, +=, -=).
*************** gimplify_modify_expr (tree *expr_p, tree
*** 2060,2066 ****
       FIXME this should be handled by the is_gimple_rhs predicate.  */
    if (! is_gimple_tmp_var (*to_p)
        && (TREE_CODE (*from_p) == CALL_EXPR
! 	  || (flag_non_call_exceptions && tree_could_trap_p (*from_p))))
      gimplify_expr (from_p, pre_p, post_p, is_gimple_val, fb_rvalue);
  
    if (want_value)
--- 2078,2088 ----
       FIXME this should be handled by the is_gimple_rhs predicate.  */
    if (! is_gimple_tmp_var (*to_p)
        && (TREE_CODE (*from_p) == CALL_EXPR
! 	  || (flag_non_call_exceptions && tree_could_trap_p (*from_p))
! 	  /* If we're dealing with a renamable type, either source or dest
! 	     must be a renamed variable.  */
! 	  || (is_gimple_reg_type (TREE_TYPE (*from_p))
! 	      && !is_gimple_reg (*to_p))))
      gimplify_expr (from_p, pre_p, post_p, is_gimple_val, fb_rvalue);
  
    if (want_value)
*************** gimplify_asm_expr (tree *expr_p, tree *p
*** 2308,2314 ****
      *expr_p = build_empty_stmt ();
  }
  
! /* If EXPR is a boolean expression, make sure it has BOOLEAN_TYPE.  */
  
  static tree
  gimple_boolify (tree expr)
--- 2330,2336 ----
      *expr_p = build_empty_stmt ();
  }
  
! /* EXPR is used in a boolean context; make sure it has BOOLEAN_TYPE.  */
  
  static tree
  gimple_boolify (tree expr)
*** tree.c.~1~	2003-10-20 14:58:26.000000000 -0400
--- tree.c	2003-10-21 17:35:27.000000000 -0400
*************** get_unwidened (tree op, tree for_type)
*** 4200,4206 ****
      {
        unsigned int innerprec
  	= tree_low_cst (DECL_SIZE (TREE_OPERAND (op, 1)), 1);
!       int unsignedp = TREE_UNSIGNED (TREE_OPERAND (op, 1));
        type = (*lang_hooks.types.type_for_size) (innerprec, unsignedp);
  
        /* We can get this structure field in the narrowest type it fits in.
--- 4200,4207 ----
      {
        unsigned int innerprec
  	= tree_low_cst (DECL_SIZE (TREE_OPERAND (op, 1)), 1);
!       int unsignedp = (TREE_UNSIGNED (TREE_OPERAND (op, 1))
! 		       || TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op, 1))));
        type = (*lang_hooks.types.type_for_size) (innerprec, unsignedp);
  
        /* We can get this structure field in the narrowest type it fits in.
*************** get_narrower (tree op, int *unsignedp_pt
*** 4284,4291 ****
      {
        unsigned HOST_WIDE_INT innerprec
  	= tree_low_cst (DECL_SIZE (TREE_OPERAND (op, 1)), 1);
!       tree type = (*lang_hooks.types.type_for_size) (innerprec,
! 						     TREE_UNSIGNED (op));
  
        /* We can get this structure field in a narrower type that fits it,
  	 but the resulting extension to its nominal type (a fullword type)
--- 4285,4293 ----
      {
        unsigned HOST_WIDE_INT innerprec
  	= tree_low_cst (DECL_SIZE (TREE_OPERAND (op, 1)), 1);
!       int unsignedp = (TREE_UNSIGNED (TREE_OPERAND (op, 1))
! 		       || TREE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op, 1))));
!       tree type = (*lang_hooks.types.type_for_size) (innerprec, unsignedp);
  
        /* We can get this structure field in a narrower type that fits it,
  	 but the resulting extension to its nominal type (a fullword type)

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