* [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
@ 2017-11-07 22:27 Jeff Law
2017-11-08 13:14 ` Trevor Saunders
2017-11-09 18:41 ` Martin Sebor
0 siblings, 2 replies; 6+ messages in thread
From: Jeff Law @ 2017-11-07 22:27 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]
So this is the first step in pulling apart tree-vrp.c. As outlined in
an earlier message the goal of this patch is just to get the vr_values
class introduced. I've tried (and mostly succeeded) in avoiding doing
significant cleanups/rewrites at this point to make reviewing this step
easier.
The vast majority of this patch is just changing functions from free
functions to member functions in the appropriate class (primarily
vr_values), pulling the appropriate static objects into that class and
adding the temporary delegators to minimize diffs at this stage.
Exceptions to this:
set_value_range changes how we get at the obstack for allocating
bitmaps. This was discussed on-list recently. Similarly in
vrp_intersect_ranges_1.
extract_range_from_unary_expr has two implementations. One is within
the vr_values class which calls out to the freestanding function. Hence
the ::extract_range_from_unary_expr.
We drop debug_all_value_ranges. It doesn't make sense in a world where
the value ranges are attached to a class instance. There is still a
method to dump all the value ranges within the class instance.
The vrp_prop and vrp_folder class definitions have to move to earlier
points in the file.
We pass a vrp_prop class instance through the gimple walkers using the
wi->info field. check_all_array_refs sets that up and we recover the
class instance pointer in the check_array_bounds callback.
I wasn't up to converting the gimple folder to classes at this time. So
we set/wipe x_vr_values (file scoped static) around the calls into the
gimple walkers and recover the vr_values class instance from x_vr_values
within vrp_valueize and vrp_valueize_1.
I use a similar hack to recover the vr_values instance in the callback
to simplify a statement for jump threading. This code is on the
chopping block -- I expect it all to just disappear shortly. Similarly
I just pass in vr_values to identify_jump_threads. Again, I expect to
be removing all this code shortly and wanted to keep this as simple as
possible rather than waste time cleaning up code I'm just going to remove.
I did do some simplifications in vrp_finalize. It wanted to access
vr_value and num_vr_values directly. Now it goes through the
get_value_range interface.
I introduced a set_value_range_ member function in vr_values, then
twiddled a couple evrp routines to use that rather than access vr_value
directly.
In a few places I didn't bother creating delegators. The delegators
wouldn't have simplified anything. See execute_vrp and execute_early_vrp.
You'll note the vr_values class is large. I've tried to avoid the
temptation to start simplifying stuff and instead try to stick with
moving routines into the appropriate class based on the code as it
stands right now. The size of vr_values is probably a good indicator
that it's doing too much.
As suggested yesterday, I've added various delegator methods to the
classes to minimize code churn at this time. I suspect we're going to
want to remove most, if not all of them. But again, the goal in this
patch is to get the class structure in place with minimal amount of
collateral damage to make this step easy to review.
It's worth noting you see no changes that bleed out of tree-vrp.c and
the vast majority of the patch is just changing the function signature
so that they're part of the newly created class. I consider that a
positive :-)
Bootstrapped and regression tested on x86_64.
OK for the trunk?
Jeff
ps. Next step is to pull the class methods into their own files without
changing *any* parts of their implementation. As a bit of a preview,
tree-vrp.c is currently around 12000 lines. After moving methods it'll
be about 7k. vr-values.c will have around 5k lines. The split out EVRP
pass is ~600 which will be further split roughly in half between the
core of evrp and the reusable range analysis module.
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 49122 bytes --]
* vr-values.h: New file with vr_values class.
* tree-vrp.c: Include vr-values.h
(vrp_value_range_pool, vrp_equiv_obstack, num_vr_values): Move static
data objects into the vr_values class.
(vr_value, values_propagated, vr_phi_edge_counts): Likewise.
(set_value_range): Do not reference vrp_equiv_obstack. Get it
from the existing bitmap instead.
(vrp_intersect_ranges_1): Likewise.
(get_value_range): Make it a member function within vr_values class.
(set_defs_to_varying, update_value_range, add_equivalence): Likewise.
(vrp_stmt_computes_nonzero_p, op_with_boolean_value_range_p): Likewise.
(op_with_constant_singleton_value_range): Likewise.
(extract_range_for_var_from_comparison_expr): Likewise.
(extract_range_from_assert, extract_range_from_ssa_name): Likewise.
(extract_range_from_binary_expr): Likewise.
(extract_range_from_unary_expr): Likewise.
(extract_range_from_cond_expr, extrat_range_from_comparison): Likewise.
(check_for_binary_op_overflow, extract_range_basic): Likewise.
(extract_range_from_assignment, adjust_range_with_scev): Likewise.
(dump_all_value_ranges, get_vr_for_comparison): Likewise.
(compare_name_with_value, compare_names): Likewise.
(vrp_evaluate_conditional_warnv_with_ops_using_ranges): Likewise.
(vrp_evaluate_conditional_warnv_with_ops): Likewise. Remove prototype.
(vrp_evaluate_conditional, vrp_visit_cond_stmt): Likewise.
(vrp_visit_switch_stmt, extract_range_from_stmt): Likewise.
(extract_range_from_phi_node): Likewise.
(simplify_truth_ops_using_ranges): Likewise.
(simplify_div_or_mod_using_ranges): Likewise.
(simplify_min_or_max_using_ranges, simplify_abs_using_ranges): Likewise.
(simplify_bit_ops_using_ranges, simplify_cond_using_ranges_1): Likewise.
(simplify_cond_using_ranges_2, simplify_switch_using_ranges): Likewise.
(simplify_float_conversion_using_ranges): Likewise.
(simplify_internal_call_using_ranges): Likewise.
(two_valued_val_range_p, simplify_stmt_using_ranges): Likewise.
(vrp_visit_assignment_or_call): Likewise. Smuggle class instance
poitner via x_vr_values for calls into gimple folder.
(vrp_initialize_lattice): Make this the vr_values ctor.
(vrp_free_lattice): Make this the vr_values dtor.
(set_value_range_): New function.
(class vrp_prop): Move to earlier point in file. Add vr_values
data member. Add various member functions as well as member
functions that delegate to vr_values.
(check_array_ref): Make a member function within vrp_prop class.
(search_for_addr_array, vrp_initialize): Likewise.
(vrp_finalize): Likewise. Revamp to avoid direct access to
vr_value, values_propagated, etc.
(check_array_bounds): Extract vrp_prop class instance pointer from
walk info structure. Use it to call member functions.
(check_all_array_refs): Make a member function within vrp_prop class.
Smuggle class instance pointer via walk info structure.
(x_vr_values): New local static.
(vrp_valueize): Use x_vr_values to get class instance.
(vr_valueize_1): Likewise.
(class vrp_folder): Move to earlier point in file. Add vr_values
data member. Add various member functions as well as member
functions that delegate to vr_values.
(fold_predicate_in): Make a mber fucntion within vrp_folder class.
(simplify_stmt_for_jump_threading): Extract smuggled vr_values
class instance from vr_values. Use it to call member functions.
(vrp_dom_walker): Add vr_values data member.
(vrp_dom_walker::after_dom_children): Smuggle vr_values class
instance via x_vr_values.
(identify_jump_threads): Accept vr_values as argument. Store
it into the walker structure.
(evrp_dom_walker): Add vr_values class data member. Add various
delegators.
(evrp_dom_walker::try_find_new_range): Use vr_values data
member to access the memory allocator.
(evrp_dom_walker::before_dom_children): Store vr_values class
instance into the vrp_folder class.
(evrp_dom_walker::push_value_range): Rework to avoid direct
access to num_vr_values and vr_value.
(evrp_dom_walker::pop_value_range): Likewise.
(execute_early_vrp): Remove call to vrp_initialize_lattice.
Use vr_values to get to dump_all_value_ranges member function.
Remove call to vrp_free_lattice. Call vrp_initialize, vrp_finalize,
and simplify_cond_using_ranges_2 via vrp_prop class instance.
Pass vr_values class instance down to identify_jump_threads.
Remove call to vrp_free_lattice.
(debug_all_value_ranges): Remove.
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 2b7d962..a7d05eb 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -64,13 +64,10 @@ along with GCC; see the file COPYING3. If not see
#include "tree-cfgcleanup.h"
#include "stringpool.h"
#include "attribs.h"
+#include "vr-values.h"
#define VR_INITIALIZER { VR_UNDEFINED, NULL_TREE, NULL_TREE, NULL }
-/* Allocation pools for tree-vrp allocations. */
-static object_allocator<value_range> vrp_value_range_pool ("Tree VRP value ranges");
-static bitmap_obstack vrp_equiv_obstack;
-
/* Set of SSA names found live during the RPO traversal of the function
for still active basic-blocks. */
static sbitmap *live;
@@ -87,9 +84,6 @@ live_on_edge (edge e, tree name)
/* Local functions. */
static int compare_values (tree val1, tree val2);
static int compare_values_warnv (tree val1, tree val2, bool *);
-static tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
- tree, tree, bool, bool *,
- bool *);
struct assert_info
{
@@ -145,17 +139,6 @@ static bitmap need_assert_for;
ASSERT_EXPRs for SSA name N_I should be inserted. */
static assert_locus **asserts_for;
-/* Value range array. After propagation, VR_VALUE[I] holds the range
- of values that SSA name N_I may take. */
-static unsigned num_vr_values;
-static value_range **vr_value;
-static bool values_propagated;
-
-/* For a PHI node which sets SSA name N_I, VR_COUNTS[I] holds the
- number of executable edges we saw the last time we visited the
- node. */
-static int *vr_phi_edge_counts;
-
struct switch_update {
gswitch *stmt;
tree vec;
@@ -272,10 +255,13 @@ set_value_range (value_range *vr, enum value_range_type t, tree min,
vr->max = max;
/* Since updating the equivalence set involves deep copying the
- bitmaps, only do it if absolutely necessary. */
+ bitmaps, only do it if absolutely necessary.
+
+ All equivalence bitmaps are allocated from the same obstack. So
+ we can use the obstack associated with EQUIV to allocate vr->equiv. */
if (vr->equiv == NULL
&& equiv != NULL)
- vr->equiv = BITMAP_ALLOC (&vrp_equiv_obstack);
+ vr->equiv = BITMAP_ALLOC (equiv->obstack);
if (equiv != vr->equiv)
{
@@ -506,8 +492,8 @@ abs_extent_range (value_range *vr, tree min, tree max)
If we have no values ranges recorded (ie, VRP is not running), then
return NULL. Otherwise create an empty range if none existed for VAR. */
-static value_range *
-get_value_range (const_tree var)
+value_range *
+vr_values::get_value_range (const_tree var)
{
static const value_range vr_const_varying
= { VR_VARYING, NULL_TREE, NULL_TREE, NULL };
@@ -579,8 +565,8 @@ get_value_range (const_tree var)
/* Set value-ranges of all SSA names defined by STMT to varying. */
-static void
-set_defs_to_varying (gimple *stmt)
+void
+vr_values::set_defs_to_varying (gimple *stmt)
{
ssa_op_iter i;
tree def;
@@ -628,8 +614,8 @@ vrp_bitmap_equal_p (const_bitmap b1, const_bitmap b2)
this function. Do not call update_value_range when NEW_VR
is the range object associated with another SSA name. */
-static inline bool
-update_value_range (const_tree var, value_range *new_vr)
+bool
+vr_values::update_value_range (const_tree var, value_range *new_vr)
{
value_range *old_vr;
bool is_new;
@@ -687,8 +673,8 @@ update_value_range (const_tree var, value_range *new_vr)
/* Add VAR and VAR's equivalence set to EQUIV. This is the central
point where equivalence processing can be turned on/off. */
-static void
-add_equivalence (bitmap *equiv, const_tree var)
+void
+vr_values::add_equivalence (bitmap *equiv, const_tree var)
{
unsigned ver = SSA_NAME_VERSION (var);
value_range *vr = get_value_range (var);
@@ -930,8 +916,8 @@ gimple_stmt_nonzero_p (gimple *stmt)
/* Like tree_expr_nonzero_p, but this function uses value ranges
obtained so far. */
-static bool
-vrp_stmt_computes_nonzero (gimple *stmt)
+bool
+vr_values::vrp_stmt_computes_nonzero (gimple *stmt)
{
if (gimple_stmt_nonzero_p (stmt))
return true;
@@ -1257,8 +1243,8 @@ value_range_constant_singleton (value_range *vr)
otherwise return NULL_TREE. This returns OP itself if OP is a
constant. */
-static tree
-op_with_constant_singleton_value_range (tree op)
+tree
+vr_values::op_with_constant_singleton_value_range (tree op)
{
if (is_gimple_min_invariant (op))
return op;
@@ -1271,8 +1257,8 @@ op_with_constant_singleton_value_range (tree op)
/* Return true if op is in a boolean [0, 1] value-range. */
-static bool
-op_with_boolean_value_range_p (tree op)
+bool
+vr_values::op_with_boolean_value_range_p (tree op)
{
value_range *vr;
@@ -1295,10 +1281,11 @@ op_with_boolean_value_range_p (tree op)
/* Extract value range information for VAR when (OP COND_CODE LIMIT) is
true and store it in *VR_P. */
-static void
-extract_range_for_var_from_comparison_expr (tree var, enum tree_code cond_code,
- tree op, tree limit,
- value_range *vr_p)
+void
+vr_values::extract_range_for_var_from_comparison_expr (tree var,
+ enum tree_code cond_code,
+ tree op, tree limit,
+ value_range *vr_p)
{
tree min, max, type;
value_range *limit_vr;
@@ -1542,8 +1529,8 @@ extract_range_for_var_from_comparison_expr (tree var, enum tree_code cond_code,
/* Extract value range information from an ASSERT_EXPR EXPR and store
it in *VR_P. */
-static void
-extract_range_from_assert (value_range *vr_p, tree expr)
+void
+vr_values::extract_range_from_assert (value_range *vr_p, tree expr)
{
tree var = ASSERT_EXPR_VAR (expr);
tree cond = ASSERT_EXPR_COND (expr);
@@ -1588,8 +1575,8 @@ extract_range_from_assert (value_range *vr_p, tree expr)
Even if y_5 is deemed VARYING, we can determine that x_3 > y_5 is
always false. */
-static void
-extract_range_from_ssa_name (value_range *vr, tree var)
+void
+vr_values::extract_range_from_ssa_name (value_range *vr, tree var)
{
value_range *var_vr = get_value_range (var);
@@ -3024,10 +3011,10 @@ extract_range_from_binary_expr_1 (value_range *vr,
the ranges of each of its operands with resulting type EXPR_TYPE.
The resulting range is stored in *VR. */
-static void
-extract_range_from_binary_expr (value_range *vr,
- enum tree_code code,
- tree expr_type, tree op0, tree op1)
+void
+vr_values::extract_range_from_binary_expr (value_range *vr,
+ enum tree_code code,
+ tree expr_type, tree op0, tree op1)
{
value_range vr0 = VR_INITIALIZER;
value_range vr1 = VR_INITIALIZER;
@@ -3371,9 +3358,9 @@ extract_range_from_unary_expr (value_range *vr,
the range of its operand with resulting type TYPE.
The resulting range is stored in *VR. */
-static void
-extract_range_from_unary_expr (value_range *vr, enum tree_code code,
- tree type, tree op0)
+void
+vr_values::extract_range_from_unary_expr (value_range *vr, enum tree_code code,
+ tree type, tree op0)
{
value_range vr0 = VR_INITIALIZER;
@@ -3386,15 +3373,15 @@ extract_range_from_unary_expr (value_range *vr, enum tree_code code,
else
set_value_range_to_varying (&vr0);
- extract_range_from_unary_expr (vr, code, type, &vr0, TREE_TYPE (op0));
+ ::extract_range_from_unary_expr (vr, code, type, &vr0, TREE_TYPE (op0));
}
/* Extract range information from a conditional expression STMT based on
the ranges of each of its operands and the expression code. */
-static void
-extract_range_from_cond_expr (value_range *vr, gassign *stmt)
+void
+vr_values::extract_range_from_cond_expr (value_range *vr, gassign *stmt)
{
tree op0, op1;
value_range vr0 = VR_INITIALIZER;
@@ -3427,9 +3414,9 @@ extract_range_from_cond_expr (value_range *vr, gassign *stmt)
/* Extract range information from a comparison expression EXPR based
on the range of its operand and the expression code. */
-static void
-extract_range_from_comparison (value_range *vr, enum tree_code code,
- tree type, tree op0, tree op1)
+void
+vr_values::extract_range_from_comparison (value_range *vr, enum tree_code code,
+ tree type, tree op0, tree op1)
{
bool sop;
tree val;
@@ -3458,9 +3445,9 @@ extract_range_from_comparison (value_range *vr, enum tree_code code,
always overflow. Set *OVF to true if it is known to always
overflow. */
-static bool
-check_for_binary_op_overflow (enum tree_code subcode, tree type,
- tree op0, tree op1, bool *ovf)
+bool
+vr_values::check_for_binary_op_overflow (enum tree_code subcode, tree type,
+ tree op0, tree op1, bool *ovf)
{
value_range vr0 = VR_INITIALIZER;
value_range vr1 = VR_INITIALIZER;
@@ -3563,8 +3550,8 @@ check_for_binary_op_overflow (enum tree_code subcode, tree type,
primarily on generic routines in fold in conjunction with range data.
Store the result in *VR */
-static void
-extract_range_basic (value_range *vr, gimple *stmt)
+void
+vr_values::extract_range_basic (value_range *vr, gimple *stmt)
{
bool sop;
tree type = gimple_expr_type (stmt);
@@ -3914,8 +3901,8 @@ extract_range_basic (value_range *vr, gimple *stmt)
/* Try to compute a useful range out of assignment STMT and store it
in *VR. */
-static void
-extract_range_from_assignment (value_range *vr, gassign *stmt)
+void
+vr_values::extract_range_from_assignment (value_range *vr, gassign *stmt)
{
enum tree_code code = gimple_assign_rhs_code (stmt);
@@ -3953,9 +3940,9 @@ extract_range_from_assignment (value_range *vr, gassign *stmt)
would be profitable to adjust VR using scalar evolution information
for VAR. If so, update VR with the new limits. */
-static void
-adjust_range_with_scev (value_range *vr, struct loop *loop,
- gimple *stmt, tree var)
+void
+vr_values::adjust_range_with_scev (value_range *vr, struct loop *loop,
+ gimple *stmt, tree var)
{
tree init, step, chrec, tmin, tmax, min, max, type, tem;
enum ev_direction dir;
@@ -4399,7 +4386,6 @@ compare_range_with_value (enum tree_code comp, value_range *vr, tree val,
void dump_value_range (FILE *, const value_range *);
void debug_value_range (value_range *);
void dump_all_value_ranges (FILE *);
-void debug_all_value_ranges (void);
void dump_vr_equiv (FILE *, bitmap);
void debug_vr_equiv (bitmap);
@@ -4473,7 +4459,7 @@ debug_value_range (value_range *vr)
/* Dump value ranges of all SSA_NAMEs to FILE. */
void
-dump_all_value_ranges (FILE *file)
+vr_values::dump_all_value_ranges (FILE *file)
{
size_t i;
@@ -4491,16 +4477,6 @@ dump_all_value_ranges (FILE *file)
fprintf (file, "\n");
}
-
-/* Dump all value ranges to stderr. */
-
-DEBUG_FUNCTION void
-debug_all_value_ranges (void)
-{
- dump_all_value_ranges (stderr);
-}
-
-
/* Given a COND_EXPR COND of the form 'V OP W', and an SSA name V,
create a new SSA name N and return the assertion assignment
'N = ASSERT_EXPR <V, V OP W>'. */
@@ -6632,6 +6608,34 @@ insert_range_assertions (void)
BITMAP_FREE (need_assert_for);
}
+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;
+
+ void vrp_initialize (void);
+ void vrp_finalize (bool);
+ void check_all_array_refs (void);
+ void check_array_ref (location_t, tree, bool);
+ void search_for_addr_array (tree, location_t);
+
+ class vr_values vr_values;
+ /* Temporary delegator to minimize code churn. */
+ value_range *get_value_range (const_tree op)
+ { return vr_values.get_value_range (op); }
+ void set_defs_to_varying (gimple *stmt)
+ { return vr_values.set_defs_to_varying (stmt); }
+ void extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
+ tree *output_p, value_range *vr)
+ { vr_values.extract_range_from_stmt (stmt, taken_edge_p, output_p, vr); }
+ bool update_value_range (const_tree op, value_range *vr)
+ { return vr_values.update_value_range (op, vr); }
+ void extract_range_basic (value_range *vr, gimple *stmt)
+ { vr_values.extract_range_basic (vr, stmt); }
+ void extract_range_from_phi_node (gphi *phi, value_range *vr)
+ { vr_values.extract_range_from_phi_node (phi, vr); }
+};
/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
and "struct" hacks. If VRP can determine that the
array subscript is a constant, check if it is outside valid
@@ -6639,8 +6643,9 @@ insert_range_assertions (void)
non-overlapping with valid range.
IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR. */
-static void
-check_array_ref (location_t location, tree ref, bool ignore_off_by_one)
+void
+vrp_prop::check_array_ref (location_t location, tree ref,
+ bool ignore_off_by_one)
{
value_range *vr = NULL;
tree low_sub, up_sub;
@@ -6732,8 +6737,8 @@ check_array_ref (location_t location, tree ref, bool ignore_off_by_one)
/* Searches if the expr T, located at LOCATION computes
address of an ARRAY_REF, and call check_array_ref on it. */
-static void
-search_for_addr_array (tree t, location_t location)
+void
+vrp_prop::search_for_addr_array (tree t, location_t location)
{
/* Check each ARRAY_REFs in the reference chain. */
do
@@ -6818,12 +6823,13 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data)
*walk_subtree = TRUE;
+ vrp_prop *vrp_prop = (class vrp_prop *)wi->info;
if (TREE_CODE (t) == ARRAY_REF)
- check_array_ref (location, t, false /*ignore_off_by_one*/);
+ vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/);
else if (TREE_CODE (t) == ADDR_EXPR)
{
- search_for_addr_array (t, location);
+ vrp_prop->search_for_addr_array (t, location);
*walk_subtree = FALSE;
}
@@ -6833,8 +6839,8 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data)
/* Walk over all statements of all reachable BBs and call check_array_bounds
on them. */
-static void
-check_all_array_refs (void)
+void
+vrp_prop::check_all_array_refs ()
{
basic_block bb;
gimple_stmt_iterator si;
@@ -6861,6 +6867,8 @@ check_all_array_refs (void)
memset (&wi, 0, sizeof (wi));
+ wi.info = this;
+
walk_gimple_op (gsi_stmt (si),
check_array_bounds,
&wi);
@@ -7111,8 +7119,7 @@ stmt_interesting_for_vrp (gimple *stmt)
/* Initialize VRP lattice. */
-static void
-vrp_initialize_lattice ()
+vr_values::vr_values () : vrp_value_range_pool ("Tree VRP value ranges")
{
values_propagated = false;
num_vr_values = num_ssa_names;
@@ -7123,8 +7130,8 @@ vrp_initialize_lattice ()
/* Initialization required by ssa_propagate engine. */
-static void
-vrp_initialize ()
+void
+vrp_prop::vrp_initialize ()
{
basic_block bb;
@@ -7165,6 +7172,9 @@ vrp_initialize ()
}
}
+/* A hack. */
+static class vr_values *x_vr_values;
+
/* Return the singleton value-range for NAME or NAME. */
static inline tree
@@ -7172,7 +7182,7 @@ vrp_valueize (tree name)
{
if (TREE_CODE (name) == SSA_NAME)
{
- value_range *vr = get_value_range (name);
+ value_range *vr = x_vr_values->get_value_range (name);
if (vr->type == VR_RANGE
&& (TREE_CODE (vr->min) == SSA_NAME
|| is_gimple_min_invariant (vr->min))
@@ -7197,7 +7207,7 @@ vrp_valueize_1 (tree name)
if (!gimple_nop_p (def_stmt)
&& prop_simulate_again_p (def_stmt))
return NULL_TREE;
- value_range *vr = get_value_range (name);
+ value_range *vr = x_vr_values->get_value_range (name);
if (range_int_cst_singleton_p (vr))
return vr->min;
}
@@ -7207,8 +7217,9 @@ vrp_valueize_1 (tree name)
/* Visit assignment STMT. If it produces an interesting range, record
the range in VR and set LHS to OUTPUT_P. */
-static void
-vrp_visit_assignment_or_call (gimple *stmt, tree *output_p, value_range *vr)
+void
+vr_values::vrp_visit_assignment_or_call (gimple *stmt, tree *output_p,
+ value_range *vr)
{
tree lhs;
enum gimple_code code = gimple_code (stmt);
@@ -7227,8 +7238,10 @@ vrp_visit_assignment_or_call (gimple *stmt, tree *output_p, value_range *vr)
*output_p = lhs;
/* Try folding the statement to a constant first. */
+ x_vr_values = this;
tree tem = gimple_fold_stmt_to_constant_1 (stmt, vrp_valueize,
vrp_valueize_1);
+ x_vr_values = NULL;
if (tem)
{
if (TREE_CODE (tem) == SSA_NAME
@@ -7256,8 +7269,8 @@ vrp_visit_assignment_or_call (gimple *stmt, tree *output_p, value_range *vr)
or a symbolic range containing the SSA_NAME only if the value range
is varying or undefined. */
-static inline value_range
-get_vr_for_comparison (int i)
+value_range
+vr_values::get_vr_for_comparison (int i)
{
value_range vr = *get_value_range (ssa_name (i));
@@ -7279,9 +7292,9 @@ get_vr_for_comparison (int i)
compare_range_with_value, including the setting of
*STRICT_OVERFLOW_P. */
-static tree
-compare_name_with_value (enum tree_code comp, tree var, tree val,
- bool *strict_overflow_p, bool use_equiv_p)
+tree
+vr_values::compare_name_with_value (enum tree_code comp, tree var, tree val,
+ bool *strict_overflow_p, bool use_equiv_p)
{
bitmap_iterator bi;
unsigned i;
@@ -7364,9 +7377,9 @@ compare_name_with_value (enum tree_code comp, tree var, tree val,
whether we relied on undefined signed overflow in the comparison. */
-static tree
-compare_names (enum tree_code comp, tree n1, tree n2,
- bool *strict_overflow_p)
+tree
+vr_values::compare_names (enum tree_code comp, tree n1, tree n2,
+ bool *strict_overflow_p)
{
tree t, retval;
bitmap e1, e2;
@@ -7479,10 +7492,9 @@ compare_names (enum tree_code comp, tree n1, tree n2,
/* Helper function for vrp_evaluate_conditional_warnv & other
optimizers. */
-static tree
-vrp_evaluate_conditional_warnv_with_ops_using_ranges (enum tree_code code,
- tree op0, tree op1,
- bool * strict_overflow_p)
+tree
+vr_values::vrp_evaluate_conditional_warnv_with_ops_using_ranges
+ (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
{
value_range *vr0, *vr1;
@@ -7502,10 +7514,12 @@ vrp_evaluate_conditional_warnv_with_ops_using_ranges (enum tree_code code,
/* Helper function for vrp_evaluate_conditional_warnv. */
-static tree
-vrp_evaluate_conditional_warnv_with_ops (enum tree_code code, tree op0,
- tree op1, bool use_equiv_p,
- bool *strict_overflow_p, bool *only_ranges)
+tree
+vr_values::vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
+ tree op0, tree op1,
+ bool use_equiv_p,
+ bool *strict_overflow_p,
+ bool *only_ranges)
{
tree ret;
if (only_ranges)
@@ -7574,8 +7588,9 @@ vrp_evaluate_conditional_warnv_with_ops (enum tree_code code, tree op0,
based on undefined signed overflow, issue a warning if
appropriate. */
-static tree
-vrp_evaluate_conditional (tree_code code, tree op0, tree op1, gimple *stmt)
+tree
+vr_values::vrp_evaluate_conditional (tree_code code, tree op0,
+ tree op1, gimple *stmt)
{
bool sop;
tree ret;
@@ -7666,8 +7681,8 @@ vrp_evaluate_conditional (tree_code code, tree op0, tree op1, gimple *stmt)
will be taken out of STMT's basic block, record it in
*TAKEN_EDGE_P. Otherwise, set *TAKEN_EDGE_P to NULL. */
-static void
-vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
+void
+vr_values::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
{
tree val;
@@ -7950,8 +7965,8 @@ find_case_label_ranges (gswitch *stmt, value_range *vr, size_t *min_idx1,
will be taken out of STMT's basic block, record it in
*TAKEN_EDGE_P. Otherwise, *TAKEN_EDGE_P set to NULL. */
-static void
-vrp_visit_switch_stmt (gswitch *stmt, edge *taken_edge_p)
+void
+vr_values::vrp_visit_switch_stmt (gswitch *stmt, edge *taken_edge_p)
{
tree op, val;
value_range *vr;
@@ -8042,9 +8057,9 @@ vrp_visit_switch_stmt (gswitch *stmt, edge *taken_edge_p)
If STMT is a conditional branch and we can determine its truth
value, the taken edge is recorded in *TAKEN_EDGE_P. */
-static void
-extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
- tree *output_p, value_range *vr)
+void
+vr_values::extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
+ tree *output_p, value_range *vr)
{
if (dump_file && (dump_flags & TDF_DETAILS))
@@ -8063,13 +8078,6 @@ 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.
@@ -8828,7 +8836,9 @@ vrp_intersect_ranges_1 (value_range *vr0, value_range *vr1)
bitmap_ior_into (vr0->equiv, vr1->equiv);
else if (vr1->equiv && !vr0->equiv)
{
- vr0->equiv = BITMAP_ALLOC (&vrp_equiv_obstack);
+ /* All equivalence bitmaps are allocated from the same obstack. So
+ we can use the obstack associated with VR to allocate vr0->equiv. */
+ vr0->equiv = BITMAP_ALLOC (vr1->equiv->obstack);
bitmap_copy (vr0->equiv, vr1->equiv);
}
}
@@ -8955,8 +8965,8 @@ vrp_meet (value_range *vr0, const value_range *vr1)
edges. If a valid value range can be derived from all the incoming
value ranges, set a new range in VR_RESULT. */
-static void
-extract_range_from_phi_node (gphi *phi, value_range *vr_result)
+void
+vr_values::extract_range_from_phi_node (gphi *phi, value_range *vr_result)
{
size_t i;
tree lhs = PHI_RESULT (phi);
@@ -9200,8 +9210,9 @@ vrp_prop::visit_phi (gphi *phi)
/* Simplify boolean operations if the source is known
to be already a boolean. */
-static bool
-simplify_truth_ops_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_truth_ops_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree lhs, op0, op1;
@@ -9276,8 +9287,9 @@ simplify_truth_ops_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
[-op1min + 1, op1min - 1] for signed and [0, op1min - 1] for unsigned
modulo. */
-static bool
-simplify_div_or_mod_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_div_or_mod_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree val = NULL;
@@ -9400,8 +9412,9 @@ simplify_div_or_mod_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
/* Simplify a min or max if the ranges of the two operands are
disjoint. Return true if we do simplify. */
-static bool
-simplify_min_or_max_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_min_or_max_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
tree op0 = gimple_assign_rhs1 (stmt);
tree op1 = gimple_assign_rhs2 (stmt);
@@ -9447,8 +9460,8 @@ simplify_min_or_max_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
ABS_EXPR. If the operand is <= 0, then simplify the
ABS_EXPR into a NEGATE_EXPR. */
-static bool
-simplify_abs_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_abs_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
{
tree op = gimple_assign_rhs1 (stmt);
value_range *vr = get_value_range (op);
@@ -9503,8 +9516,9 @@ simplify_abs_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
set by | are already known to be one from VR, the bit
operation is redundant. */
-static bool
-simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
tree op0 = gimple_assign_rhs1 (stmt);
tree op1 = gimple_assign_rhs2 (stmt);
@@ -9700,8 +9714,8 @@ range_fits_type_p (value_range *vr, unsigned dest_precision, signop dest_sgn)
test if the range information indicates only one value can satisfy
the original conditional. */
-static bool
-simplify_cond_using_ranges_1 (gcond *stmt)
+bool
+vr_values::simplify_cond_using_ranges_1 (gcond *stmt)
{
tree op0 = gimple_cond_lhs (stmt);
tree op1 = gimple_cond_rhs (stmt);
@@ -9785,8 +9799,8 @@ simplify_cond_using_ranges_1 (gcond *stmt)
of the type conversion. Doing so makes the conversion dead which helps
subsequent passes. */
-static void
-simplify_cond_using_ranges_2 (gcond *stmt)
+void
+vr_values::simplify_cond_using_ranges_2 (gcond *stmt)
{
tree op0 = gimple_cond_lhs (stmt);
tree op1 = gimple_cond_rhs (stmt);
@@ -9842,8 +9856,8 @@ simplify_cond_using_ranges_2 (gcond *stmt)
/* Simplify a switch statement using the value range of the switch
argument. */
-static bool
-simplify_switch_using_ranges (gswitch *stmt)
+bool
+vr_values::simplify_switch_using_ranges (gswitch *stmt)
{
tree op = gimple_switch_index (stmt);
value_range *vr = NULL;
@@ -10100,9 +10114,9 @@ simplify_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
/* Simplify a conversion from integral SSA name to float in STMT. */
-static bool
-simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi,
- gimple *stmt)
+bool
+vr_values::simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
tree rhs1 = gimple_assign_rhs1 (stmt);
value_range *vr = get_value_range (rhs1);
@@ -10164,8 +10178,9 @@ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi,
/* Simplify an internal fn call using ranges if possible. */
-static bool
-simplify_internal_call_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
+bool
+vr_values::simplify_internal_call_using_ranges (gimple_stmt_iterator *gsi,
+ gimple *stmt)
{
enum tree_code subcode;
bool is_ubsan = false;
@@ -10266,8 +10281,8 @@ simplify_internal_call_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
/* Return true if VAR is a two-valued variable. Set a and b with the
two-values when it is true. Return false otherwise. */
-static bool
-two_valued_val_range_p (tree var, tree *a, tree *b)
+bool
+vr_values::two_valued_val_range_p (tree var, tree *a, tree *b)
{
value_range *vr = get_value_range (var);
if ((vr->type != VR_RANGE
@@ -10301,8 +10316,8 @@ two_valued_val_range_p (tree var, tree *a, tree *b)
/* Simplify STMT using ranges if possible. */
-static bool
-simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
+bool
+vr_values::simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
{
gimple *stmt = gsi_stmt (*gsi);
if (is_gimple_assign (stmt))
@@ -10445,12 +10460,33 @@ simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
return false;
}
+class vrp_folder : public substitute_and_fold_engine
+{
+ public:
+ tree get_value (tree) FINAL OVERRIDE;
+ bool fold_stmt (gimple_stmt_iterator *) FINAL OVERRIDE;
+ bool fold_predicate_in (gimple_stmt_iterator *);
+
+ class vr_values *vr_values;
+
+ /* Delegators. */
+ tree vrp_evaluate_conditional (tree_code code, tree op0,
+ tree op1, gimple *stmt)
+ { return vr_values->vrp_evaluate_conditional (code, op0, op1, stmt); }
+ bool simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
+ { return vr_values->simplify_stmt_using_ranges (gsi); }
+ tree op_with_constant_singleton_value_range (tree op)
+ { return vr_values->op_with_constant_singleton_value_range (op); }
+
+
+};
+
/* If the statement pointed by SI has a predicate whose value can be
computed using the value range information computed by VRP, compute
its value and return true. Otherwise, return false. */
-static bool
-fold_predicate_in (gimple_stmt_iterator *si)
+bool
+vrp_folder::fold_predicate_in (gimple_stmt_iterator *si)
{
bool assignment_p = false;
tree val;
@@ -10507,13 +10543,6 @@ fold_predicate_in (gimple_stmt_iterator *si)
return false;
}
-class vrp_folder : public substitute_and_fold_engine
-{
- public:
- tree get_value (tree) FINAL OVERRIDE;
- bool fold_stmt (gimple_stmt_iterator *) FINAL OVERRIDE;
-};
-
/* Callback for substitute_and_fold folding the stmt at *SI. */
bool
@@ -10579,6 +10608,7 @@ simplify_stmt_for_jump_threading (gimple *stmt, gimple *within_stmt,
if (cached_lhs && is_gimple_min_invariant (cached_lhs))
return cached_lhs;
+ vr_values *vr_values = x_vr_values;
if (gcond *cond_stmt = dyn_cast <gcond *> (stmt))
{
tree op0 = gimple_cond_lhs (cond_stmt);
@@ -10587,8 +10617,8 @@ simplify_stmt_for_jump_threading (gimple *stmt, gimple *within_stmt,
tree op1 = gimple_cond_rhs (cond_stmt);
op1 = lhs_of_dominating_assert (op1, bb, stmt);
- return vrp_evaluate_conditional (gimple_cond_code (cond_stmt),
- op0, op1, within_stmt);
+ return vr_values->vrp_evaluate_conditional (gimple_cond_code (cond_stmt),
+ op0, op1, within_stmt);
}
/* We simplify a switch statement by trying to determine which case label
@@ -10602,7 +10632,7 @@ simplify_stmt_for_jump_threading (gimple *stmt, gimple *within_stmt,
op = lhs_of_dominating_assert (op, bb, stmt);
- value_range *vr = get_value_range (op);
+ value_range *vr = vr_values->get_value_range (op);
if ((vr->type != VR_RANGE && vr->type != VR_ANTI_RANGE)
|| symbolic_range_p (vr))
return NULL_TREE;
@@ -10663,7 +10693,7 @@ simplify_stmt_for_jump_threading (gimple *stmt, gimple *within_stmt,
&& (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
|| POINTER_TYPE_P (TREE_TYPE (lhs))))
{
- extract_range_from_assignment (&new_vr, assign_stmt);
+ vr_values->extract_range_from_assignment (&new_vr, assign_stmt);
if (range_int_cst_singleton_p (&new_vr))
return new_vr.min;
}
@@ -10686,11 +10716,14 @@ public:
virtual edge before_dom_children (basic_block);
virtual void after_dom_children (basic_block);
+ class vr_values *vr_values;
+
private:
class const_and_copies *m_const_and_copies;
class avail_exprs_stack *m_avail_exprs_stack;
gcond *m_dummy_cond;
+
};
/* Called before processing dominator children of BB. We want to look
@@ -10743,9 +10776,11 @@ vrp_dom_walker::after_dom_children (basic_block bb)
integer_zero_node, integer_zero_node,
NULL, NULL);
+ x_vr_values = vr_values;
thread_outgoing_edges (bb, m_dummy_cond, m_const_and_copies,
m_avail_exprs_stack,
simplify_stmt_for_jump_threading);
+ x_vr_values = NULL;
m_avail_exprs_stack->pop_to_marker ();
m_const_and_copies->pop_to_marker ();
@@ -10772,7 +10807,7 @@ vrp_dom_walker::after_dom_children (basic_block bb)
for later realization. */
static void
-identify_jump_threads (void)
+identify_jump_threads (class vr_values *vr_values)
{
int i;
edge e;
@@ -10804,6 +10839,7 @@ identify_jump_threads (void)
= new class avail_exprs_stack (avail_exprs);
vrp_dom_walker walker (CDI_DOMINATORS, equiv_stack, avail_exprs_stack);
+ walker.vr_values = vr_values;
walker.walk (cfun->cfg->x_entry_block_ptr);
/* Clear EDGE_IGNORE. */
@@ -10820,8 +10856,7 @@ identify_jump_threads (void)
/* Free VRP lattice. */
-static void
-vrp_free_lattice ()
+vr_values::~vr_values ()
{
/* Free allocated memory. */
free (vr_value);
@@ -10837,48 +10872,49 @@ vrp_free_lattice ()
/* Traverse all the blocks folding conditionals with known ranges. */
-static void
-vrp_finalize (bool warn_array_bounds_p)
+void
+vrp_prop::vrp_finalize (bool warn_array_bounds_p)
{
size_t i;
- values_propagated = true;
+ vr_values.values_propagated = true;
if (dump_file)
{
fprintf (dump_file, "\nValue ranges after VRP:\n\n");
- dump_all_value_ranges (dump_file);
+ vr_values.dump_all_value_ranges (dump_file);
fprintf (dump_file, "\n");
}
/* Set value range to non pointer SSA_NAMEs. */
- for (i = 0; i < num_vr_values; i++)
- if (vr_value[i])
- {
- tree name = ssa_name (i);
+ for (i = 0; i < num_ssa_names; i++)
+ {
+ tree name = ssa_name (i);
+ if (!name)
+ continue;
- if (!name
- || (vr_value[i]->type == VR_VARYING)
- || (vr_value[i]->type == VR_UNDEFINED)
- || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
- || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
- continue;
+ value_range *vr = get_value_range (name);
+ if (!name
+ || (vr->type == VR_VARYING)
+ || (vr->type == VR_UNDEFINED)
+ || (TREE_CODE (vr->min) != INTEGER_CST)
+ || (TREE_CODE (vr->max) != INTEGER_CST))
+ continue;
- if (POINTER_TYPE_P (TREE_TYPE (name))
- && ((vr_value[i]->type == VR_RANGE
- && range_includes_zero_p (vr_value[i]->min,
- vr_value[i]->max) == 0)
- || (vr_value[i]->type == VR_ANTI_RANGE
- && range_includes_zero_p (vr_value[i]->min,
- vr_value[i]->max) == 1)))
- set_ptr_nonnull (name);
- else if (!POINTER_TYPE_P (TREE_TYPE (name)))
- set_range_info (name, vr_value[i]->type,
- wi::to_wide (vr_value[i]->min),
- wi::to_wide (vr_value[i]->max));
- }
+ if (POINTER_TYPE_P (TREE_TYPE (name))
+ && ((vr->type == VR_RANGE
+ && range_includes_zero_p (vr->min, vr->max) == 0)
+ || (vr->type == VR_ANTI_RANGE
+ && range_includes_zero_p (vr->min, vr->max) == 1)))
+ set_ptr_nonnull (name);
+ else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+ set_range_info (name, vr->type,
+ wi::to_wide (vr->min),
+ wi::to_wide (vr->max));
+ }
class vrp_folder vrp_folder;
+ vrp_folder.vr_values = &vr_values;
vrp_folder.substitute_and_fold ();
if (warn_array_bounds && warn_array_bounds_p)
@@ -10912,6 +10948,38 @@ public:
bitmap need_eh_cleanup;
auto_vec<gimple *> stmts_to_fixup;
auto_vec<gimple *> stmts_to_remove;
+
+ class vr_values vr_values;
+
+ /* Temporary delegators. */
+ value_range *get_value_range (const_tree op)
+ { return vr_values.get_value_range (op); }
+ bool update_value_range (const_tree op, value_range *vr)
+ { return vr_values.update_value_range (op, vr); }
+ void extract_range_from_phi_node (gphi *phi, value_range *vr)
+ { vr_values.extract_range_from_phi_node (phi, vr); }
+ void extract_range_for_var_from_comparison_expr (tree var,
+ enum tree_code cond_code,
+ tree op, tree limit,
+ value_range *vr_p)
+ { vr_values.extract_range_for_var_from_comparison_expr (var, cond_code,
+ op, limit, vr_p); }
+ void adjust_range_with_scev (value_range *vr, struct loop *loop,
+ gimple *stmt, tree var)
+ { vr_values.adjust_range_with_scev (vr, loop, stmt, var); }
+ tree op_with_constant_singleton_value_range (tree op)
+ { return vr_values.op_with_constant_singleton_value_range (op); }
+ void extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
+ tree *output_p, value_range *vr)
+ { vr_values.extract_range_from_stmt (stmt, taken_edge_p, output_p, vr); }
+ void set_defs_to_varying (gimple *stmt)
+ { return vr_values.set_defs_to_varying (stmt); }
+ void set_value_range_ (tree name, value_range *vr)
+ { vr_values.set_value_range_ (name, vr); }
+ void simplify_cond_using_ranges_2 (gcond *stmt)
+ { vr_values.simplify_cond_using_ranges_2 (stmt); }
+ void vrp_visit_cond_stmt (gcond *cond, edge *e)
+ { vr_values.vrp_visit_cond_stmt (cond, e); }
};
/* Find new range for NAME such that (OP CODE LIMIT) is true. */
@@ -10934,7 +11002,7 @@ evrp_dom_walker::try_find_new_range (tree name,
&& vrp_operand_equal_p (old_vr->min, vr.min)
&& vrp_operand_equal_p (old_vr->max, vr.max))
return NULL;
- value_range *new_vr = vrp_value_range_pool.allocate ();
+ value_range *new_vr = vr_values.vrp_value_range_pool.allocate ();
*new_vr = vr;
return new_vr;
}
@@ -11203,6 +11271,7 @@ evrp_dom_walker::before_dom_children (basic_block bb)
/* Try folding stmts with the VR discovered. */
class vrp_folder vrp_folder;
+ vrp_folder.vr_values = &vr_values;
bool did_replace = vrp_folder.replace_uses_in (stmt);
if (fold_stmt (&gsi, follow_single_use_edges)
|| did_replace)
@@ -11269,13 +11338,19 @@ evrp_dom_walker::after_dom_children (basic_block bb ATTRIBUTE_UNUSED)
stack.pop ();
}
+void
+vr_values::set_value_range_ (tree var, value_range *vr)
+{
+ if (SSA_NAME_VERSION (var) >= num_vr_values)
+ return;
+ vr_value[SSA_NAME_VERSION (var)] = vr;
+}
+
/* Push the Value Range of VAR to the stack and update it with new VR. */
void
evrp_dom_walker::push_value_range (tree var, value_range *vr)
{
- if (SSA_NAME_VERSION (var) >= num_vr_values)
- return;
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "pushing new range for ");
@@ -11285,7 +11360,7 @@ evrp_dom_walker::push_value_range (tree var, value_range *vr)
fprintf (dump_file, "\n");
}
stack.safe_push (std::make_pair (var, get_value_range (var)));
- vr_value[SSA_NAME_VERSION (var)] = vr;
+ set_value_range_ (var, vr);
}
/* Pop the Value Range from the vrp_stack and update VAR with it. */
@@ -11303,7 +11378,7 @@ evrp_dom_walker::pop_value_range (tree var)
dump_value_range (dump_file, vr);
fprintf (dump_file, "\n");
}
- vr_value[SSA_NAME_VERSION (var)] = vr;
+ set_value_range_ (var, vr);
stack.pop ();
return vr;
}
@@ -11330,7 +11405,6 @@ execute_early_vrp ()
FOR_EACH_EDGE (e, ei, bb->preds)
e->flags |= EDGE_EXECUTABLE;
}
- vrp_initialize_lattice ();
/* Walk stmts in dominance order and propagate VRP. */
evrp_dom_walker walker;
@@ -11339,7 +11413,7 @@ execute_early_vrp ()
if (dump_file)
{
fprintf (dump_file, "\nValue ranges after Early VRP:\n\n");
- dump_all_value_ranges (dump_file);
+ walker.vr_values.dump_all_value_ranges (dump_file);
fprintf (dump_file, "\n");
}
@@ -11377,7 +11451,6 @@ execute_early_vrp ()
fixup_noreturn_call (stmt);
}
- vrp_free_lattice ();
scev_finalize ();
loop_optimizer_finalize ();
return 0;
@@ -11451,15 +11524,14 @@ execute_vrp (bool warn_array_bounds_p)
/* For visiting PHI nodes we need EDGE_DFS_BACK computed. */
mark_dfs_back_edges ();
- vrp_initialize_lattice ();
- vrp_initialize ();
class vrp_prop vrp_prop;
+ vrp_prop.vrp_initialize ();
vrp_prop.ssa_propagate ();
- vrp_finalize (warn_array_bounds_p);
+ vrp_prop.vrp_finalize (warn_array_bounds_p);
/* We must identify jump threading opportunities before we release
the datastructures built by VRP. */
- identify_jump_threads ();
+ identify_jump_threads (&vrp_prop.vr_values);
/* A comparison of an SSA_NAME against a constant where the SSA_NAME
was set by a type conversion can often be rewritten to use the
@@ -11473,11 +11545,9 @@ execute_vrp (bool warn_array_bounds_p)
{
gimple *last = last_stmt (bb);
if (last && gimple_code (last) == GIMPLE_COND)
- simplify_cond_using_ranges_2 (as_a <gcond *> (last));
+ vrp_prop.vr_values.simplify_cond_using_ranges_2 (as_a <gcond *> (last));
}
- vrp_free_lattice ();
-
free_numbers_of_iterations_estimates (cfun);
/* ASSERT_EXPRs must be removed before finalizing jump threads
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
new file mode 100644
index 0000000..e1b2cbb
--- /dev/null
+++ b/gcc/vr-values.h
@@ -0,0 +1,126 @@
+/* Support routines for Value Range Propagation (VRP).
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+#ifndef GCC_VR_VALUES_H
+#define GCC_VR_VALUES_H
+
+/* The VR_VALUES class holds the current view of range information
+ for all the SSA_NAMEs in the IL.
+
+ It can be used to hold context sensitive range information during
+ a dominator walk or it may be used to hold range information in the
+ standard VRP pass as ranges are propagated through the lattice to a
+ steady state.
+
+ This information is independent of the range information that gets
+ attached to SSA_NAMEs. A pass such as VRP may choose to transfer
+ the global information it produces into global range information that
+ gets attached to an SSA_NAME. It's unclear how useful that global
+ information will be in a world where we can compute context sensitive
+ range information fast or perform on-demand queries. */
+class vr_values
+{
+ public:
+ vr_values (void);
+ ~vr_values (void);
+
+ /* Value range array. After propagation, VR_VALUE[I] holds the range
+ of values that SSA name N_I may take. */
+ unsigned int num_vr_values;
+ value_range **vr_value;
+ bool values_propagated;
+
+ /* For a PHI node which sets SSA name N_I, VR_COUNTS[I] holds the
+ number of executable edges we saw the last time we visited the
+ node. */
+ int *vr_phi_edge_counts;
+
+ /* Allocation pools for tree-vrp allocations. */
+ object_allocator<value_range> vrp_value_range_pool;
+ bitmap_obstack vrp_equiv_obstack;
+
+ value_range *get_value_range (const_tree);
+ void set_value_range_ (tree, value_range *);
+
+ void set_defs_to_varying (gimple *);
+ bool update_value_range (const_tree, value_range *);
+ void add_equivalence (bitmap *, const_tree);
+ bool vrp_stmt_computes_nonzero (gimple *);
+ tree op_with_constant_singleton_value_range (tree);
+ bool op_with_boolean_value_range_p (tree);
+ bool check_for_binary_op_overflow (enum tree_code, tree, tree, tree, bool *);
+ void adjust_range_with_scev (value_range *, struct loop *, gimple *, tree);
+ value_range get_vr_for_comparison (int);
+ tree compare_name_with_value (enum tree_code, tree, tree, bool *, bool);
+ tree compare_names (enum tree_code, tree, tree, bool *);
+ bool two_valued_val_range_p (tree, tree *, tree *);
+
+ tree vrp_evaluate_conditional_warnv_with_ops_using_ranges (enum tree_code,
+ tree, tree,
+ bool *);
+ tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
+ tree, tree, bool,
+ bool *, bool *);
+ tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
+
+
+ void dump_all_value_ranges (FILE *);
+
+ void extract_range_for_var_from_comparison_expr (tree, enum tree_code,
+ tree, tree, value_range *);
+ void extract_range_from_assert (value_range *, tree);
+ void extract_range_from_ssa_name (value_range *, tree);
+ void extract_range_from_binary_expr (value_range *, enum tree_code,
+ tree, tree, tree);
+ void extract_range_from_unary_expr (value_range *, enum tree_code,
+ tree, tree);
+ void extract_range_from_phi_node (gphi *, value_range *);
+ void extract_range_from_cond_expr (value_range *, gassign *);
+ void extract_range_basic (value_range *, gimple *);
+ void extract_range_from_assignment (value_range *, gassign *);
+ void extract_range_from_stmt (gimple *, edge *, tree *, value_range *);
+ void extract_range_from_comparison (value_range *, enum tree_code,
+ tree, tree, tree);
+
+ void vrp_visit_assignment_or_call (gimple*, tree *, value_range *);
+ void vrp_visit_switch_stmt (gswitch *, edge *);
+ void vrp_visit_cond_stmt (gcond *, edge *);
+
+ bool simplify_truth_ops_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_div_or_mod_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_abs_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_bit_ops_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_min_or_max_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_cond_using_ranges_1 (gcond *);
+ void simplify_cond_using_ranges_2 (gcond *);
+ bool simplify_switch_using_ranges (gswitch *);
+ bool simplify_float_conversion_using_ranges (gimple_stmt_iterator *,
+ gimple *);
+ bool simplify_internal_call_using_ranges (gimple_stmt_iterator *, gimple *);
+ bool simplify_stmt_using_ranges (gimple_stmt_iterator *);
+
+
+
+
+private:
+
+
+};
+
+#endif /* GCC_VR_VALUES_H */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
2017-11-07 22:27 [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class Jeff Law
@ 2017-11-08 13:14 ` Trevor Saunders
2017-11-09 15:27 ` Richard Biener
2017-11-09 18:41 ` Martin Sebor
1 sibling, 1 reply; 6+ messages in thread
From: Trevor Saunders @ 2017-11-08 13:14 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Tue, Nov 07, 2017 at 02:03:19PM -0700, Jeff Law wrote:
> So this is the first step in pulling apart tree-vrp.c. As outlined in
> an earlier message the goal of this patch is just to get the vr_values
> class introduced. I've tried (and mostly succeeded) in avoiding doing
> significant cleanups/rewrites at this point to make reviewing this step
> easier.
>
> The vast majority of this patch is just changing functions from free
> functions to member functions in the appropriate class (primarily
> vr_values), pulling the appropriate static objects into that class and
> adding the temporary delegators to minimize diffs at this stage.
> Exceptions to this:
>
> set_value_range changes how we get at the obstack for allocating
> bitmaps. This was discussed on-list recently. Similarly in
> vrp_intersect_ranges_1.
>
> extract_range_from_unary_expr has two implementations. One is within
> the vr_values class which calls out to the freestanding function. Hence
> the ::extract_range_from_unary_expr.
>
> We drop debug_all_value_ranges. It doesn't make sense in a world where
> the value ranges are attached to a class instance. There is still a
> method to dump all the value ranges within the class instance.
>
> The vrp_prop and vrp_folder class definitions have to move to earlier
> points in the file.
>
> We pass a vrp_prop class instance through the gimple walkers using the
> wi->info field. check_all_array_refs sets that up and we recover the
> class instance pointer in the check_array_bounds callback.
>
> I wasn't up to converting the gimple folder to classes at this time. So
> we set/wipe x_vr_values (file scoped static) around the calls into the
> gimple walkers and recover the vr_values class instance from x_vr_values
> within vrp_valueize and vrp_valueize_1.
>
> I use a similar hack to recover the vr_values instance in the callback
> to simplify a statement for jump threading. This code is on the
> chopping block -- I expect it all to just disappear shortly. Similarly
> I just pass in vr_values to identify_jump_threads. Again, I expect to
> be removing all this code shortly and wanted to keep this as simple as
> possible rather than waste time cleaning up code I'm just going to remove.
>
> I did do some simplifications in vrp_finalize. It wanted to access
> vr_value and num_vr_values directly. Now it goes through the
> get_value_range interface.
>
> I introduced a set_value_range_ member function in vr_values, then
> twiddled a couple evrp routines to use that rather than access vr_value
> directly.
>
> In a few places I didn't bother creating delegators. The delegators
> wouldn't have simplified anything. See execute_vrp and execute_early_vrp.
>
> You'll note the vr_values class is large. I've tried to avoid the
> temptation to start simplifying stuff and instead try to stick with
> moving routines into the appropriate class based on the code as it
> stands right now. The size of vr_values is probably a good indicator
> that it's doing too much.
>
> As suggested yesterday, I've added various delegator methods to the
> classes to minimize code churn at this time. I suspect we're going to
> want to remove most, if not all of them. But again, the goal in this
> patch is to get the class structure in place with minimal amount of
> collateral damage to make this step easy to review.
Yeah, this all looks pretty reasonable, ime breaking things also helps
when writing the patch and you break something since there's less to
undo before getting to a working state. If anything I would have broken
this up even more, moving the existing classes up can probably be its
own thing, as well as the use of equiv->obstack.
Trev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
2017-11-08 13:14 ` Trevor Saunders
@ 2017-11-09 15:27 ` Richard Biener
2017-11-09 18:47 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2017-11-09 15:27 UTC (permalink / raw)
To: Trevor Saunders; +Cc: Jeff Law, gcc-patches
On Wed, Nov 8, 2017 at 1:53 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> On Tue, Nov 07, 2017 at 02:03:19PM -0700, Jeff Law wrote:
>> So this is the first step in pulling apart tree-vrp.c. As outlined in
>> an earlier message the goal of this patch is just to get the vr_values
>> class introduced. I've tried (and mostly succeeded) in avoiding doing
>> significant cleanups/rewrites at this point to make reviewing this step
>> easier.
>>
>> The vast majority of this patch is just changing functions from free
>> functions to member functions in the appropriate class (primarily
>> vr_values), pulling the appropriate static objects into that class and
>> adding the temporary delegators to minimize diffs at this stage.
>> Exceptions to this:
>>
>> set_value_range changes how we get at the obstack for allocating
>> bitmaps. This was discussed on-list recently. Similarly in
>> vrp_intersect_ranges_1.
>>
>> extract_range_from_unary_expr has two implementations. One is within
>> the vr_values class which calls out to the freestanding function. Hence
>> the ::extract_range_from_unary_expr.
>>
>> We drop debug_all_value_ranges. It doesn't make sense in a world where
>> the value ranges are attached to a class instance. There is still a
>> method to dump all the value ranges within the class instance.
>>
>> The vrp_prop and vrp_folder class definitions have to move to earlier
>> points in the file.
>>
>> We pass a vrp_prop class instance through the gimple walkers using the
>> wi->info field. check_all_array_refs sets that up and we recover the
>> class instance pointer in the check_array_bounds callback.
>>
>> I wasn't up to converting the gimple folder to classes at this time. So
>> we set/wipe x_vr_values (file scoped static) around the calls into the
>> gimple walkers and recover the vr_values class instance from x_vr_values
>> within vrp_valueize and vrp_valueize_1.
>>
>> I use a similar hack to recover the vr_values instance in the callback
>> to simplify a statement for jump threading. This code is on the
>> chopping block -- I expect it all to just disappear shortly. Similarly
>> I just pass in vr_values to identify_jump_threads. Again, I expect to
>> be removing all this code shortly and wanted to keep this as simple as
>> possible rather than waste time cleaning up code I'm just going to remove.
>>
>> I did do some simplifications in vrp_finalize. It wanted to access
>> vr_value and num_vr_values directly. Now it goes through the
>> get_value_range interface.
>>
>> I introduced a set_value_range_ member function in vr_values, then
>> twiddled a couple evrp routines to use that rather than access vr_value
>> directly.
>>
>> In a few places I didn't bother creating delegators. The delegators
>> wouldn't have simplified anything. See execute_vrp and execute_early_vrp.
>>
>> You'll note the vr_values class is large. I've tried to avoid the
>> temptation to start simplifying stuff and instead try to stick with
>> moving routines into the appropriate class based on the code as it
>> stands right now. The size of vr_values is probably a good indicator
>> that it's doing too much.
>>
>> As suggested yesterday, I've added various delegator methods to the
>> classes to minimize code churn at this time. I suspect we're going to
>> want to remove most, if not all of them. But again, the goal in this
>> patch is to get the class structure in place with minimal amount of
>> collateral damage to make this step easy to review.
>
> Yeah, this all looks pretty reasonable, ime breaking things also helps
> when writing the patch and you break something since there's less to
> undo before getting to a working state. If anything I would have broken
> this up even more, moving the existing classes up can probably be its
> own thing, as well as the use of equiv->obstack.
Patch looks good to me. I saw
+ void set_value_range_ (tree, value_range *);
and when seeing uses I thought temporary hack because of that
"SSA_NAME_VERSION > length" check but then there's no
set_value_range () function (without _) (or I missed it). Can you just
remove the _ or add a comment?
There's excess vertical space at the end of the vr_values class.
Otherwise ok. No need to split up further.
Thanks,
Richard.
> Trev
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
2017-11-07 22:27 [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class Jeff Law
2017-11-08 13:14 ` Trevor Saunders
@ 2017-11-09 18:41 ` Martin Sebor
2017-11-09 19:33 ` Jeff Law
1 sibling, 1 reply; 6+ messages in thread
From: Martin Sebor @ 2017-11-09 18:41 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 11/07/2017 02:03 PM, Jeff Law wrote:
> So this is the first step in pulling apart tree-vrp.c. As outlined in
> an earlier message the goal of this patch is just to get the vr_values
> class introduced. I've tried (and mostly succeeded) in avoiding doing
> significant cleanups/rewrites at this point to make reviewing this step
> easier.
>
> The vast majority of this patch is just changing functions from free
> functions to member functions in the appropriate class (primarily
> vr_values), pulling the appropriate static objects into that class and
> adding the temporary delegators to minimize diffs at this stage.
> Exceptions to this:
>
> set_value_range changes how we get at the obstack for allocating
> bitmaps. This was discussed on-list recently. Similarly in
> vrp_intersect_ranges_1.
>
> extract_range_from_unary_expr has two implementations. One is within
> the vr_values class which calls out to the freestanding function. Hence
> the ::extract_range_from_unary_expr.
>
> We drop debug_all_value_ranges. It doesn't make sense in a world where
> the value ranges are attached to a class instance. There is still a
> method to dump all the value ranges within the class instance.
>
> The vrp_prop and vrp_folder class definitions have to move to earlier
> points in the file.
>
> We pass a vrp_prop class instance through the gimple walkers using the
> wi->info field. check_all_array_refs sets that up and we recover the
> class instance pointer in the check_array_bounds callback.
>
> I wasn't up to converting the gimple folder to classes at this time. So
> we set/wipe x_vr_values (file scoped static) around the calls into the
> gimple walkers and recover the vr_values class instance from x_vr_values
> within vrp_valueize and vrp_valueize_1.
>
> I use a similar hack to recover the vr_values instance in the callback
> to simplify a statement for jump threading. This code is on the
> chopping block -- I expect it all to just disappear shortly. Similarly
> I just pass in vr_values to identify_jump_threads. Again, I expect to
> be removing all this code shortly and wanted to keep this as simple as
> possible rather than waste time cleaning up code I'm just going to remove.
>
> I did do some simplifications in vrp_finalize. It wanted to access
> vr_value and num_vr_values directly. Now it goes through the
> get_value_range interface.
>
> I introduced a set_value_range_ member function in vr_values, then
> twiddled a couple evrp routines to use that rather than access vr_value
> directly.
>
> In a few places I didn't bother creating delegators. The delegators
> wouldn't have simplified anything. See execute_vrp and execute_early_vrp.
>
> You'll note the vr_values class is large. I've tried to avoid the
> temptation to start simplifying stuff and instead try to stick with
> moving routines into the appropriate class based on the code as it
> stands right now. The size of vr_values is probably a good indicator
> that it's doing too much.
>
> As suggested yesterday, I've added various delegator methods to the
> classes to minimize code churn at this time. I suspect we're going to
> want to remove most, if not all of them. But again, the goal in this
> patch is to get the class structure in place with minimal amount of
> collateral damage to make this step easy to review.
>
> It's worth noting you see no changes that bleed out of tree-vrp.c and
> the vast majority of the patch is just changing the function signature
> so that they're part of the newly created class. I consider that a
> positive :-)
FWIW, I like the "C++-ification" of the API.
Just a few high-level suggestions:
If class vr_values is meant to be copyable and assignable it
should define a copy ctor and assignment operator. Otherwise
it should make them private/deleted (whatever is the convention).
As it is, it looks to me that if a copy is created by accident
the dtor will have undefined behavior.
Also in vr_values, should all the data members really be public?
(If the class maintains any invariants I would expect the members
to be private and accessors provided for them to prevent clients
from inadvertently breaking the invariants).
Consider defining constructors and destructors for the classes
instead of providing initialize and finialize member functions.
(If a dtor takes an argument make it a member of the class.)
Consider renaming the vrp_prop::vr_values data member so that
it doesn't clash with the class name. Same in vrp_folder. There,
I would also suggest to use a different name than in vrp_prop since
the former is a pointer and the latter is an object of the type so
it could get confusing.
Martin
> Bootstrapped and regression tested on x86_64.
>
> OK for the trunk?
>
>
> Jeff
>
> ps. Next step is to pull the class methods into their own files without
> changing *any* parts of their implementation. As a bit of a preview,
> tree-vrp.c is currently around 12000 lines. After moving methods it'll
> be about 7k. vr-values.c will have around 5k lines. The split out EVRP
> pass is ~600 which will be further split roughly in half between the
> core of evrp and the reusable range analysis module.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
2017-11-09 15:27 ` Richard Biener
@ 2017-11-09 18:47 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2017-11-09 18:47 UTC (permalink / raw)
To: Richard Biener, Trevor Saunders; +Cc: gcc-patches
On 11/09/2017 08:24 AM, Richard Biener wrote:
> On Wed, Nov 8, 2017 at 1:53 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
>> On Tue, Nov 07, 2017 at 02:03:19PM -0700, Jeff Law wrote:
>>> So this is the first step in pulling apart tree-vrp.c. As outlined in
>>> an earlier message the goal of this patch is just to get the vr_values
>>> class introduced. I've tried (and mostly succeeded) in avoiding doing
>>> significant cleanups/rewrites at this point to make reviewing this step
>>> easier.
>>>
>>> The vast majority of this patch is just changing functions from free
>>> functions to member functions in the appropriate class (primarily
>>> vr_values), pulling the appropriate static objects into that class and
>>> adding the temporary delegators to minimize diffs at this stage.
>>> Exceptions to this:
>>>
>>> set_value_range changes how we get at the obstack for allocating
>>> bitmaps. This was discussed on-list recently. Similarly in
>>> vrp_intersect_ranges_1.
>>>
>>> extract_range_from_unary_expr has two implementations. One is within
>>> the vr_values class which calls out to the freestanding function. Hence
>>> the ::extract_range_from_unary_expr.
>>>
>>> We drop debug_all_value_ranges. It doesn't make sense in a world where
>>> the value ranges are attached to a class instance. There is still a
>>> method to dump all the value ranges within the class instance.
>>>
>>> The vrp_prop and vrp_folder class definitions have to move to earlier
>>> points in the file.
>>>
>>> We pass a vrp_prop class instance through the gimple walkers using the
>>> wi->info field. check_all_array_refs sets that up and we recover the
>>> class instance pointer in the check_array_bounds callback.
>>>
>>> I wasn't up to converting the gimple folder to classes at this time. So
>>> we set/wipe x_vr_values (file scoped static) around the calls into the
>>> gimple walkers and recover the vr_values class instance from x_vr_values
>>> within vrp_valueize and vrp_valueize_1.
>>>
>>> I use a similar hack to recover the vr_values instance in the callback
>>> to simplify a statement for jump threading. This code is on the
>>> chopping block -- I expect it all to just disappear shortly. Similarly
>>> I just pass in vr_values to identify_jump_threads. Again, I expect to
>>> be removing all this code shortly and wanted to keep this as simple as
>>> possible rather than waste time cleaning up code I'm just going to remove.
>>>
>>> I did do some simplifications in vrp_finalize. It wanted to access
>>> vr_value and num_vr_values directly. Now it goes through the
>>> get_value_range interface.
>>>
>>> I introduced a set_value_range_ member function in vr_values, then
>>> twiddled a couple evrp routines to use that rather than access vr_value
>>> directly.
>>>
>>> In a few places I didn't bother creating delegators. The delegators
>>> wouldn't have simplified anything. See execute_vrp and execute_early_vrp.
>>>
>>> You'll note the vr_values class is large. I've tried to avoid the
>>> temptation to start simplifying stuff and instead try to stick with
>>> moving routines into the appropriate class based on the code as it
>>> stands right now. The size of vr_values is probably a good indicator
>>> that it's doing too much.
>>>
>>> As suggested yesterday, I've added various delegator methods to the
>>> classes to minimize code churn at this time. I suspect we're going to
>>> want to remove most, if not all of them. But again, the goal in this
>>> patch is to get the class structure in place with minimal amount of
>>> collateral damage to make this step easy to review.
>>
>> Yeah, this all looks pretty reasonable, ime breaking things also helps
>> when writing the patch and you break something since there's less to
>> undo before getting to a working state. If anything I would have broken
>> this up even more, moving the existing classes up can probably be its
>> own thing, as well as the use of equiv->obstack.
>
> Patch looks good to me. I saw
>
> + void set_value_range_ (tree, value_range *);
There's a set_value_range free function I didn't want to conflict with:
/* Set value range VR to {T, MIN, MAX, EQUIV}. */
void
set_value_range (value_range *vr, enum value_range_type t, tree min,
tree max, bitmap equiv)
The free function is passed in a VR we want to initialize/set.
THe set_value_range_ member function is used to set an entry in the
vr_values array. I'll choose a better name for the member function.
> and when seeing uses I thought temporary hack because of that
> "SSA_NAME_VERSION > length" check but then there's no
> set_value_range () function (without _) (or I missed it). Can you just
> remove the _ or add a comment?
>
> There's excess vertical space at the end of the vr_values class.
Will fix.
>
> Otherwise ok. No need to split up further.
Funny, I'd split out the two hunks Trevor suggested yesterday. It's a
nice mindless little task I can have running while I look at other
stuff. GIven I've already split it up and it's a tiny plus for
bisecting, when I commit, I'll commit the 3 chunks individually.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class
2017-11-09 18:41 ` Martin Sebor
@ 2017-11-09 19:33 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2017-11-09 19:33 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
On 11/09/2017 11:12 AM, Martin Sebor wrote:
> On 11/07/2017 02:03 PM, Jeff Law wrote:
>> So this is the first step in pulling apart tree-vrp.c. As outlined in
>> an earlier message the goal of this patch is just to get the vr_values
>> class introduced. I've tried (and mostly succeeded) in avoiding doing
>> significant cleanups/rewrites at this point to make reviewing this step
>> easier.
>>
>> The vast majority of this patch is just changing functions from free
>> functions to member functions in the appropriate class (primarily
>> vr_values), pulling the appropriate static objects into that class and
>> adding the temporary delegators to minimize diffs at this stage.
>> Exceptions to this:
>>
>> set_value_range changes how we get at the obstack for allocating
>> bitmaps. This was discussed on-list recently. Similarly in
>> vrp_intersect_ranges_1.
>>
>> extract_range_from_unary_expr has two implementations. One is within
>> the vr_values class which calls out to the freestanding function. Hence
>> the ::extract_range_from_unary_expr.
>>
>> We drop debug_all_value_ranges. It doesn't make sense in a world where
>> the value ranges are attached to a class instance. There is still a
>> method to dump all the value ranges within the class instance.
>>
>> The vrp_prop and vrp_folder class definitions have to move to earlier
>> points in the file.
>>
>> We pass a vrp_prop class instance through the gimple walkers using the
>> wi->info field. check_all_array_refs sets that up and we recover the
>> class instance pointer in the check_array_bounds callback.
>>
>> I wasn't up to converting the gimple folder to classes at this time. So
>> we set/wipe x_vr_values (file scoped static) around the calls into the
>> gimple walkers and recover the vr_values class instance from x_vr_values
>> within vrp_valueize and vrp_valueize_1.
>>
>> I use a similar hack to recover the vr_values instance in the callback
>> to simplify a statement for jump threading. This code is on the
>> chopping block -- I expect it all to just disappear shortly. Similarly
>> I just pass in vr_values to identify_jump_threads. Again, I expect to
>> be removing all this code shortly and wanted to keep this as simple as
>> possible rather than waste time cleaning up code I'm just going to
>> remove.
>>
>> I did do some simplifications in vrp_finalize. It wanted to access
>> vr_value and num_vr_values directly. Now it goes through the
>> get_value_range interface.
>>
>> I introduced a set_value_range_ member function in vr_values, then
>> twiddled a couple evrp routines to use that rather than access vr_value
>> directly.
>>
>> In a few places I didn't bother creating delegators. The delegators
>> wouldn't have simplified anything. See execute_vrp and
>> execute_early_vrp.
>>
>> You'll note the vr_values class is large. I've tried to avoid the
>> temptation to start simplifying stuff and instead try to stick with
>> moving routines into the appropriate class based on the code as it
>> stands right now. The size of vr_values is probably a good indicator
>> that it's doing too much.
>>
>> As suggested yesterday, I've added various delegator methods to the
>> classes to minimize code churn at this time. I suspect we're going to
>> want to remove most, if not all of them. But again, the goal in this
>> patch is to get the class structure in place with minimal amount of
>> collateral damage to make this step easy to review.
>>
>> It's worth noting you see no changes that bleed out of tree-vrp.c and
>> the vast majority of the patch is just changing the function signature
>> so that they're part of the newly created class. I consider that a
>> positive :-)
>
> FWIW, I like the "C++-ification" of the API.
>
> Just a few high-level suggestions:
>
> If class vr_values is meant to be copyable and assignable it
> should define a copy ctor and assignment operator. Otherwise
> it should make them private/deleted (whatever is the convention).
> As it is, it looks to me that if a copy is created by accident
> the dtor will have undefined behavior.
I'd actually like to defer finalizing this until a little bit later
(measured in days, not weeks or months).
Ideally we'd be using C++11 and rvalue references so that we could
transfer ownership of the array within the vr_values class as we move
from (in traditional vrp) propagation through the lattice to propagation
into the IL.
What I've got here in another branch slightly different in that the
ctors expect to be passed in the underlying array and we want to define
a private, empty copy ctor.
I'm still pondering whether to push C++11, go with what I've done on a
private branch or something else. It also plays into generally cleaning
up initialization and finalization.
>
> Also in vr_values, should all the data members really be public?
> (If the class maintains any invariants I would expect the members
> to be private and accessors provided for them to prevent clients
> from inadvertently breaking the invariants).
The amount of privatization one can do right now is minimal. I've
chosen to not do lots of rewriting to facilitate privatization of
methods and data at this stage. There's a little, but I've really tried
to keep this at a minimum to make the review step easier.
Once the class structure is in place and location of implementation is
rationalized it's easy to then start looking at each member with an eye
towards privatization. I've done some of that here on another branch.
Similarly one the classification and movement of methods is done, it's
easy to go back and review the remaining touch points to identify
targets for further isolation and classification work.
>
> Consider defining constructors and destructors for the classes
> instead of providing initialize and finialize member functions.
> (If a dtor takes an argument make it a member of the class.)
Absolutely. Though you'll find that ordering of initialization within
traditional vrp is painful. Again, I know this because I've done some
of this on another branch. You have to be careful about when you
instantiate the vr_values class relative to creating of assert_exprs
(which create additional SSA_NAMEs) relative to scev initialization and
initialization of the bb flags (inserting assert_exprs creates new
blocks too).
All this comes into play as one sets up ctors/dtors for the classes.
>
> Consider renaming the vrp_prop::vr_values data member so that
> it doesn't clash with the class name. Same in vrp_folder. There,
> I would also suggest to use a different name than in vrp_prop since
> the former is a pointer and the latter is an object of the type so
> it could get confusing.
Will certainly consider.
I think the general message is yes, we want to do all these things. But
we need to stage them in as independent cleanups to facilitate the
review process. It's also the case that this work often becomes easier
once a fair amount of the c++-ification is in-place.
Thanks,
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-09 19:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 22:27 [PATCH] 1/n Refactoring tree-vrp.c, step one introducing vr_values class Jeff Law
2017-11-08 13:14 ` Trevor Saunders
2017-11-09 15:27 ` Richard Biener
2017-11-09 18:47 ` Jeff Law
2017-11-09 18:41 ` Martin Sebor
2017-11-09 19:33 ` Jeff Law
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).