[PATCH 4/4] [GCC11] tree-optimization/104288 - range on entry should check dominators for non-null.

[PATCH 4/4] [GCC11] tree-optimization/104288 - range on entry should check dominators for non-null.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

The patches resolves the issue for GCC11 in a much simpler way.

By default, ranger and EVRP are running in hybrid mode. This means if 
ranger misses something, evrp will pick up the slack. This enables us to 
change the 2 places which check for non-null to ignore potentially 
incorrect block-wide information and only query dominator blocks for 
on-entry ranges.  This allows ranger to be conservative, and EVRP will 
pick up the intra-block changes.

Bootstraps with no regressions.  OK for gcc 11 branch?



[-- Attachment #2: 0004.patch --]
[-- Type: text/x-patch, Size: 4653 bytes --]

From b807249b310153349a2593203eba44c456b6208e Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Mon, 31 Jan 2022 11:37:16 -0500
Subject: [PATCH] Range on entry should only check dominators for non-null.

Range-on-entry checks should no check the state of non-null within the current
block.   If dominators are present, use the dominator.

	PR tree-optimization/104288
	gcc/
	* gimple-range-cache.cc (ssa_range_in_bb): Only use non-null from the
	dominator entry ranges.
	* gimple-range.cc (gimple_ranger::range_of_expr): Ditto.
	gcc/testsuite/
	* gcc.dg/pr104288.c: New.
---
 gcc/gimple-range-cache.cc       | 19 ++++++++++++-------
 gcc/gimple-range.cc             | 16 ++++++++++------
 gcc/testsuite/gcc.dg/pr104288.c | 23 +++++++++++++++++++++++
 3 files changed, 45 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr104288.c

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 6ddeca3766d..b05b804d513 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -905,14 +905,19 @@ ranger_cache::ssa_range_in_bb (irange &r, tree name, basic_block bb)
       // Try to pick up any known global value as a best guess for now.
       if (!m_globals.get_global_range (r, name))
 	r = gimple_range_global (name);
+      // Check for non-null on entry if dominators are available by
+      // resetting def_bb to the block we want to search.
+      if (dom_info_available_p (CDI_DOMINATORS))
+	{
+	  basic_block dom_bb = get_immediate_dominator (CDI_DOMINATORS, bb);
+	  // Check if pointers have any non-null dereferences.  Non-call
+	  // exceptions mean we could throw in the middle of the block, so just
+	  // punt for now on those.
+	  if (dom_bb && r.varying_p () && !cfun->can_throw_non_call_exceptions
+	      && m_non_null.non_null_deref_p (name, dom_bb))
+	    r = range_nonzero (TREE_TYPE (name));
+	}
     }
-
-  // Check if pointers have any non-null dereferences.  Non-call
-  // exceptions mean we could throw in the middle of the block, so just
-  // punt for now on those.
-  if (r.varying_p () && m_non_null.non_null_deref_p (name, bb) &&
-      !cfun->can_throw_non_call_exceptions)
-    r = range_nonzero (TREE_TYPE (name));
 }
 
 // Return a static range for NAME on entry to basic block BB in R.  If
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index f861459ed96..42c637458ad 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -979,10 +979,14 @@ 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);
-  else
-    // Otherwise OP comes from outside this block, use range on entry.
-    range_on_entry (r, bb, expr);
+    return range_of_stmt (r, def_stmt, expr);
+
+  // Otherwise OP comes from outside this block, use range on entry.
+  range_on_entry (r, bb, expr);
+  // Check for non-null in the predecessor if dominators are available.
+  if (!dom_info_available_p (CDI_DOMINATORS))
+    return true;
+  basic_block dom_bb = get_immediate_dominator (CDI_DOMINATORS, bb);
 
   // No range yet, see if there is a dereference in the block.
   // We don't care if it's between the def and a use within a block
@@ -992,8 +996,8 @@ gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
   // in which case we may need to walk from S back to the def/top of block
   // to make sure the deref happens between S and there before claiming
   // there is a deref.   Punt for now.
-  if (!cfun->can_throw_non_call_exceptions && r.varying_p () &&
-      m_cache.m_non_null.non_null_deref_p (expr, bb))
+  if (dom_bb && !cfun->can_throw_non_call_exceptions && r.varying_p ()
+      && m_cache.m_non_null.non_null_deref_p (expr, dom_bb))
     r = range_nonzero (TREE_TYPE (expr));
 
   return true;
diff --git a/gcc/testsuite/gcc.dg/pr104288.c b/gcc/testsuite/gcc.dg/pr104288.c
new file mode 100644
index 00000000000..95eb196f9e4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104288.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp -fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" { keeps_null_pointer_checks } } */
+
+void keep(int result) __attribute__((noipa));
+void keep(int result)
+{
+    if (result)
+        __builtin_exit(0);
+}
+
+void foo (void *p) __attribute__((nonnull(1)));
+
+void bar (void *p)
+{
+  keep (p == 0);
+  foo (p);
+  if (!p)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "evrp" } } */
+/* { dg-final { scan-tree-dump-times  "== 0B;" 1 "evrp" } } */
-- 
2.17.2

Re: [PATCH 4/4] [GCC11] tree-optimization/104288 - range on entry should check dominators for non-null.

[Richard Biener]

On Mon, Feb 7, 2022 at 3:34 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The patches resolves the issue for GCC11 in a much simpler way.
>
> By default, ranger and EVRP are running in hybrid mode. This means if
> ranger misses something, evrp will pick up the slack. This enables us to
> change the 2 places which check for non-null to ignore potentially
> incorrect block-wide information and only query dominator blocks for
> on-entry ranges.  This allows ranger to be conservative, and EVRP will
> pick up the intra-block changes.
>
> Bootstraps with no regressions.  OK for gcc 11 branch?

OK.

Thanks,
Richard.

>
>