On 2/7/22 09:29, Andrew MacLeod wrote:
I have a proposal for PR 104288.
ALL patches (in sequence) bootstrap on their own and each cause no
regressions.
I've been continuing to work with this towards the GCC13 solution for
statement side effects. And There is another possibility we could
pursue which is less intrusive
I adapted the portions of patch 2/4 which process nonnull, but changes
it from being in the nonnull class to being in the cache.
THe main difference is it continues to use a single bit, just changing
all uses of it to *always* mean its non-null on exit to the block.
Range-on-entry is changed to only check dominator blocks, and
range_of_expr doesn't check the bit at all.. in both ranger and the cache.
When we are doing the block walk and process side effects, the on-entry
*cache* range for the block is adjusted to be non-null instead. The
statements which precede this stmt have already been processed, and all
remaining statements in this block will now pick up this new non-value
from the on-entry cache. This should be perfectly safe.
The final tweak is when range_of_expr is looking the def block, it
normally does not have an on entry cache value. (the def is in the
block, so there is no entry cache value). It now checks to see if we
have set one, which can only happen when we have been doing the
statement walk and set the on-entry cache with a non-null value. This
allows us to pick up the non-null range in the def block too... (once we
have passed a statement which set nonnull of course).
THis patch provides much less change to trunk, and is probably a better
approach as its closer to what is going to happen in GCC13.
Im still working with it, so the patch isnt fully refined yet... but it
bootstraps and passes all test with no regressions. And passes the new
tests too. I figured I would mention this before you look to hard at
the other patchset. the GCC11 patch doesn't change.
Let me know if you prefer this.... I think I do :-) less churn, and
closer to end state.
Andrew
commit da6185993cf85f38b08e4c2801ca8e5fe914e40d
Author: Andrew MacLeod <amacl...@redhat.com>
Date: Mon Feb 7 15:52:16 2022 -0500
Change nonnull to mean nonnull at exit.
* gimple-range-cache.cc (non_null_ref::set_nonnull): New.
(ranger_cache::range_of_def): Don't check non-null.
(ranger_cache::entry_range): Don't check non-null.
(ranger_cache::try_update_to_always_nonnull): New.
(ranger_cache::infer_nonnull_call_attributes): New.
(non_null_loadstore): New.
(ranger_cache::block_apply_nonnull): New.
* ranger-cache.h (class non_null_ref): Update prototypes.
(class ranger_cache): Update prototypes.
* gimple-range-path.cc (path_range_query::range_defined_in_block): Do
not search dominators.
(path_range_query::adjust_for_non_null_uses): Ditto.
* gimple-range.cc (gimple_ranger::range_of_expr): Check on-entry for
def overrides. Do not check nonnull.
(gimple_ranger::range_on_entry): Check dominators for nonnull.
(gimple_ranger::range_on_exit): Check for nonnull.
* gimple-range.cc (gimple_ranger::register_side_effects): New.
* gimple-range.h (gimple_ranger::register_side_effects): New.
* tree-vrp.cc (rvrp_folder::fold_stmt): Call register_side_effects.
diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 16c881b13e1..c6aefb98b42 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -29,6 +29,10 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-pretty-print.h"
#include "gimple-range.h"
#include "tree-cfg.h"
+#include "target.h"
+#include "attribs.h"
+#include "gimple-iterator.h"
+#include "gimple-walk.h"
#define DEBUG_RANGE_CACHE (dump_file \
&& (param_ranger_debug & RANGER_DEBUG_CACHE))
@@ -50,6 +54,22 @@ non_null_ref::~non_null_ref ()
m_nn.release ();
}
+// This routine will update NAME in BB to be nonnull if it is not already.
+// return TRUE if the update happens.,
+
+bool
+non_null_ref::set_nonnull (basic_block bb, tree name)
+{
+ gcc_checking_assert (gimple_range_ssa_p (name)
+ && POINTER_TYPE_P (TREE_TYPE (name)));
+ // Only process when its not already set.
+ if (non_null_deref_p (name, bb, false))
+ return false;
+ // From this point forward, we'll be NON-NULL for sure if we see one.
+ bitmap_set_bit (m_nn[SSA_NAME_VERSION (name)], bb->index);
+ return true;
+}
+
// Return true if NAME has a non-null dereference in block bb. If this is the
// first query for NAME, calculate the summary first.
// If SEARCH_DOM is true, the search the dominator tree as well.
@@ -1014,9 +1034,6 @@ ranger_cache::range_of_def (irange &r, tree name, basic_block bb)
else
r = gimple_range_global (name);
}
-
- if (bb)
- m_non_null.adjust_range (r, name, bb, false);
}
// Get the range of NAME as it occurs on entry to block BB.
@@ -1034,8 +1051,6 @@ ranger_cache::entry_range (irange &r, tree name, basic_block bb)
// Otherwise pick up the best available global value.
if (!m_on_entry.get_bb_range (r, name, bb))
range_of_def (r, name);
-
- m_non_null.adjust_range (r, name, bb, false);
}
// Get the range of NAME as it occurs on exit from block BB.
@@ -1467,3 +1482,107 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block bb)
}
return true;
}
+
+// This routine will update NAME in block BB to the nonnull state.
+// It will then update the on-netry cache for this block to be non-null
+// if it isn't already.
+
+void
+ranger_cache::try_update_to_always_nonnull (basic_block bb, tree name)
+{
+ if (gimple_range_ssa_p (name) && POINTER_TYPE_P (TREE_TYPE (name)))
+ {
+ m_non_null.set_nonnull (bb, name);
+ // Update the on-entry cache for BB to be non-zero
+ int_range_max r;
+ exit_range (r, name, bb);
+ if (m_non_null.adjust_range (r, name, bb, false))
+ m_on_entry.set_bb_range (name, bb, r);
+ }
+}
+
+// Adapted from infer_nonnull_range_by_attribute for calls to process
+// all operands which have the nonnull attribute set in stmt.
+
+void
+ranger_cache::infer_nonnull_call_attributes (gcall *stmt)
+{
+ if (gimple_call_internal_p (stmt))
+ return;
+
+ basic_block bb = gimple_bb (stmt);
+ tree fntype = gimple_call_fntype (stmt);
+ tree attrs = TYPE_ATTRIBUTES (fntype);
+ for (; attrs; attrs = TREE_CHAIN (attrs))
+ {
+ attrs = lookup_attribute ("nonnull", attrs);
+
+ /* If "nonnull" wasn't specified, we know nothing about
+ the argument. */
+ if (attrs == NULL_TREE)
+ return;
+
+ /* Check if "nonnull" applies to all the arguments. */
+ if (TREE_VALUE (attrs) == NULL_TREE)
+ {
+ for (unsigned int i = 0; i < gimple_call_num_args (stmt); i++)
+ {
+ tree op = gimple_call_arg (stmt, i);
+ try_update_to_always_nonnull (bb, op);
+ }
+ return;
+ }
+
+ /* Now see whats in this nonnull list. */
+ for (tree t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t))
+ {
+ unsigned int idx = TREE_INT_CST_LOW (TREE_VALUE (t)) - 1;
+ if (idx < gimple_call_num_args (stmt))
+ {
+ tree op = gimple_call_arg (stmt, idx);
+ try_update_to_always_nonnull (bb, op);
+ }
+ }
+ }
+}
+
+// Adapted from infer_nonnull_range_by_dereference and check_loadstore
+// to work for all operands.
+
+static bool
+non_null_loadstore (gimple *s, tree op, tree, void *data)
+{
+ if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
+ {
+ /* Some address spaces may legitimately dereference zero. */
+ addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op));
+ if (!targetm.addr_space.zero_address_valid (as))
+ {
+ tree ssa = TREE_OPERAND (op, 0);
+ basic_block bb = gimple_bb (s);
+ ((ranger_cache *)data)->try_update_to_always_nonnull (bb, ssa);
+ }
+ }
+ return false;
+}
+
+// This routine is used during a block walk to move the state of non-null for
+// any operands on stmt S to always nonnull.
+
+void
+ranger_cache::block_apply_nonnull (gimple *s)
+{
+ if (!flag_delete_null_pointer_checks)
+ return;
+ if (is_a<gphi *> (s))
+ return;
+ if (gimple_code (s) == GIMPLE_ASM || gimple_clobber_p (s))
+ return;
+ if (is_a<gcall *> (s))
+ {
+ infer_nonnull_call_attributes (as_a<gcall *> (s));
+ return;
+ }
+ walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
+ non_null_loadstore);
+}
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index b54b6882aa8..f189fd8898f 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -36,6 +36,7 @@ public:
bool non_null_deref_p (tree name, basic_block bb, bool search_dom = true);
bool adjust_range (irange &r, tree name, basic_block bb,
bool search_dom = true);
+ bool set_nonnull (basic_block bb, tree name);
private:
vec <bitmap> m_nn;
void process_name (tree name);
@@ -106,6 +107,8 @@ public:
void propagate_updated_value (tree name, basic_block bb);
+ void block_apply_nonnull (gimple *s);
+ void try_update_to_always_nonnull (basic_block bb, tree name);
non_null_ref m_non_null;
gori_compute m_gori;
@@ -118,6 +121,8 @@ private:
void fill_block_cache (tree name, basic_block bb, basic_block def_bb);
void propagate_cache (tree name);
+ void infer_nonnull_call_attributes (gcall *stmt);
+
void range_of_def (irange &r, tree name, basic_block bb = NULL);
void entry_range (irange &r, tree expr, basic_block bb);
void exit_range (irange &r, tree expr, basic_block bb);
diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index 3bf9bd1e981..483bcd2e582 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -358,7 +358,7 @@ path_range_query::range_defined_in_block (irange &r, tree name, basic_block bb)
}
if (bb)
- m_non_null.adjust_range (r, name, bb);
+ m_non_null.adjust_range (r, name, bb, false);
if (DEBUG_SOLVER && (bb || !r.varying_p ()))
{
@@ -528,7 +528,7 @@ path_range_query::adjust_for_non_null_uses (basic_block bb)
else
r.set_varying (TREE_TYPE (name));
- if (m_non_null.adjust_range (r, name, bb))
+ if (m_non_null.adjust_range (r, name, bb, false))
set_cache (r, name);
}
}
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 73398ddef99..bffda11a558 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-scalar-evolution.h"
#include "gimple-range.h"
#include "gimple-fold.h"
+#include "gimple-walk.h"
gimple_ranger::gimple_ranger () :
non_executable_edge_flag (cfun),
@@ -117,8 +118,9 @@ gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
// If name is defined in this block, try to get an range from S.
if (def_stmt && gimple_bb (def_stmt) == bb)
{
- range_of_stmt (r, def_stmt, expr);
- m_cache.m_non_null.adjust_range (r, expr, bb, true);
+ // Check for a defintion override from a block walk.
+ if (!m_cache.block_range (r, bb, expr, false))
+ range_of_stmt (r, def_stmt, expr);
}
// Otherwise OP comes from outside this block, use range on entry.
else
@@ -151,7 +153,12 @@ gimple_ranger::range_on_entry (irange &r, basic_block bb, tree name)
if (m_cache.block_range (entry_range, bb, name))
r.intersect (entry_range);
- m_cache.m_non_null.adjust_range (r, name, bb, true);
+ if (dom_info_available_p (CDI_DOMINATORS))
+ {
+ basic_block dom_bb = get_immediate_dominator (CDI_DOMINATORS, bb);
+ if (dom_bb)
+ m_cache.m_non_null.adjust_range (r, name, dom_bb, true);
+ }
if (idx)
tracer.trailer (idx, "range_on_entry", true, name, r);
@@ -185,6 +192,7 @@ gimple_ranger::range_on_exit (irange &r, basic_block bb, tree name)
range_of_expr (r, name, s);
else
range_on_entry (r, bb, name);
+ m_cache.m_non_null.adjust_range (r, name, bb, false);
gcc_checking_assert (r.undefined_p ()
|| range_compatible_p (r.type (), TREE_TYPE (name)));
@@ -436,6 +444,16 @@ gimple_ranger::fold_stmt (gimple_stmt_iterator *gsi, tree (*valueize) (tree))
return ret;
}
+// Called during dominator walks to register any side effects that take effect
+// from this point forward. Current release is only for tracking non-null
+// within a block.
+
+void
+gimple_ranger::register_side_effects (gimple *s)
+{
+ m_cache.block_apply_nonnull (s);
+}
+
// This routine will export whatever global ranges are known to GCC
// SSA_RANGE_NAME_INFO and SSA_NAME_PTR_INFO fields.
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index ec568153a8d..0733a534853 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -60,6 +60,7 @@ public:
void dump_bb (FILE *f, basic_block bb);
auto_edge_flag non_executable_edge_flag;
bool fold_stmt (gimple_stmt_iterator *gsi, tree (*) (tree));
+ void register_side_effects (gimple *s);
protected:
bool fold_range_internal (irange &r, gimple *s, tree name);
void prefill_name (irange &r, tree name);
diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
index 62946450b13..e9f19d0c8b9 100644
--- a/gcc/tree-vrp.cc
+++ b/gcc/tree-vrp.cc
@@ -4309,9 +4309,11 @@ public:
bool fold_stmt (gimple_stmt_iterator *gsi) OVERRIDE
{
- if (m_simplifier.simplify (gsi))
- return true;
- return m_ranger->fold_stmt (gsi, follow_single_use_edges);
+ bool ret = m_simplifier.simplify (gsi);
+ if (!ret)
+ ret = m_ranger->fold_stmt (gsi, follow_single_use_edges);
+ m_ranger->register_side_effects (gsi_stmt (*gsi));
+ return ret;
}
private:
