Re: [RFA][PATCH] Provide a class interface to ssa_propagate

[Jeff Law <law at redhat dot com>]

On 10/24/2017 02:49 PM, David Malcolm wrote:
> On Tue, 2017-10-24 at 11:40 -0600, Jeff Law wrote:
>> tree-ssa-propagate.c provides a fairly generic engine to propagate
>> values through a lattice while in SSA form.  The engine uses two
>> callbacks to allow passes to provide pass specific handling of
>> statements and phi nodes.
>>
>> The callback mechanism served us well in a C world.  It is however
>> somewhat painful to have state in those callbacks without resorting
>> to
>> global variables or passing around void * objects which contain the
>> class instance pointer.
>>
>> For example, tree-vrp uses the propagation engine to compute global
>> range information.  Its callbacks vrp_visit_stmt and vrp_visit_phi
>> and
>> their children read/modify a variety of tree-vrp.c statics such as
>> vr_data.
>>
>> In some changes I'm working on I'd really like to move vr_data into a
>> distinct class and avoid having direct accesses to the underlying
>> array.
>>
>> So the problem is how are routines like vrp_visit_stmt and
>> vrp_visit_phi
>> and their children supposed to access the class instance?
>>
>> One way would be to just add a void * argument to them and pass the
>> class instance around.  Alternately we could leave the global
>> variable
>> in place and have it set up, checked and wiped clean by the vr_data
>> class's ctor/dtor.  Both are valid and would work, but they're a bit
>> ugly IMHO.
>>
>> This patch takes another approach.  It builds a simple little class
>> around ssa_propagate where the statement and phi visitors are virtual
>> functions.  Thus clients can override the visitors *and* they'll get
>> a
>> class instance pointer.
>>
>> I haven't gone hog wild with C++-ification, basically just enough to
>> get
>> the class around ssa_propagate and its children which are going to
>> need
>> to pass down the class instance to the virtual functions.  There's a
>> lot
>> more that could be done here.
>>
>> As you can see the client side changes are pretty minimal.  They just
>> derive a new class from ssa_propagation_engine to provide their
>> implementations of the statement and phi visitor.  More importantly,
>> they can hang data off that derived class which we'll exploit later.
>>
>> There will be a similar patch for the substitute_and_fold which has
>> callbacks of its own.
>>
>> Bootstrapped and regression tested on x86_64.
>>
>> The ChangeLog makes the patch look huge.  But it's actually
>> relatively
>> small and the client side bits are repetitive.
>>
>> OK for the trunk?
>>
>> Jeff
> 
> Looks like you forgot to attach the patch.
Ugh.  Attached, with the FINAL OVERRIDE addition.


Jeff

	* tree-ssa-propagate.h (ssa_prop_visit_stmt_fn): Remove typedef.
	(ssa_prop_visit_phi_fn): Likewise.
	(class ssa_propagation_engine): New class to provide an interface
	into ssa_propagate.
	* tree-ssa-propagate.c (ssa_prop_visit_stmt): Remove file scoped
	variable.
	(ssa_prop_visit_phi): Likewise.
	(ssa_propagation_engine::simulate_stmt): Moved into class.
	Call visit_phi/visit_stmt from the class rather than via
	file scoped static variables.
	(ssa_propagation_engine::simulate_block): Moved into class.
	(ssa_propagation_engine::process_ssa_edge_worklist): Similarly.
	(ssa_propagation_engine::ssa_propagate): Similarly.  No longer
	set file scoped statics for the visit_stmt/visit_phi callbacks.
	* tree-complex.c (complex_propagate): New class derived from
	ssa_propagation_engine.
	(complex_propagate::visit_stmt): Renamed from complex_visit_stmt.
	(complex_propagate::visit_phi): Renamed from complex_visit_phi.
	(tree_lower_complex): Call ssa_propagate via the complex_propagate
	class.
	* tree-ssa-ccp.c: (ccp_propagate): New class derived from
	ssa_propagation_engine.
	(ccp_propagate::visit_phi): Renamed from ccp_visit_phi_node.
	(ccp_propagate::visit_stmt): Renamed from ccp_visit_stmt.
	(do_ssa_ccp): Call ssa_propagate from the ccp_propagate class.
	* tree-ssa-copy.c (copy_prop): New class derived from
	ssa_propagation_engine.
	(copy_prop::visit_stmt): Renamed from copy_prop_visit_stmt.
	(copy_prop::visit_phi): Renamed from copy_prop_visit_phi_node.
	(execute_copy_prop): Call ssa_propagate from the copy_prop class.
	* tree-vrp.c (vrp_prop): New class derived from ssa_propagation_engine.
	(vrp_prop::visit_stmt): Renamed from vrp_visit_stmt.
	(vrp_prop::visit_phi): Renamed from vrp_visit_phi_node.
	(execute_vrp): Call ssa_propagate from the vrp_prop class.



diff --git a/gcc/tree-complex.c b/gcc/tree-complex.c
index e2d93b7..e9e7e2a 100644
--- a/gcc/tree-complex.c
+++ b/gcc/tree-complex.c
@@ -60,6 +60,11 @@ typedef int complex_lattice_t;
 
 #define PAIR(a, b)  ((a) << 2 | (b))
 
+class complex_propagate : public ssa_propagation_engine
+{
+  enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
+  enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
+};
 
 static vec<complex_lattice_t> complex_lattice_values;
 
@@ -300,9 +305,9 @@ init_dont_simulate_again (void)
 
 /* Evaluate statement STMT against the complex lattice defined above.  */
 
-static enum ssa_prop_result
-complex_visit_stmt (gimple *stmt, edge *taken_edge_p ATTRIBUTE_UNUSED,
-		    tree *result_p)
+enum ssa_prop_result
+complex_propagate::visit_stmt (gimple *stmt, edge *taken_edge_p ATTRIBUTE_UNUSED,
+			       tree *result_p)
 {
   complex_lattice_t new_l, old_l, op1_l, op2_l;
   unsigned int ver;
@@ -395,8 +400,8 @@ complex_visit_stmt (gimple *stmt, edge *taken_edge_p ATTRIBUTE_UNUSED,
 
 /* Evaluate a PHI node against the complex lattice defined above.  */
 
-static enum ssa_prop_result
-complex_visit_phi (gphi *phi)
+enum ssa_prop_result
+complex_propagate::visit_phi (gphi *phi)
 {
   complex_lattice_t new_l, old_l;
   unsigned int ver;
@@ -1673,7 +1678,8 @@ tree_lower_complex (void)
   complex_lattice_values.safe_grow_cleared (num_ssa_names);
 
   init_parameter_lattice_values ();
-  ssa_propagate (complex_visit_stmt, complex_visit_phi);
+  class complex_propagate complex_propagate;
+  complex_propagate.ssa_propagate ();
 
   complex_variable_components = new int_tree_htab_type (10);
 
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 569b057..d4b3623 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -171,6 +171,13 @@ struct ccp_prop_value_t {
     widest_int mask;
 };
 
+class ccp_propagate : public ssa_propagation_engine
+{
+ public:
+  enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
+  enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
+};
+
 /* Array of propagated constant values.  After propagation,
    CONST_VAL[I].VALUE holds the constant value for SSA_NAME(I).  If
    the constant is held in an SSA name representing a memory store
@@ -1064,8 +1071,8 @@ ccp_lattice_meet (ccp_prop_value_t *val1, ccp_prop_value_t *val2)
    PHI node is determined calling ccp_lattice_meet with all the arguments
    of the PHI node that are incoming via executable edges.  */
 
-static enum ssa_prop_result
-ccp_visit_phi_node (gphi *phi)
+enum ssa_prop_result
+ccp_propagate::visit_phi (gphi *phi)
 {
   unsigned i;
   ccp_prop_value_t new_val;
@@ -2378,8 +2385,8 @@ visit_cond_stmt (gimple *stmt, edge *taken_edge_p)
    value, set *TAKEN_EDGE_P accordingly.  If STMT produces a varying
    value, return SSA_PROP_VARYING.  */
 
-static enum ssa_prop_result
-ccp_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
+enum ssa_prop_result
+ccp_propagate::visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
 {
   tree def;
   ssa_op_iter iter;
@@ -2441,7 +2448,8 @@ do_ssa_ccp (bool nonzero_p)
   calculate_dominance_info (CDI_DOMINATORS);
 
   ccp_initialize ();
-  ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node);
+  class ccp_propagate ccp_propagate;
+  ccp_propagate.ssa_propagate ();
   if (ccp_finalize (nonzero_p || flag_ipa_bit_cp))
     {
       todo = (TODO_cleanup_cfg | TODO_update_ssa);
diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index 9f0fe54..9db11e2 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -68,6 +68,13 @@ struct prop_value_t {
     tree value;
 };
 
+class copy_prop : public ssa_propagation_engine
+{
+ public:
+  enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
+  enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
+};
+
 static prop_value_t *copy_of;
 static unsigned n_copy_of;
 
@@ -263,8 +270,8 @@ copy_prop_visit_cond_stmt (gimple *stmt, edge *taken_edge_p)
    If the new value produced by STMT is varying, return
    SSA_PROP_VARYING.  */
 
-static enum ssa_prop_result
-copy_prop_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *result_p)
+enum ssa_prop_result
+copy_prop::visit_stmt (gimple *stmt, edge *taken_edge_p, tree *result_p)
 {
   enum ssa_prop_result retval;
 
@@ -317,8 +324,8 @@ copy_prop_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *result_p)
 /* Visit PHI node PHI.  If all the arguments produce the same value,
    set it to be the value of the LHS of PHI.  */
 
-static enum ssa_prop_result
-copy_prop_visit_phi_node (gphi *phi)
+enum ssa_prop_result
+copy_prop::visit_phi (gphi *phi)
 {
   enum ssa_prop_result retval;
   unsigned i;
@@ -601,7 +608,8 @@ static unsigned int
 execute_copy_prop (void)
 {
   init_copy_prop ();
-  ssa_propagate (copy_prop_visit_stmt, copy_prop_visit_phi_node);
+  class copy_prop copy_prop;
+  copy_prop.ssa_propagate ();
   if (fini_copy_prop ())
     return TODO_cleanup_cfg;
   return 0;
diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 00ab3d7..90df285 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -108,10 +108,6 @@
      [3] Advanced Compiler Design and Implementation,
 	 Steven Muchnick, Morgan Kaufmann, 1997, Section 12.6  */
 
-/* Function pointers used to parameterize the propagation engine.  */
-static ssa_prop_visit_stmt_fn ssa_prop_visit_stmt;
-static ssa_prop_visit_phi_fn ssa_prop_visit_phi;
-
 /* Worklist of control flow edge destinations.  This contains
    the CFG order number of the blocks so we can iterate in CFG
    order by visiting in bit-order.  */
@@ -217,8 +213,8 @@ add_control_edge (edge e)
 
 /* Simulate the execution of STMT and update the work lists accordingly.  */
 
-static void
-simulate_stmt (gimple *stmt)
+void
+ssa_propagation_engine::simulate_stmt (gimple *stmt)
 {
   enum ssa_prop_result val = SSA_PROP_NOT_INTERESTING;
   edge taken_edge = NULL;
@@ -234,11 +230,11 @@ simulate_stmt (gimple *stmt)
 
   if (gimple_code (stmt) == GIMPLE_PHI)
     {
-      val = ssa_prop_visit_phi (as_a <gphi *> (stmt));
+      val = visit_phi (as_a <gphi *> (stmt));
       output_name = gimple_phi_result (stmt);
     }
   else
-    val = ssa_prop_visit_stmt (stmt, &taken_edge, &output_name);
+    val = visit_stmt (stmt, &taken_edge, &output_name);
 
   if (val == SSA_PROP_VARYING)
     {
@@ -321,8 +317,8 @@ simulate_stmt (gimple *stmt)
    when an SSA edge is added to it in simulate_stmt.  Return true if a stmt
    was simulated.  */
 
-static void
-process_ssa_edge_worklist ()
+void
+ssa_propagation_engine::process_ssa_edge_worklist (void)
 {
   /* Process the next entry from the worklist.  */
   unsigned stmt_uid = bitmap_first_set_bit (ssa_edge_worklist);
@@ -345,8 +341,8 @@ process_ssa_edge_worklist ()
 /* Simulate the execution of BLOCK.  Evaluate the statement associated
    with each variable reference inside the block.  */
 
-static void
-simulate_block (basic_block block)
+void
+ssa_propagation_engine::simulate_block (basic_block block)
 {
   gimple_stmt_iterator gsi;
 
@@ -781,19 +777,15 @@ update_call_from_tree (gimple_stmt_iterator *si_p, tree expr)
     return false;
 }
 
-
 /* Entry point to the propagation engine.
 
-   VISIT_STMT is called for every statement visited.
-   VISIT_PHI is called for every PHI node visited.  */
+   The VISIT_STMT virtual function is called for every statement
+   visited and the VISIT_PHI virtual function is called for every PHI
+   node visited.  */
 
 void
-ssa_propagate (ssa_prop_visit_stmt_fn visit_stmt,
-	       ssa_prop_visit_phi_fn visit_phi)
+ssa_propagation_engine::ssa_propagate (void)
 {
-  ssa_prop_visit_stmt = visit_stmt;
-  ssa_prop_visit_phi = visit_phi;
-
   ssa_prop_init ();
 
   /* Iterate until the worklists are empty.  */
diff --git a/gcc/tree-ssa-propagate.h b/gcc/tree-ssa-propagate.h
index 9a8ecc8..cff9e53 100644
--- a/gcc/tree-ssa-propagate.h
+++ b/gcc/tree-ssa-propagate.h
@@ -62,9 +62,6 @@ enum ssa_prop_result {
 
 
 /* Call-back functions used by the value propagation engine.  */
-typedef enum ssa_prop_result (*ssa_prop_visit_stmt_fn) (gimple *, edge *,
-							tree *);
-typedef enum ssa_prop_result (*ssa_prop_visit_phi_fn) (gphi *);
 typedef bool (*ssa_prop_fold_stmt_fn) (gimple_stmt_iterator *gsi);
 typedef tree (*ssa_prop_get_value_fn) (tree);
 
@@ -73,7 +70,6 @@ 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 void ssa_propagate (ssa_prop_visit_stmt_fn, ssa_prop_visit_phi_fn);
 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);
@@ -85,4 +81,27 @@ 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.  */
+
+class ssa_propagation_engine
+{
+ public:
+
+  /* Main interface into the propagation engine.  */
+  void ssa_propagate (void);
+
+  /* Virtual functions the clients must provide to visit statements
+     and phi nodes respectively.  */
+  virtual enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) = 0;
+  virtual enum ssa_prop_result visit_phi (gphi *) = 0;
+
+ private:
+  /* Internal implementation details.  */
+  void simulate_stmt (gimple *stmt);
+  void process_ssa_edge_worklist (void);
+  void simulate_block (basic_block);
+
+};
+
 #endif /* _TREE_SSA_PROPAGATE_H  */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 2c86b8e..85deecc 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -8092,6 +8092,13 @@ extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
     vrp_visit_switch_stmt (as_a <gswitch *> (stmt), taken_edge_p);
 }
 
+class vrp_prop : public ssa_propagation_engine
+{
+ public:
+  enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
+  enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
+};
+
 /* Evaluate statement STMT.  If the statement produces a useful range,
    return SSA_PROP_INTERESTING and record the SSA name with the
    interesting range into *OUTPUT_P.
@@ -8101,8 +8108,8 @@ extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
 
    If STMT produces a varying value, return SSA_PROP_VARYING.  */
 
-static enum ssa_prop_result
-vrp_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
+enum ssa_prop_result
+vrp_prop::visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
 {
   value_range vr = VR_INITIALIZER;
   tree lhs = gimple_get_lhs (stmt);
@@ -9193,8 +9200,8 @@ update_range:
    edges.  If a valid value range can be derived from all the incoming
    value ranges, set a new range for the LHS of PHI.  */
 
-static enum ssa_prop_result
-vrp_visit_phi_node (gphi *phi)
+enum ssa_prop_result
+vrp_prop::visit_phi (gphi *phi)
 {
   tree lhs = PHI_RESULT (phi);
   value_range vr_result = VR_INITIALIZER;
@@ -11469,7 +11476,8 @@ execute_vrp (bool warn_array_bounds_p)
 
   vrp_initialize_lattice ();
   vrp_initialize ();
-  ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node);
+  class vrp_prop vrp_prop;
+  vrp_prop.ssa_propagate ();
   vrp_finalize (warn_array_bounds_p);
 
   /* We must identify jump threading opportunities before we release