On 5/24/21 9:13 PM, Martin Sebor wrote:
On 5/21/21 5:39 AM, Aldy Hernandez via Gcc-patches wrote:
+ /* Range query mechanism for functions. The default is to pick up
+ global ranges. If a pass wants on-demand ranges OTOH, it must
+ call enable/disable_ranger(). */
+ range_query * GTY ((skip)) x_range_query;
+
Mostly out of curiosity: why the 'x_' prefix? (I'm used to seeing
that in the expansion of the option macros but it seems out of place
here.)
To disambiguate from the similarly named type. I thought of using
"query" for the field name, but that seemed too generic as to be confusing.
/* Last statement uid. */
int last_stmt_uid;
@@ -712,4 +718,14 @@ extern const char *current_function_name (void);
extern void used_types_insert (tree);
+extern range_query *get_global_range_query ();
+
+// Returns the currently active range access class. This is meant to
be used
+// with the `class range_query' API. When there is no active range
class,
+// global ranges are used.
+#define RANGE_QUERY(fun) (fun)->x_range_query
+
+// As above, but for accessing global ranges between passes.
+#define GLOBAL_RANGE_QUERY get_global_range_query ()
Unless there's some reason that escapes me, could these be functions
(possibly defined inline) rather than macros? (They're a lot easier
to work with, such as when I want to trace calls in some way.)
Sure, we could make it an inline function.
+
#endif /* GCC_FUNCTION_H */
diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 2c922e32913..c645d15f5af 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -384,7 +384,6 @@ block_range_cache::dump (FILE *f, basic_block bb,
bool print_varying)
ssa_global_cache::ssa_global_cache ()
{
m_tab.create (0);
- m_tab.safe_grow_cleared (num_ssa_names);
m_irange_allocator = new irange_allocator;
}
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 710bc7f9632..4e1aeee8ed0 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1402,3 +1402,133 @@ trace_ranger::range_of_expr (irange &r, tree
name, gimple *s)
return trailer (idx, "range_of_expr", res, name, r);
}
+
+// Return the legacy global range for NAME if it has one, otherwise
+// return VARYING.
I realize this is just being moved from elsewhere but I would suggest
to take this opportunity and also document the effects on the range
argument.
+
+static void
+get_range_global (irange &r, tree name)
+{
+ tree type = TREE_TYPE (name);
+
+ if (SSA_NAME_IS_DEFAULT_DEF (name))
+ {
+ tree sym = SSA_NAME_VAR (name);
+ // Adapted from vr_values::get_lattice_entry().
+ // Use a range from an SSA_NAME's available range.
+ if (TREE_CODE (sym) == PARM_DECL)
+ {
+ // Try to use the "nonnull" attribute to create ~[0, 0]
+ // anti-ranges for pointers. Note that this is only valid with
+ // default definitions of PARM_DECLs.
+ if (POINTER_TYPE_P (type)
+ && ((cfun && nonnull_arg_p (sym)) || get_ptr_nonnull (name)))
+ r.set_nonzero (type);
+ else if (INTEGRAL_TYPE_P (type))
+ {
+ get_range_info (name, r);
+ if (r.undefined_p ())
+ r.set_varying (type);
+ }
+ else
+ r.set_varying (type);
+ }
+ // If this is a local automatic with no definition, use undefined.
+ else if (TREE_CODE (sym) != RESULT_DECL)
+ r.set_undefined ();
+ else
+ r.set_varying (type);
+ }
+ else if (!POINTER_TYPE_P (type) && SSA_NAME_RANGE_INFO (name))
+ {
+ get_range_info (name, r);
+ if (r.undefined_p ())
+ r.set_varying (type);
+ }
+ else if (POINTER_TYPE_P (type) && SSA_NAME_PTR_INFO (name))
+ {
+ if (get_ptr_nonnull (name))
+ r.set_nonzero (type);
+ else
+ r.set_varying (type);
+ }
+ else
+ r.set_varying (type);
+}
+
+// ?? Like above, but only for default definitions of NAME.
I couldn't help but notice another variation on the dubious ???
yet wide-spread convention:
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570442.html
Per the comment in that message, my use indicates something that could
be done different but is not a bug.
This is
+// so VRP passes using ranger do not start with known ranges,
+// otherwise we'd eliminate builtin_unreachables too early because of
+// inlining.
+//
+// Without this restriction, the test in g++.dg/tree-ssa/pr61034.C has
+// all of its unreachable calls removed too early. We should
+// investigate whether we should just adjust the test above.
+
+value_range
+gimple_range_global (tree name)
+{
+ gcc_checking_assert (gimple_range_ssa_p (name));
+ tree type = TREE_TYPE (name);
+
+ if (SSA_NAME_IS_DEFAULT_DEF (name))
+ {
+ value_range vr;
+ get_range_global (vr, name);
+ return vr;
+ }
+ return value_range (type);
+}
+
+// ----------------------------------------------
+// global_range_query implementation.
+
+global_range_query global_ranges;
+
+range_query *
+get_global_range_query ()
+{
+ if (cfun)
+ return RANGE_QUERY (cfun);
+
+ return &global_ranges;
+}
+
+bool
+global_range_query::range_of_expr (irange &r, tree expr, gimple *)
+{
+ tree type = TREE_TYPE (expr);
+
+ if (!irange::supports_type_p (type) || !gimple_range_ssa_p (expr))
+ return get_tree_range (r, expr);
+
+ get_range_global (r, expr);
+
+ return true;
+}
+
+void
+enable_ranger ()
+{
+ gimple_ranger *r;
+
+ if (param_evrp_mode & EVRP_MODE_TRACE)
+ r = new trace_ranger;
+ else
+ r = new gimple_ranger;
+
+ RANGE_QUERY (cfun) = r;
+}
+
+void
+disable_ranger (bool export_ranges)
+{
+ gimple_ranger *r = (gimple_ranger *) RANGE_QUERY (cfun);
+
+ if (export_ranges)
+ r->export_global_ranges ();
+
+ delete r;
The above is a downcast from range_query*, so if the instance is
something else the code won't work correctly and might crash,
right? Or it might call the wrong function/dtor. Should there
be a check of some sort to make sure that doesn't happen?
Yeah, Andrew and I had argued about this. The use is confined to two
tiny functions that can be seen in one screenful
(enable/disable_ranger), and no one should be setting x_range_query
directly.
Aldy
