Re: [PATCH] tree-optimization/109524 - ICE with VRP edge removal

Re: [PATCH] tree-optimization/109524 - ICE with VRP edge removal

[Jakub Jelinek]

On Mon, Apr 17, 2023 at 08:41:38AM +0000, Richard Biener wrote:
> VRP queues edges to process late for updating global ranges for
> __builtin_unreachable.  But this interferes with edge removal
> from substitute_and_fold.  The following deals with this by
> looking up the edge with source/dest block indices which do not
> become stale.
> 
> For GCC 14 we probably want to refactor substitute_and_fold but
> that doesn't seem appropriate at this stage.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/109524
> 	* tree-vrp.cc (remove_unreachable::m_list): Change to a
> 	vector of pairs of block indices.
> 	(remove_unreachable::maybe_register_block): Adjust.
> 	(remove_unreachable::remove_and_update_globals): Likewise.
> 	Deal with removed blocks.
> 
> 	* g++.dg/pr109524.C: New testcase.

Ok, thanks.
> --- a/gcc/tree-vrp.cc
> +++ b/gcc/tree-vrp.cc
> @@ -75,7 +75,7 @@ public:
>    ~remove_unreachable () { m_list.release (); }
>    void maybe_register_block (basic_block bb);
>    bool remove_and_update_globals (bool final_p);
> -  vec<edge> m_list;
> +  vec<std::pair<int, int> > m_list;

We are in C++11, you could use >> instead of > > here I think.

	Jakub

[PATCH] tree-optimization/109524 - ICE with VRP edge removal

[Richard Biener]

VRP queues edges to process late for updating global ranges for
__builtin_unreachable.  But this interferes with edge removal
from substitute_and_fold.  The following deals with this by
looking up the edge with source/dest block indices which do not
become stale.

For GCC 14 we probably want to refactor substitute_and_fold but
that doesn't seem appropriate at this stage.

Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?

Thanks,
Richard.

	PR tree-optimization/109524
	* tree-vrp.cc (remove_unreachable::m_list): Change to a
	vector of pairs of block indices.
	(remove_unreachable::maybe_register_block): Adjust.
	(remove_unreachable::remove_and_update_globals): Likewise.
	Deal with removed blocks.

	* g++.dg/pr109524.C: New testcase.
---
 gcc/testsuite/g++.dg/pr109524.C | 41 +++++++++++++++++++++++++++++++++
 gcc/tree-vrp.cc                 | 13 +++++++----
 2 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr109524.C

diff --git a/gcc/testsuite/g++.dg/pr109524.C b/gcc/testsuite/g++.dg/pr109524.C
new file mode 100644
index 00000000000..a560839cd5b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr109524.C
@@ -0,0 +1,41 @@
+// { dg-do compile }
+// { dg-require-effective-target c++11 }
+// { dg-options "-O3 -fno-tree-forwprop -fnon-call-exceptions -fno-tree-ccp -fno-tree-fre" }
+
+struct nn;
+void f();
+int *m;
+struct _Guard {
+        ~_Guard() {
+          if (_M_guarded)
+                  __builtin_unreachable();
+        }
+        nn *_M_guarded;
+};
+struct nn {
+  int * _M_dataplus;
+  nn(nn &)
+        {
+                f();
+                _Guard   __guard;
+                m = _M_dataplus;
+        }
+  nn(){}
+};
+  void hnn(nn *a)
+        {
+                f();
+                _Guard   __guard;
+                m = a->_M_dataplus;
+        }
+bool gg();
+static inline nn
+hh(nn str) {
+  if (gg())
+    return str;
+  __builtin_unreachable();
+}
+void h() {
+
+  hh({});
+}
diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
index be7d06f565c..f4d484526c7 100644
--- a/gcc/tree-vrp.cc
+++ b/gcc/tree-vrp.cc
@@ -75,7 +75,7 @@ public:
   ~remove_unreachable () { m_list.release (); }
   void maybe_register_block (basic_block bb);
   bool remove_and_update_globals (bool final_p);
-  vec<edge> m_list;
+  vec<std::pair<int, int> > m_list;
   gimple_ranger &m_ranger;
 };
 
@@ -103,9 +103,9 @@ remove_unreachable::maybe_register_block (basic_block bb)
     return;
 
   if (un0)
-    m_list.safe_push (e1);
+    m_list.safe_push (std::make_pair (e1->src->index, e1->dest->index));
   else
-    m_list.safe_push (e0);
+    m_list.safe_push (std::make_pair (e0->src->index, e0->dest->index));
 }
 
 // Process the edges in the list, change the conditions and removing any
@@ -132,7 +132,12 @@ remove_unreachable::remove_and_update_globals (bool final_p)
   auto_bitmap all_exports;
   for (i = 0; i < m_list.length (); i++)
     {
-      edge e = m_list[i];
+      auto eb = m_list[i];
+      basic_block src = BASIC_BLOCK_FOR_FN (cfun, eb.first);
+      basic_block dest = BASIC_BLOCK_FOR_FN (cfun, eb.second);
+      if (!src || !dest)
+	continue;
+      edge e = find_edge (src, dest);
       gimple *s = gimple_outgoing_range_stmt_p (e->src);
       gcc_checking_assert (gimple_code (s) == GIMPLE_COND);
       bool lhs_p = TREE_CODE (gimple_cond_lhs (s)) == SSA_NAME;
-- 
2.35.3