[RFA][PATCH] Provide a class interface into substitute_and_fold.

Jeff Law law@redhat.com
Tue Oct 24 18:57:00 GMT 2017


This is similar to the introduction of the ssa_propagate_engine, but for
the substitution/replacements bits.

In a couple places the pass specific virtual functions are just wrappers
around existing functions.  A good example of this is
ccp_folder::get_value.  Many other routines in tree-ssa-ccp.c want to
use get_constant_value.  Some may be convertable to use the class
instance, but I haven't looked closely.

Another example is vrp_folder::get_value.  In this case we're wrapping
op_with_constant_singleton_value.  In a later patch that moves into the
to-be-introduced vr_values class so we'll delegate to that class rather
than wrap.

FWIW I did look at having a single class for the propagation engine and
the substitution engine.  That turned out to be a bit problematical due
to the calls into the substitution engine from the evrp bits which don't
use the propagation engine at all.  Given propagation and substitution
are distinct concepts I ultimately decided the cleanest path forward was
to keep the two classes separate.

Bootstrapped and regression tested on x86_64.  OK for the trunk?

Jeff


-------------- next part --------------
	* tree-ssa-ccp.c (ccp_folder): New class derived from
	substitute_and_fold_engine.
	(ccp_folder::get_value): New member function.
	(ccp_folder::fold_stmt): Renamed from ccp_fold_stmt.
	(ccp_fold_stmt): Remove prototype.
	(ccp_finalize): Call substitute_and_fold from the ccp_class.
	* tree-ssa-copy.c (copy_folder): New class derived from
	substitute_and_fold_engine.
	(copy_folder::get_value): Renamed from get_value.
	(fini_copy_prop): Call substitute_and_fold from copy_folder class.
	* tree-vrp.c (vrp_folder): New class derived from
	substitute_and_fold_engine.
	(vrp_folder::fold_stmt): Renamed from vrp_fold_stmt.
	(vrp_folder::get_value): New member function.
	(vrp_finalize): Call substitute_and_fold from vrp_folder class.
	(evrp_dom_walker::before_dom_children): Similarly for replace_uses_in.
	* tree-ssa-propagate.h (substitute_and_fold_engine): New class to
	provide a class interface to folder/substitute routines.
	(ssa_prop_fold_stmt_fn): Remove typedef.
	(ssa_prop_get_value_fn): Likewise.
	(subsitute_and_fold): Remove prototype.
	(replace_uses_in): Likewise.
	* tree-ssa-propagate.c (substitute_and_fold_engine::replace_uses_in):
	Renamed from replace_uses_in.  Call the virtual member function
	(substitute_and_fold_engine::replace_phi_args_in): Similarly.
	(substitute_and_fold_dom_walker): Remove initialization of
	data member entries for calbacks.  Add substitute_and_fold_engine
	member and initialize it.
	(substitute_and_fold_dom_walker::before_dom_children0: Use the
	member functions for get_value, replace_phi_args_in c
	replace_uses_in, and fold_stmt calls.
	(substitute_and_fold_engine::substitute_and_fold): Renamed from
	substitute_and_fold.  Remove assert.   Update ctor call.


diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index fec562e..da06172 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -188,7 +188,6 @@ static ccp_prop_value_t *const_val;
 static unsigned n_const_val;
 
 static void canonicalize_value (ccp_prop_value_t *);
-static bool ccp_fold_stmt (gimple_stmt_iterator *);
 static void ccp_lattice_meet (ccp_prop_value_t *, ccp_prop_value_t *);
 
 /* Dump constant propagation value VAL to file OUTF prefixed by PREFIX.  */
@@ -909,6 +908,24 @@ do_dbg_cnt (void)
 }
 
 
+/* We want to provide our own GET_VALUE and FOLD_STMT virtual methods.  */
+class ccp_folder : public substitute_and_fold_engine
+{
+ public:
+  tree get_value (tree);
+  bool fold_stmt (gimple_stmt_iterator *);
+};
+
+/* This method just wraps GET_CONSTANT_VALUE for now.  Over time
+   naked calls to GET_CONSTANT_VALUE should be eliminated in favor
+   of calling member functions.  */
+
+tree
+ccp_folder::get_value (tree op)
+{
+  return get_constant_value (op);
+}
+
 /* Do final substitution of propagated values, cleanup the flowgraph and
    free allocated storage.  If NONZERO_P, record nonzero bits.
 
@@ -967,7 +984,8 @@ ccp_finalize (bool nonzero_p)
     }
 
   /* Perform substitutions based on the known constant values.  */
-  something_changed = substitute_and_fold (get_constant_value, ccp_fold_stmt);
+  class ccp_folder ccp_folder;
+  something_changed = ccp_folder.substitute_and_fold ();
 
   free (const_val);
   const_val = NULL;
@@ -2176,8 +2194,8 @@ fold_builtin_alloca_with_align (gimple *stmt)
 /* Fold the stmt at *GSI with CCP specific information that propagating
    and regular folding does not catch.  */
 
-static bool
-ccp_fold_stmt (gimple_stmt_iterator *gsi)
+bool
+ccp_folder::fold_stmt (gimple_stmt_iterator *gsi)
 {
   gimple *stmt = gsi_stmt (*gsi);
 
diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index a57535c..e45d188 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -489,10 +489,16 @@ init_copy_prop (void)
     }
 }
 
+class copy_folder : public substitute_and_fold_engine
+{
+ public:
+  tree get_value (tree);
+};
+
 /* Callback for substitute_and_fold to get at the final copy-of values.  */
 
-static tree
-get_value (tree name)
+tree
+copy_folder::get_value (tree name)
 {
   tree val;
   if (SSA_NAME_VERSION (name) >= n_copy_of)
@@ -557,7 +563,8 @@ fini_copy_prop (void)
 	}
     }
 
-  bool changed = substitute_and_fold (get_value, NULL);
+  class copy_folder copy_folder;
+  bool changed = copy_folder.substitute_and_fold ();
   if (changed)
     {
       free_numbers_of_iterations_estimates (cfun);
diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 90df285..62955be 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -853,7 +853,7 @@ static struct prop_stats_d prop_stats;
    PROP_VALUE. Return true if at least one reference was replaced.  */
 
 bool
-replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value)
+substitute_and_fold_engine::replace_uses_in (gimple *stmt)
 {
   bool replaced = false;
   use_operand_p use;
@@ -862,7 +862,7 @@ replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value)
   FOR_EACH_SSA_USE_OPERAND (use, stmt, iter, SSA_OP_USE)
     {
       tree tuse = USE_FROM_PTR (use);
-      tree val = (*get_value) (tuse);
+      tree val = get_value (tuse);
 
       if (val == tuse || val == NULL_TREE)
 	continue;
@@ -891,8 +891,8 @@ replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value)
 /* Replace propagated values into all the arguments for PHI using the
    values from PROP_VALUE.  */
 
-static bool
-replace_phi_args_in (gphi *phi, ssa_prop_get_value_fn get_value)
+bool
+substitute_and_fold_engine::replace_phi_args_in (gphi *phi)
 {
   size_t i;
   bool replaced = false;
@@ -909,7 +909,7 @@ replace_phi_args_in (gphi *phi, ssa_prop_get_value_fn get_value)
 
       if (TREE_CODE (arg) == SSA_NAME)
 	{
-	  tree val = (*get_value) (arg);
+	  tree val = get_value (arg);
 
 	  if (val && val != arg && may_propagate_copy (arg, val))
 	    {
@@ -960,10 +960,10 @@ class substitute_and_fold_dom_walker : public dom_walker
 {
 public:
     substitute_and_fold_dom_walker (cdi_direction direction,
-				    ssa_prop_get_value_fn get_value_fn_,
-				    ssa_prop_fold_stmt_fn fold_fn_)
-	: dom_walker (direction), get_value_fn (get_value_fn_),
-      fold_fn (fold_fn_), something_changed (false)
+				    class substitute_and_fold_engine *engine)
+	: dom_walker (direction),
+          something_changed (false),
+	  substitute_and_fold_engine (engine)
     {
       stmts_to_remove.create (0);
       stmts_to_fixup.create (0);
@@ -979,12 +979,12 @@ public:
     virtual edge before_dom_children (basic_block);
     virtual void after_dom_children (basic_block) {}
 
-    ssa_prop_get_value_fn get_value_fn;
-    ssa_prop_fold_stmt_fn fold_fn;
     bool something_changed;
     vec<gimple *> stmts_to_remove;
     vec<gimple *> stmts_to_fixup;
     bitmap need_eh_cleanup;
+
+    class substitute_and_fold_engine *substitute_and_fold_engine;
 };
 
 edge
@@ -1001,7 +1001,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
 	continue;
       if (res && TREE_CODE (res) == SSA_NAME)
 	{
-	  tree sprime = get_value_fn (res);
+	  tree sprime = substitute_and_fold_engine->get_value (res);
 	  if (sprime
 	      && sprime != res
 	      && may_propagate_copy (res, sprime))
@@ -1010,7 +1010,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
 	      continue;
 	    }
 	}
-      something_changed |= replace_phi_args_in (phi, get_value_fn);
+      something_changed |= substitute_and_fold_engine->replace_phi_args_in (phi);
     }
 
   /* Propagate known values into stmts.  In some case it exposes
@@ -1027,7 +1027,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
       tree lhs = gimple_get_lhs (stmt);
       if (lhs && TREE_CODE (lhs) == SSA_NAME)
 	{
-	  tree sprime = get_value_fn (lhs);
+	  tree sprime = substitute_and_fold_engine->get_value (lhs);
 	  if (sprime
 	      && sprime != lhs
 	      && may_propagate_copy (lhs, sprime)
@@ -1056,7 +1056,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
 			   && gimple_call_noreturn_p (stmt));
 
       /* Replace real uses in the statement.  */
-      did_replace |= replace_uses_in (stmt, get_value_fn);
+      did_replace |= substitute_and_fold_engine->replace_uses_in (stmt);
 
       /* If we made a replacement, fold the statement.  */
       if (did_replace)
@@ -1069,16 +1069,13 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
       /* Some statements may be simplified using propagator
 	 specific information.  Do this before propagating
 	 into the stmt to not disturb pass specific information.  */
-      if (fold_fn)
+      update_stmt_if_modified (stmt);
+      if (substitute_and_fold_engine->fold_stmt(&i))
 	{
-	  update_stmt_if_modified (stmt);
-	  if ((*fold_fn)(&i))
-	    {
-	      did_replace = true;
-	      prop_stats.num_stmts_folded++;
-	      stmt = gsi_stmt (i);
-	      gimple_set_modified (stmt, true);
-	    }
+	  did_replace = true;
+	  prop_stats.num_stmts_folded++;
+	  stmt = gsi_stmt (i);
+	  gimple_set_modified (stmt, true);
 	}
 
       /* If this is a control statement the propagator left edges
@@ -1164,19 +1161,15 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
    Return TRUE when something changed.  */
 
 bool
-substitute_and_fold (ssa_prop_get_value_fn get_value_fn,
-		     ssa_prop_fold_stmt_fn fold_fn)
+substitute_and_fold_engine::substitute_and_fold (void)
 {
-  gcc_assert (get_value_fn);
-
   if (dump_file && (dump_flags & TDF_DETAILS))
     fprintf (dump_file, "\nSubstituting values and folding statements\n\n");
 
   memset (&prop_stats, 0, sizeof (prop_stats));
 
   calculate_dominance_info (CDI_DOMINATORS);
-  substitute_and_fold_dom_walker walker(CDI_DOMINATORS,
-					get_value_fn, fold_fn);
+  substitute_and_fold_dom_walker walker (CDI_DOMINATORS, this);
   walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
 
   /* We cannot remove stmts during the BB walk, especially not release
diff --git a/gcc/tree-ssa-propagate.h b/gcc/tree-ssa-propagate.h
index cff9e53..629ae77 100644
--- a/gcc/tree-ssa-propagate.h
+++ b/gcc/tree-ssa-propagate.h
@@ -61,17 +61,11 @@ enum ssa_prop_result {
 };
 
 
-/* Call-back functions used by the value propagation engine.  */
-typedef bool (*ssa_prop_fold_stmt_fn) (gimple_stmt_iterator *gsi);
-typedef tree (*ssa_prop_get_value_fn) (tree);
-
-
 extern bool valid_gimple_rhs_p (tree);
 extern void move_ssa_defining_stmt_for_defs (gimple *, gimple *);
 extern bool update_gimple_call (gimple_stmt_iterator *, tree, int, ...);
 extern bool update_call_from_tree (gimple_stmt_iterator *, tree);
 extern bool stmt_makes_single_store (gimple *);
-extern bool substitute_and_fold (ssa_prop_get_value_fn, ssa_prop_fold_stmt_fn);
 extern bool may_propagate_copy (tree, tree);
 extern bool may_propagate_copy_into_stmt (gimple *, tree);
 extern bool may_propagate_copy_into_asm (tree);
@@ -79,7 +73,6 @@ extern void propagate_value (use_operand_p, tree);
 extern void replace_exp (use_operand_p, tree);
 extern void propagate_tree_value (tree *, tree);
 extern void propagate_tree_value_into_stmt (gimple_stmt_iterator *, tree);
-extern bool replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value);
 
 /* Public interface into the SSA propagation engine.  Clients should inherit
    from this class and provide their own visitors.  */
@@ -104,4 +97,14 @@ class ssa_propagation_engine
 
 };
 
+class substitute_and_fold_engine
+{
+ public:
+  bool substitute_and_fold (void);
+  bool replace_uses_in (gimple *);
+  virtual bool fold_stmt (gimple_stmt_iterator *) { return false; }
+  virtual tree get_value (tree) { return NULL_TREE; }
+  bool replace_phi_args_in (gphi *);
+};
+
 #endif /* _TREE_SSA_PROPAGATE_H  */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index e8fce3e..ea8ceee 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10536,10 +10536,17 @@ fold_predicate_in (gimple_stmt_iterator *si)
   return false;
 }
 
+class vrp_folder : public substitute_and_fold_engine
+{
+ public:
+  tree get_value (tree);
+  bool fold_stmt (gimple_stmt_iterator *);
+};
+
 /* Callback for substitute_and_fold folding the stmt at *SI.  */
 
-static bool
-vrp_fold_stmt (gimple_stmt_iterator *si)
+bool
+vrp_folder::fold_stmt (gimple_stmt_iterator *si)
 {
   if (fold_predicate_in (si))
     return true;
@@ -10547,6 +10554,18 @@ vrp_fold_stmt (gimple_stmt_iterator *si)
   return simplify_stmt_using_ranges (si);
 }
 
+/* If OP has a value range with a single constant value return that,
+   otherwise return NULL_TREE.  This returns OP itself if OP is a
+   constant.
+
+   Implemented as a pure wrapper right now, but this will change.  */
+
+tree
+vrp_folder::get_value (tree op)
+{
+  return op_with_constant_singleton_value_range (op);
+}
+
 /* Return the LHS of any ASSERT_EXPR where OP appears as the first
    argument to the ASSERT_EXPR and in which the ASSERT_EXPR dominates
    BB.  If no such ASSERT_EXPR is found, return OP.  */
@@ -10888,7 +10907,8 @@ vrp_finalize (bool warn_array_bounds_p)
 			  wi::to_wide (vr_value[i]->max));
       }
 
-  substitute_and_fold (op_with_constant_singleton_value_range, vrp_fold_stmt);
+  class vrp_folder vrp_folder;
+  vrp_folder.substitute_and_fold ();
 
   if (warn_array_bounds && warn_array_bounds_p)
     check_all_array_refs ();
@@ -11225,8 +11245,8 @@ evrp_dom_walker::before_dom_children (basic_block bb)
 	}
 
       /* Try folding stmts with the VR discovered.  */
-      bool did_replace
-	= replace_uses_in (stmt, op_with_constant_singleton_value_range);
+      class vrp_folder vrp_folder;
+      bool did_replace = vrp_folder.replace_uses_in (stmt);
       if (fold_stmt (&gsi, follow_single_use_edges)
 	  || did_replace)
 	{


More information about the Gcc-patches mailing list