* Re: [RFA][PATCH] Provide a class interface to ssa_propagate
2017-10-24 20:57 ` David Malcolm
@ 2017-10-25 17:20 ` Jeff Law
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Law @ 2017-10-25 17:20 UTC (permalink / raw)
To: David Malcolm, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]
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
[-- Attachment #2: P1 --]
[-- Type: text/plain, Size: 13359 bytes --]
* 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
^ permalink raw reply [flat|nested] 3+ messages in thread