public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][4.7][RFC] Fix store-sinking, make returns have a VUSE
@ 2011-03-09 15:26 Richard Guenther
  2011-03-09 15:36 ` Diego Novillo
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Guenther @ 2011-03-09 15:26 UTC (permalink / raw)
  To: gcc-patches


This old patch of mine makes the return statement of a function
have a VUSE even if it does not return a value (or does not reference
memory).  This is to have a virtual operand chain for values that
escape the function via returns.

This allows to fix store-sinking to do its job again by simply using
the fact that we always will have a PHI node merging the virtual
operands (which does not happen right now if there is no load or
store following the location we want to sink to).

I have just forward ported this patch to trunk and don't remember
whether it caused any problems back when I originally worked on this
(IIRC it was during stage3 of 4.5).  So, this is just a heads-up
in case you have any issues with having more virtual operands.

If the patch happens to bootstrap and test ok I plan to commit this
somewhen during stage1.

Richard.

2011-03-09  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/41490
	* tree-ssa-dce.c (propagate_necessity): Handle returns without
	value but with VUSE.
	* tree-ssa-operands.c (parse_ssa_operands): Add a VUSE on all
	return statements.
	* tree-ssa-sink.c (statement_sink_location): Fix store sinking.

	* gcc.dg/tree-ssa/ssa-sink-6.c: New testcase.
	* gcc.dg/tree-ssa/ssa-sink-7.c: Likewise.
	* gcc.dg/tree-ssa/ssa-sink-8.c: Likewise.
	* gcc.dg/tree-ssa/ssa-sink-9.c: Likewise.

Index: gcc/tree-ssa-dce.c
===================================================================
*** gcc/tree-ssa-dce.c.orig	2011-02-23 16:18:20.000000000 +0100
--- gcc/tree-ssa-dce.c	2011-03-09 15:58:34.000000000 +0100
*************** propagate_necessity (struct edge_list *e
*** 869,875 ****
  	    {
  	      tree rhs = gimple_return_retval (stmt);
  	      /* A return statement may perform a load.  */
! 	      if (TREE_CODE (rhs) != SSA_NAME
  		  && !is_gimple_min_invariant (rhs))
  		{
  		  if (!ref_may_be_aliased (rhs))
--- 869,876 ----
  	    {
  	      tree rhs = gimple_return_retval (stmt);
  	      /* A return statement may perform a load.  */
! 	      if (rhs
! 		  && TREE_CODE (rhs) != SSA_NAME
  		  && !is_gimple_min_invariant (rhs))
  		{
  		  if (!ref_may_be_aliased (rhs))
Index: gcc/tree-ssa-operands.c
===================================================================
*** gcc/tree-ssa-operands.c.orig	2010-11-30 15:31:11.000000000 +0100
--- gcc/tree-ssa-operands.c	2011-03-09 15:58:34.000000000 +0100
*************** parse_ssa_operands (gimple stmt)
*** 1065,1070 ****
--- 1065,1073 ----
        /* Add call-clobbered operands, if needed.  */
        if (code == GIMPLE_CALL)
  	maybe_add_call_vops (stmt);
+ 
+       if (code == GIMPLE_RETURN)
+ 	append_vuse (gimple_vop (cfun));
      }
  }
  
Index: gcc/tree-ssa-sink.c
===================================================================
*** gcc/tree-ssa-sink.c.orig	2011-02-15 15:08:49.000000000 +0100
--- gcc/tree-ssa-sink.c	2011-03-09 16:08:46.000000000 +0100
*************** statement_sink_location (gimple stmt, ba
*** 276,299 ****
    ssa_op_iter iter;
    imm_use_iterator imm_iter;
  
!   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
!     {
!       FOR_EACH_IMM_USE_FAST (one_use, imm_iter, def)
! 	{
! 	  if (is_gimple_debug (USE_STMT (one_use)))
! 	    continue;
! 
! 	  break;
! 	}
!       if (one_use != NULL_USE_OPERAND_P)
!         break;
!     }
  
!   /* Return if there are no immediate uses of this stmt.  */
!   if (one_use == NULL_USE_OPERAND_P)
      return false;
  
!   if (gimple_code (stmt) != GIMPLE_ASSIGN)
      return false;
  
    /* There are a few classes of things we can't or don't move, some because we
--- 276,292 ----
    ssa_op_iter iter;
    imm_use_iterator imm_iter;
  
!   /* We only can sink assignments.  */
!   if (!is_gimple_assign (stmt))
!     return false;
  
!   /* We only can sink stmts with a single definition.  */
!   def_p = single_ssa_def_operand (stmt, SSA_OP_ALL_DEFS);
!   if (def_p == NULL_DEF_OPERAND_P)
      return false;
  
!   /* Return if there are no immediate uses of this stmt.  */
!   if (has_zero_uses (DEF_FROM_PTR (def_p)))
      return false;
  
    /* There are a few classes of things we can't or don't move, some because we
*************** statement_sink_location (gimple stmt, ba
*** 323,342 ****
    */
    if (stmt_ends_bb_p (stmt)
        || gimple_has_side_effects (stmt)
-       || is_hidden_global_store (stmt)
        || gimple_has_volatile_ops (stmt)
!       || gimple_vuse (stmt)
        || (cfun->has_local_explicit_reg_vars
  	  && TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt))) == BLKmode))
      return false;
  
!   FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, iter, SSA_OP_ALL_DEFS)
!     {
!       tree def = DEF_FROM_PTR (def_p);
!       if (is_global_var (SSA_NAME_VAR (def))
! 	  || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (def))
! 	return false;
!     }
  
    FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
      {
--- 316,329 ----
    */
    if (stmt_ends_bb_p (stmt)
        || gimple_has_side_effects (stmt)
        || gimple_has_volatile_ops (stmt)
!       || (gimple_vuse (stmt) && !gimple_vdef (stmt))
        || (cfun->has_local_explicit_reg_vars
  	  && TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt))) == BLKmode))
      return false;
  
!   if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (DEF_FROM_PTR (def_p)))
!     return false;
  
    FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
      {
*************** statement_sink_location (gimple stmt, ba
*** 345,355 ****
  	return false;
      }
  
    /* If all the immediate uses are not in the same place, find the nearest
       common dominator of all the immediate uses.  For PHI nodes, we have to
       find the nearest common dominator of all of the predecessor blocks, since
       that is where insertion would have to take place.  */
!   if (!all_immediate_uses_same_place (stmt))
      {
        bool debug_stmts = false;
        basic_block commondom = nearest_common_dominator_of_uses (stmt,
--- 332,371 ----
  	return false;
      }
  
+   use = NULL;
+ 
+   /* If stmt is a store the one and only use needs to be the VOP
+      merging PHI node.  */
+   if (gimple_vdef (stmt))
+     {
+       FOR_EACH_IMM_USE_FAST (use_p, imm_iter, DEF_FROM_PTR (def_p))
+ 	{
+ 	  gimple use_stmt = USE_STMT (use_p);
+ 
+ 	  /* A killing definition is not a use.  */
+ 	  if (gimple_assign_single_p (use_stmt)
+ 	      && gimple_vdef (use_stmt)
+ 	      && operand_equal_p (gimple_assign_lhs (stmt),
+ 				  gimple_assign_lhs (use_stmt), 0))
+ 	    continue;
+ 
+ 	  if (gimple_code (use_stmt) != GIMPLE_PHI)
+ 	    return false;
+ 
+ 	  if (use
+ 	      && use != use_stmt)
+ 	    return false;
+ 
+ 	  use = use_stmt;
+ 	}
+       if (!use)
+ 	return false;
+     }
    /* If all the immediate uses are not in the same place, find the nearest
       common dominator of all the immediate uses.  For PHI nodes, we have to
       find the nearest common dominator of all of the predecessor blocks, since
       that is where insertion would have to take place.  */
!   else if (!all_immediate_uses_same_place (stmt))
      {
        bool debug_stmts = false;
        basic_block commondom = nearest_common_dominator_of_uses (stmt,
*************** statement_sink_location (gimple stmt, ba
*** 387,417 ****
  
        return true;
      }
! 
!   use = USE_STMT (one_use);
!   if (gimple_code (use) != GIMPLE_PHI)
      {
!       sinkbb = gimple_bb (use);
!       if (sinkbb == frombb || sinkbb->loop_depth > frombb->loop_depth
! 	  || sinkbb->loop_father != frombb->loop_father)
! 	return false;
  
!       /* Move the expression to a post dominator can't reduce the number of
!          executions.  */
!       if (dominated_by_p (CDI_POST_DOMINATORS, frombb, sinkbb))
!         return false;
  
!       *togsi = gsi_for_stmt (use);
  
!       return true;
      }
  
!   /* Note that at this point, all uses must be in the same statement, so it
!      doesn't matter which def op we choose, pick the first one.  */
!   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
!     break;
! 
!   sinkbb = find_bb_for_arg (use, def);
    if (!sinkbb)
      return false;
  
--- 403,437 ----
  
        return true;
      }
!   else
      {
!       FOR_EACH_IMM_USE_FAST (one_use, imm_iter, DEF_FROM_PTR (def_p))
! 	{
! 	  if (is_gimple_debug (USE_STMT (one_use)))
! 	    continue;
! 	  break;
! 	}
!       use = USE_STMT (one_use);
  
!       if (gimple_code (use) != GIMPLE_PHI)
! 	{
! 	  sinkbb = gimple_bb (use);
! 	  if (sinkbb == frombb || sinkbb->loop_depth > frombb->loop_depth
! 	      || sinkbb->loop_father != frombb->loop_father)
! 	    return false;
! 
! 	  /* Move the expression to a post dominator can't reduce the number of
! 	     executions.  */
! 	  if (dominated_by_p (CDI_POST_DOMINATORS, frombb, sinkbb))
! 	    return false;
  
! 	  *togsi = gsi_for_stmt (use);
  
! 	  return true;
! 	}
      }
  
!   sinkbb = find_bb_for_arg (use, DEF_FROM_PTR (def_p));
    if (!sinkbb)
      return false;
  
*************** sink_code_in_bb (basic_block bb)
*** 486,491 ****
--- 506,519 ----
  		   bb->index, (gsi_bb (togsi))->index);
  	}
  
+       /* Prepare for VOP update.  */
+       if (gimple_vdef (stmt))
+ 	{
+ 	  unlink_stmt_vdef (stmt);
+ 	  gimple_set_vdef (stmt, gimple_vop (cfun));
+ 	  mark_sym_for_renaming (gimple_vop (cfun));
+ 	}
+ 
        /* If this is the end of the basic block, we need to insert at the end
           of the basic block.  */
        if (gsi_end_p (togsi))
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-6.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-6.c	2011-03-09 16:13:45.000000000 +0100
***************
*** 0 ****
--- 1,18 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-sink" } */
+ 
+ int foo(int *a, int r)
+ {
+   int ret = 0;
+   *a = 1;
+   if (r == 3)
+     *a = 5;
+   else
+     ret = r + 20;
+   return ret;
+ }
+ 
+ /* *a = 1 should be sunk to the else block.  */
+ 
+ /* { dg-final { scan-tree-dump-times "Sinking" 1 "sink" } } */
+ /* { dg-final { cleanup-tree-dump "sink" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-7.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-7.c	2011-03-09 16:13:39.000000000 +0100
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-sink" } */
+ 
+ int foo(int *a, int r, short *b)
+ {
+   int ret = 0;
+   *a = 1;
+   if (r == 3)
+     *a = 5;
+   else
+     ret = r + 20;
+   *b = 9;
+   return ret;
+ }
+ 
+ /* *a = 1 should be sunk to the else block.  */
+ 
+ /* { dg-final { scan-tree-dump-times "Sinking" 1 "sink" } } */
+ /* { dg-final { cleanup-tree-dump "sink" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-8.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-8.c	2011-03-09 16:13:18.000000000 +0100
***************
*** 0 ****
--- 1,28 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-sink" } */
+ 
+ int foo(int *a, int r, short *b)
+ {
+   int ret = 0;
+   *a = 1;
+   switch (r)
+     {
+       case 3:
+ 	  *a = 5;
+ 	  break;
+       case 4:
+       case 5:
+ 	  *a = 9;
+ 	  ret = r + 25;
+ 	  break;
+       default:
+ 	  ret = r + 20;
+     }
+   *b = 9;
+   return ret;
+ }
+ 
+ /* *a = 1 should be sunk into the default case.  */
+ 
+ /* { dg-final { scan-tree-dump-times "Sinking" 1 "sink" } } */
+ /* { dg-final { cleanup-tree-dump "sink" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-9.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-9.c	2011-03-09 16:15:52.000000000 +0100
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-sink" } */
+ 
+ int foo(int *a, int r, int *b)
+ {
+   int ret = 0;
+   *a = 1;
+   if (r == 3)
+     {
+       *a = 5;
+       *b = 3;
+     }
+   return ret;
+ }
+ 
+ /* *a = 1 should be sunk to the else block.  */
+ 
+ /* { dg-final { scan-tree-dump-times "Sinking" 1 "sink" } } */
+ /* { dg-final { cleanup-tree-dump "sink" } } */

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH][4.7][RFC] Fix store-sinking, make returns have a VUSE
  2011-03-09 15:26 [PATCH][4.7][RFC] Fix store-sinking, make returns have a VUSE Richard Guenther
@ 2011-03-09 15:36 ` Diego Novillo
  0 siblings, 0 replies; 2+ messages in thread
From: Diego Novillo @ 2011-03-09 15:36 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Wed, Mar 9, 2011 at 10:26, Richard Guenther <rguenther@suse.de> wrote:

> I have just forward ported this patch to trunk and don't remember
> whether it caused any problems back when I originally worked on this
> (IIRC it was during stage3 of 4.5).  So, this is just a heads-up
> in case you have any issues with having more virtual operands.

Not at all.  This looks useful.


Diego.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-03-09 15:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-09 15:26 [PATCH][4.7][RFC] Fix store-sinking, make returns have a VUSE Richard Guenther
2011-03-09 15:36 ` Diego Novillo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).