diff mbox series

[2/4] tree-optimization/104288 - Register side effects during ranger vrp domwalk.

Message ID 53667fe7-7f67-8366-8363-ede9c3aebcb8@redhat.com
State New
Headers show
Series tree-optimization/104288 - Add more granularity to non-null tracking | expand

Commit Message

Andrew MacLeod Feb. 7, 2022, 2:29 p.m. UTC 
This patch adds the ability to register side effects within a block to 
ranger. This is currently only for tracking non-null values.

the rvrp_folder performs a DOM walk and then attempts to simplify and 
the fold each stmt during the walk.  This patch adds a call to 
register_side_effects() to record any effects the stmt might have before 
returning.

This checks if the non-null property is set for any ssa-names on the 
statement, and if they are, moves the state to only non-null for the 
block...  This allows us to adjust the property within the block as we 
do the DOM walk.  All further queries within the block will then come 
back as non-null.   ALl other queries will be from outside the block, so 
they will see the same results anyway.

Unfortunately, none of the non-null routines in gimple.cc appear to work 
"in bulk", but rather, on a specific tree-ssa operand request at a 
time.  For this patch, I need to scan the entire statement looking for 
any operand that is nonnull (or the performance impact is awful).

I took the code in the various infer_nonnull routines, and packed it all 
together into the two routines block_apply_nonnull && 
infer_nonnull_call_attributes,  which basically perform the same 
functionality as  infer_nonnull_range_by_dereference and 
infer_nonnull_range_by_attribute, only on all the operands at once.  I 
think its right, but you may want to have a look.   I intend to leverage 
this code in GCC13  for a more generalized range_after_stmt mechanism 
that will do away with the immediate-use visits of the current non-null

This patch also has zero effect on generated code as none of the 
nonnull_deref_p() queries make use of the new granularity available 
yet.  It does set the table for the next patch. Performance impact of 
this is also marginal, about a 1% hit to EVRP/VRP.

Bootstraps with no regressions.  OK for trunk?

Andrew

Comments

Richard Biener Feb. 8, 2022, 8:14 a.m. UTC | #1 
On Mon, Feb 7, 2022 at 3:32 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch adds the ability to register side effects within a block to
> ranger. This is currently only for tracking non-null values.
>
> the rvrp_folder performs a DOM walk and then attempts to simplify and
> the fold each stmt during the walk.  This patch adds a call to
> register_side_effects() to record any effects the stmt might have before
> returning.
>
> This checks if the non-null property is set for any ssa-names on the
> statement, and if they are, moves the state to only non-null for the
> block...  This allows us to adjust the property within the block as we
> do the DOM walk.  All further queries within the block will then come
> back as non-null.   ALl other queries will be from outside the block, so
> they will see the same results anyway.
>
> Unfortunately, none of the non-null routines in gimple.cc appear to work
> "in bulk", but rather, on a specific tree-ssa operand request at a
> time.  For this patch, I need to scan the entire statement looking for
> any operand that is nonnull (or the performance impact is awful).

In tree.cc we have get_nonnull_args which you'd pass the
gimple_call_fntype, its expense is allocating a bitmap (if any arg is nonnull)
which you'd then need to iterate over and free.  I think that's an OK
overhead for not duplicating the walk?

> I took the code in the various infer_nonnull routines, and packed it all
> together into the two routines block_apply_nonnull &&
> infer_nonnull_call_attributes,  which basically perform the same
> functionality as  infer_nonnull_range_by_dereference and
> infer_nonnull_range_by_attribute, only on all the operands at once.  I
> think its right, but you may want to have a look.   I intend to leverage
> this code in GCC13  for a more generalized range_after_stmt mechanism
> that will do away with the immediate-use visits of the current non-null

+void
+non_null_ref::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);

Both ASM and calls can have pointer dereferences embedded so
I wonder whether you should do the walk_stmt_load_store_ops
anyway.  Note that any exceptional control flow (EH or abnormal)
will not have realized the outputs on a stmt (LHS of call, outputs
of asms), so nonnull carries over only across fallthru or conditional
edges.

I suppose realizing the LHS could be postponed for GCC 13, for
calls you could still walk loads so you get the non-nullness of
aggregate arguments like foo (*p) after the stmt.

But you can of course do all this for GCC 13 only.

> This patch also has zero effect on generated code as none of the
> nonnull_deref_p() queries make use of the new granularity available
> yet.  It does set the table for the next patch. Performance impact of
> this is also marginal, about a 1% hit to EVRP/VRP.
>
> Bootstraps with no regressions.  OK for trunk?
>
> Andrew
>
>
>
diff mbox series

Patch

From c4a1e28b5b1dd8f22e7d87f34961596a816c7ad7 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Wed, 2 Feb 2022 14:44:57 -0500
Subject: [PATCH 2/3] Register side effects during ranger block walk.

This patch adds the ability to register statement side effects within a block
for ranger.  This is currently only for tracking non-null values.

	* gimple-range-cache.cc (non_null_ref::try_update_to_always_nonnull):
	New.
	(non_null_ref::infer_nonnull_call_attributes): New.
	(non_null_loadstore): New.
	(non_null_ref::block_apply_nonnull): New.
	* gimple-range-cache.h (class non_null_ref): New prototypes.
	* 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.
---
 gcc/gimple-range-cache.cc | 106 ++++++++++++++++++++++++++++++++++++++
 gcc/gimple-range-cache.h  |   3 ++
 gcc/gimple-range.cc       |  11 ++++
 gcc/gimple-range.h        |   1 +
 gcc/tree-vrp.cc           |   8 +--
 5 files changed, 126 insertions(+), 3 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 52094f0bc6a..d14dc1284f2 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))
@@ -197,6 +201,108 @@  non_null_ref::process_name (tree name)
     }
 }
 
+// This routine will update NAME on stmt S to always_nonnull state, if it
+// is in contains_nonull state.  This is used during a block walk to move
+// NAME to known nonnull.
+
+void
+non_null_ref::try_update_to_always_nonnull (gimple *s, tree name)
+{
+  if (gimple_range_ssa_p (name) && POINTER_TYPE_P (TREE_TYPE (name)))
+    {
+      unsigned long state = get_bb_state (name, gimple_bb (s));
+      // Only process when both conditions exist in the BB.
+      if (state != NN_BOTH)
+	return;
+      // From this point forward, we'll be NON-NULL for sure if we see one.
+      set_bb_state (name, gimple_bb (s), NN_NON_ZERO);
+    }
+}
+
+// Adapted from infer_nonnull_range_by_attribute for calls to process
+// all operands which have the nonnull attribute set in stmt.
+
+void
+non_null_ref::infer_nonnull_call_attributes (gcall *stmt)
+{
+  if (gimple_call_internal_p (stmt))
+    return;
+
+  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 (stmt, 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 (stmt, 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);
+	  ((non_null_ref *)data)->try_update_to_always_nonnull (s, 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
+non_null_ref::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);
+}
+
 // -------------------------------------------------------------------------
 
 // This class represents the API into a cache of ranges for an SSA_NAME.
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index 2e61824593e..0d073213226 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -36,9 +36,12 @@  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);
+  void block_apply_nonnull (gimple *s);
+  void try_update_to_always_nonnull (gimple *s, tree name);
 private:
   vec <bitmap> m_nn;
   void process_name (tree name);
+  void infer_nonnull_call_attributes (gcall *stmt);
   unsigned long get_bb_state (tree name, basic_block bb);
   void set_bb_state (tree name, basic_block bb, unsigned long);
   bitmap_obstack m_bitmaps;
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 73398ddef99..eb599d7f0d9 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),
@@ -436,6 +437,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.m_non_null.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:
-- 
2.17.2