PATCH to gimplify_modify_expr to avoid extra copies

PATCH to gimplify_modify_expr to avoid extra copies

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

This patch should help to avoid the extra copies kenner has been seeing.

Brian, can you make sure that my comment about V_MAY_DEF is accurate when
you check in your patch?  :)

Tested x86_64-pc-linux-gnu, applied to trunk.

2004-06-10  Jason Merrill  <jason@redhat.com>

	* gimplify.c (gimplify_modify_expr): Don't force a temporary
	of an aggregate_value_p type.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1511 bytes --]

*** gimplify.c.~1~	2004-06-10 16:27:29.000000000 -0400
--- gimplify.c	2004-06-10 22:54:18.000000000 -0400
*************** gimplify_modify_expr (tree *expr_p, tree
*** 2493,2504 ****
  
  	 FIXME this should be handled by the is_gimple_rhs predicate.  */
  
!       if (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 the value being copied is of variable width, expose the length
--- 2493,2508 ----
  
  	 FIXME this should be handled by the is_gimple_rhs predicate.  */
  
!       if (aggregate_value_p (TREE_TYPE (*from_p), NULL_TREE))
! 	/* Don't force a temp of a large aggregate type; the copy could be
! 	   arbitrarily expensive.  Instead we will generate a V_MAY_DEF for
! 	   the assignment.  */;
!       else if (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 the value being copied is of variable width, expose the length

Re: PATCH to gimplify_modify_expr to avoid extra copies

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On Thu, 10 Jun 2004 23:05:53 -0400, Jason Merrill <jason@redhat.com> wrote:

> Brian, can you make sure that my comment about V_MAY_DEF is accurate when
> you check in your patch?  :)
>
> ! 	/* Don't force a temp of a large aggregate type; the copy could be
> ! 	   arbitrarily expensive.  Instead we will generate a V_MAY_DEF for
> ! 	   the assignment.  */;

Ah, I see you already checked it in.  Does something like this make sense
to you?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1279 bytes --]

*** tree-ssa-operands.c.~1~	2004-06-10 23:07:12.000000000 -0400
--- tree-ssa-operands.c	2004-06-10 23:24:32.000000000 -0400
*************** get_stmt_operands (tree stmt)
*** 821,827 ****
        if (TREE_CODE (TREE_OPERAND (stmt, 0)) == ARRAY_REF 
            || TREE_CODE (TREE_OPERAND (stmt, 0)) == COMPONENT_REF
  	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
! 	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR)
          get_expr_operands (stmt, &TREE_OPERAND (stmt, 0), opf_is_def, 
  	                   &prev_vops);
        else
--- 821,833 ----
        if (TREE_CODE (TREE_OPERAND (stmt, 0)) == ARRAY_REF 
            || TREE_CODE (TREE_OPERAND (stmt, 0)) == COMPONENT_REF
  	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
! 	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR
! 	  /* Use a V_MAY_DEF if the RHS might throw, as the LHS won't be
! 	     modified in that case.  FIXME we should represent somehow
! 	     that it is killed on the fallthrough path.  */
! 	  || TREE_CODE (TREE_OPERAND (stmt, 1)) == CALL_EXPR
! 	  || (flag_non_call_exceptions
! 	      && tree_could_trap_p (TREE_OPERAND (stmt, 1))))
          get_expr_operands (stmt, &TREE_OPERAND (stmt, 0), opf_is_def, 
  	                   &prev_vops);
        else

Re: PATCH to gimplify_modify_expr to avoid extra copies

[Richard Henderson]

On Thu, Jun 10, 2004 at 11:27:25PM -0400, Jason Merrill wrote:
> ! 	  /* Use a V_MAY_DEF if the RHS might throw, as the LHS won't be
> ! 	     modified in that case.  FIXME we should represent somehow
> ! 	     that it is killed on the fallthrough path.  */
> ! 	  || TREE_CODE (TREE_OPERAND (stmt, 1)) == CALL_EXPR
> ! 	  || (flag_non_call_exceptions
> ! 	      && tree_could_trap_p (TREE_OPERAND (stmt, 1))))

After gimplification, we can do better than this; use tree_could_throw_p.

Hmm, I see we currently only use this on complete statements.  There's a
bug here for expressions -- the final return should be checking
tree_could_trap_p.


r~

Re: PATCH to gimplify_modify_expr to avoid extra copies

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

I'm applying this patch.  Tested x86_64-pc-linux-gnu.

2004-06-11  Jason Merrill  <jason@redhat.com>

	* tree-ssa-operands.c: Use a V_MAY_DEF if the assignment might
	throw.
	* tree-eh.c (tree_could_throw_p): Support non-call exceptions in
	expressions.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2634 bytes --]

Index: gcc/tree-eh.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-eh.c,v
retrieving revision 2.3
diff -c -p -r2.3 tree-eh.c
*** gcc/tree-eh.c	30 May 2004 18:32:29 -0000	2.3
--- gcc/tree-eh.c	11 Jun 2004 17:33:12 -0000
*************** tree_could_throw_p (tree t)
*** 1716,1739 ****
      return false;
    if (TREE_CODE (t) == MODIFY_EXPR)
      {
!       tree sub = TREE_OPERAND (t, 1);
!       if (TREE_CODE (sub) == CALL_EXPR)
! 	t = sub;
!       else
! 	{
! 	  if (flag_non_call_exceptions)
! 	    {
! 	      if (tree_could_trap_p (sub))
! 		return true;
! 	      return tree_could_trap_p (TREE_OPERAND (t, 0));
! 	    }
! 	  return false;
! 	}
      }
  
    if (TREE_CODE (t) == CALL_EXPR)
      return (call_expr_flags (t) & ECF_NOTHROW) == 0;
! 
    return false;
  }
  
--- 1716,1731 ----
      return false;
    if (TREE_CODE (t) == MODIFY_EXPR)
      {
!       if (flag_non_call_exceptions
! 	  && tree_could_trap_p (TREE_OPERAND (t, 0)))
! 	return true;
!       t = TREE_OPERAND (t, 1);
      }
  
    if (TREE_CODE (t) == CALL_EXPR)
      return (call_expr_flags (t) & ECF_NOTHROW) == 0;
!   if (flag_non_call_exceptions)
!     return tree_could_trap_p (t);
    return false;
  }
  
Index: gcc/tree-ssa-operands.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
retrieving revision 2.5
diff -c -p -r2.5 tree-ssa-operands.c
*** gcc/tree-ssa-operands.c	10 Jun 2004 21:41:07 -0000	2.5
--- gcc/tree-ssa-operands.c	11 Jun 2004 17:33:12 -0000
*************** get_stmt_operands (tree stmt)
*** 821,827 ****
        if (TREE_CODE (TREE_OPERAND (stmt, 0)) == ARRAY_REF 
            || TREE_CODE (TREE_OPERAND (stmt, 0)) == COMPONENT_REF
  	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
! 	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR)
          get_expr_operands (stmt, &TREE_OPERAND (stmt, 0), opf_is_def, 
  	                   &prev_vops);
        else
--- 821,831 ----
        if (TREE_CODE (TREE_OPERAND (stmt, 0)) == ARRAY_REF 
            || TREE_CODE (TREE_OPERAND (stmt, 0)) == COMPONENT_REF
  	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
! 	  || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR
! 	  /* Use a V_MAY_DEF if the RHS might throw, as the LHS won't be
! 	     modified in that case.  FIXME we should represent somehow
! 	     that it is killed on the fallthrough path.  */
! 	  || tree_could_throw_p (TREE_OPERAND (stmt, 1)))
          get_expr_operands (stmt, &TREE_OPERAND (stmt, 0), opf_is_def, 
  	                   &prev_vops);
        else

Re: PATCH to gimplify_modify_expr to avoid extra copies

[Richard Kenner]

    This patch should help to avoid the extra copies kenner has been seeing.

I *think* it does, but can't be sure (I'm blowing up in a different place
in that file and can't be sure that it's later; I'm travelling now).