diff mbox

Perform anonymous constant propagation during inlining

Message ID 1725052.qXkxHEI06s@polaris
State New
Headers show

Commit Message

Eric Botcazou May 11, 2015, 2:05 p.m. UTC
> >Would you be OK with a slight variation of your earlier idea, i.e.
> >calling fold_stmt with a specific valueizer from fold_marked_statements
> >instead of the implicit no_follow_ssa_edges in the inliner?  Something
> >like:
> >
> >tree
> >follow_anonymous_single_use_edges (tree val)
> >{
> >
> >  if (TREE_CODE (val) == SSA_NAME
> >  
> >      && (!SSA_NAME_VAR (val) || DECL_IGNORED_P (SSA_NAME_VAR
> >      (var)))
> >      && has_single_use (val))
> >    
> >    return val
> >  
> >  return NULL_TREE;
> >
> >}
> 
> Yes, that works for me as well.

But not for GCC. :-)  The propagation per se works but, since the statement is 
not folded in the end, no replacement is made at all...

So we're back to square one and anonymous constant propagation seems to be the 
only way out at -O0.  The attached patch implements it in a less hackish way 
(and enables it unconditionally when optimizing as suggested by Jan) by doing 
it just before invoking fold_stmt on the marked statements so this should make 
folding more effective in the process. 

Tested (compiler only) on x86_64-suse-linux, OK for the mainline?


2015-05-11  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-inline.c: Include tree-ssa-propagate.h.
	(replace_constant_uses_in): New function.
	(fold_marked_statements): Call it before folding the statement.

	* gimple-expr.h (is_gimple_constant): Reorder.
	* tree-ssa-propagate.c (before_dom_children): Use inline accessor.

Comments

Richard Biener May 12, 2015, 8:26 a.m. UTC | #1
On Mon, May 11, 2015 at 4:05 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> >Would you be OK with a slight variation of your earlier idea, i.e.
>> >calling fold_stmt with a specific valueizer from fold_marked_statements
>> >instead of the implicit no_follow_ssa_edges in the inliner?  Something
>> >like:
>> >
>> >tree
>> >follow_anonymous_single_use_edges (tree val)
>> >{
>> >
>> >  if (TREE_CODE (val) == SSA_NAME
>> >
>> >      && (!SSA_NAME_VAR (val) || DECL_IGNORED_P (SSA_NAME_VAR
>> >      (var)))
>> >      && has_single_use (val))
>> >
>> >    return val
>> >
>> >  return NULL_TREE;
>> >
>> >}
>>
>> Yes, that works for me as well.
>
> But not for GCC. :-)  The propagation per se works but, since the statement is
> not folded in the end, no replacement is made at all...
>
> So we're back to square one and anonymous constant propagation seems to be the
> only way out at -O0.  The attached patch implements it in a less hackish way
> (and enables it unconditionally when optimizing as suggested by Jan) by doing
> it just before invoking fold_stmt on the marked statements so this should make
> folding more effective in the process.
>
> Tested (compiler only) on x86_64-suse-linux, OK for the mainline?

+  if (replaced && gimple_assign_single_p (stmt))
+    {
+      tree rhs = gimple_assign_rhs1 (stmt);
+
+      if (TREE_CODE (rhs) == ADDR_EXPR)
+       recompute_tree_invariant_for_addr_expr (rhs);
+    }

this needs to handle all non-PHI stmt kinds (asms and calls), see
tree-cfg.c:replace_uses_by
(maybe factor out a recompute_tree_invariant_for_gimple_ops).

Looking at the whole thing I think it might be better to

 a) _not_ propagate parameter values during the stmt copies
 b) _not_ mark any stmts for folding
 c) at fold_marked_stmts time instead treat the incoming parameter
     mappings (which param setup "inlined" as param_1 = value1; etc...)
     as a seed for the worklist of a simple SSA propagator doing sth like

 while (!bitmap_empty (worklist))
   {
     tree name = ssa_name (bitmap_first_bit (worklist));
     gimple stmt = SSA_NAME_DEF_STMT (name);
     gcc_assert (gimple_assign_single_p (stmt)
                   && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)));
     FOR_EACH_IMM_USE_STMT (name)
        {
         FOR_EACH_IMM_USE_ON_STMT (..)
            SET_USE (...)
         fold_stmt (use_stmt);
           if (gimple_assing_single_p (use_stmt)
               && is_gimple_min_invariant (gimple_assign_rhs1 (use_stmt)))
            bitmap_set_bit (worklist, SSA_NAME_VERSION
(gimple_assign_lhs (use_stmt)));
        }
     bitmap_clear_bit (SSA_NAME_VERSION (name));
     /* remove 'stmt' if we replaced all uses - properly generating
debug stmts  */
   }

appropriately restricted for !optimize and eventually also propagating
copies for !optimize.

This should make sure we keep all stmts properly folded after inlining
and it should more
reliably propagate constants exposed by inlining (it should be cheaper
than a full SSA CCP
pass because we likely visit less stmts by just taking constant
parameters as propagation
sources).

Richard.

>
> 2015-05-11  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree-inline.c: Include tree-ssa-propagate.h.
>         (replace_constant_uses_in): New function.
>         (fold_marked_statements): Call it before folding the statement.
>
>         * gimple-expr.h (is_gimple_constant): Reorder.
>         * tree-ssa-propagate.c (before_dom_children): Use inline accessor.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: tree-ssa-propagate.c
===================================================================
--- tree-ssa-propagate.c	(revision 222673)
+++ tree-ssa-propagate.c	(working copy)
@@ -1246,9 +1246,7 @@  substitute_and_fold_dom_walker::before_d
 	      && gimple_call_noreturn_p (stmt))
 	    stmts_to_fixup.safe_push (stmt);
 
-	  if (is_gimple_assign (stmt)
-	      && (get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
-		  == GIMPLE_SINGLE_RHS))
+	  if (gimple_assign_single_p (stmt))
 	    {
 	      tree rhs = gimple_assign_rhs1 (stmt);
 
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 222673)
+++ tree-inline.c	(working copy)
@@ -82,6 +82,7 @@  along with GCC; see the file COPYING3.
 #include "expr.h"
 #include "tree-dfa.h"
 #include "tree-ssa.h"
+#include "tree-ssa-propagate.h"
 #include "tree-pretty-print.h"
 #include "except.h"
 #include "debug.h"
@@ -4801,6 +4802,54 @@  gimple_expand_calls_inline (basic_block
   return inlined;
 }
 
+/* Replace USE references in statement STMT with constant values.  Return
+   true if at least one reference was replaced.
+
+   We do it at -O0 for anonymous SSA names or SSA names of anonymous decls,
+   this makes it possible to generate reasonable code for operators that are
+   implemented as functions and inlined on constants.  */
+
+static bool
+replace_constant_uses_in (gimple stmt)
+{
+  bool replaced = false;
+  use_operand_p use;
+  ssa_op_iter iter;
+
+  FOR_EACH_SSA_USE_OPERAND (use, stmt, iter, SSA_OP_USE)
+    {
+      tree tuse = USE_FROM_PTR (use);
+
+      if (TREE_CODE (tuse) != SSA_NAME || SSA_NAME_IS_DEFAULT_DEF (tuse))
+	continue;
+
+      if (!optimize
+	  && SSA_NAME_VAR (tuse)
+	  && !DECL_IGNORED_P (SSA_NAME_VAR (tuse)))
+	continue;
+
+      if (!gimple_assign_single_p (SSA_NAME_DEF_STMT (tuse)))
+	continue;
+
+      tree rhs = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (tuse));
+      if (!is_gimple_constant (rhs))
+	continue;
+
+      propagate_value (use, rhs);
+
+      replaced = true;
+    }
+
+  if (replaced && gimple_assign_single_p (stmt))
+    {
+      tree rhs = gimple_assign_rhs1 (stmt);
+
+      if (TREE_CODE (rhs) == ADDR_EXPR)
+	recompute_tree_invariant_for_addr_expr (rhs);
+    }
+
+  return replaced;
+}
 
 /* Walk all basic blocks created after FIRST and try to fold every statement
    in the STATEMENTS pointer set.  */
@@ -4819,7 +4868,10 @@  fold_marked_statements (int first, hash_
 	  if (statements->contains (gsi_stmt (gsi)))
 	    {
 	      gimple old_stmt = gsi_stmt (gsi);
-	      tree old_decl = is_gimple_call (old_stmt) ? gimple_call_fndecl (old_stmt) : 0;
+	      tree old_decl
+		= is_gimple_call (old_stmt)
+		  ? gimple_call_fndecl (old_stmt) : NULL_TREE;
+	      bool replaced = replace_constant_uses_in (old_stmt);
 
 	      if (old_decl && DECL_BUILT_IN (old_decl))
 		{
@@ -4827,7 +4879,7 @@  fold_marked_statements (int first, hash_
 		     we need to look at all of them.  */
 		  gimple_stmt_iterator i2 = gsi;
 		  gsi_prev (&i2);
-		  if (fold_stmt (&gsi))
+		  if (fold_stmt (&gsi) || replaced)
 		    {
 		      gimple new_stmt;
 		      /* If a builtin at the end of a bb folded into nothing,
@@ -4871,7 +4923,7 @@  fold_marked_statements (int first, hash_
 			}
 		    }
 		}
-	      else if (fold_stmt (&gsi))
+	      else if (fold_stmt (&gsi) || replaced)
 		{
 		  /* Re-read the statement from GSI as fold_stmt() may
 		     have changed it.  */
Index: gimple-expr.h
===================================================================
--- gimple-expr.h	(revision 222673)
+++ gimple-expr.h	(working copy)
@@ -136,9 +136,9 @@  is_gimple_constant (const_tree t)
     case INTEGER_CST:
     case REAL_CST:
     case FIXED_CST:
-    case STRING_CST:
     case COMPLEX_CST:
     case VECTOR_CST:
+    case STRING_CST:
       return true;
 
     default: