rtl-ssa: Fix split_clobber_group [PR115928]

One of the goals of the rtl-ssa representation was to allow a
group of consecutive clobbers to be skipped in constant time,
with amortised sublinear insertion and deletion.  This involves
putting consecutive clobbers in groups.  Splitting or joining
groups would be linear if we had to update every clobber on
each update, so the operation to query a clobber's group is
lazy and (again) amortised sublinear.

This means that, when splitting a group into two, we cannot
reuse the old group for one side.  We have to invalidate it,
so that the lazy clobber_info::group query can tell that something
has changed.  The ICE in the PR came from failing to do that.

gcc/
	PR rtl-optimization/115928
	* rtl-ssa/accesses.h (clobber_group): Add a new constructor that
	takes the first, last and root clobbers.
	* rtl-ssa/internals.inl (clobber_group::clobber_group): Define it.
	* rtl-ssa/accesses.cc (function_info::split_clobber_group): Use it.
	Allocate a new group for both sides and invalidate the previous group.
	(function_info::add_def): After calling split_clobber_group,
	remove the old group from the splay tree.

gcc/testsuite/
	PR rtl-optimization/115928
	* gcc.dg/torture/pr115928.c: New test.

diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc
index 3f1304fc5bff26c19b60328b88c38a46d7e05663..5cc05cb4be7f02f3294af002ddeae2caf7ca750f 100644 (file)
--- a/gcc/rtl-ssa/accesses.cc
+++ b/gcc/rtl-ssa/accesses.cc
@@ -792,11 +792,11 @@ function_info::merge_clobber_groups (clobber_info *clobber1,
 }
 
 // GROUP spans INSN, and INSN now sets the resource that GROUP clobbers.
-// Split GROUP around INSN and return the clobber that comes immediately
-// before INSN.
+// Split GROUP around INSN, to form two new groups, and return the clobber
+// that comes immediately before INSN.
 //
 // The resource that GROUP clobbers is known to have an associated
-// splay tree.
+// splay tree.  The caller must remove GROUP from the tree on return.
 clobber_info *
 function_info::split_clobber_group (clobber_group *group, insn_info *insn)
 {
@@ -827,27 +827,20 @@ function_info::split_clobber_group (clobber_group *group, insn_info *insn)
       prev = as_a<clobber_info *> (next->prev_def ());
     }
 
-  // Use GROUP to hold PREV and earlier clobbers.  Create a new group for
-  // NEXT onwards.
+  // Create a new group for each side of the split.  We need to invalidate
+  // the old group so that clobber_info::group can tell whether a lazy
+  // update is needed.
+  clobber_info *first_clobber = group->first_clobber ();
   clobber_info *last_clobber = group->last_clobber ();
-  clobber_group *group1 = group;
-  clobber_group *group2 = allocate<clobber_group> (next);
-
-  // Finish setting up GROUP1, making sure that the roots and extremities
-  // have a correct group pointer.  Leave the rest to be updated lazily.
-  group1->set_last_clobber (prev);
-  tree1->set_group (group1);
-  prev->set_group (group1);
-
-  // Finish setting up GROUP2, with the same approach as for GROUP1.
-  group2->set_first_clobber (next);
-  group2->set_last_clobber (last_clobber);
-  next->set_group (group2);
-  tree2->set_group (group2);
-  last_clobber->set_group (group2);
+  auto *group1 = allocate<clobber_group> (first_clobber, prev, tree1.root ());
+  auto *group2 = allocate<clobber_group> (next, last_clobber, tree2.root ());
 
   // Insert GROUP2 into the splay tree as an immediate successor of GROUP1.
-  def_splay_tree::insert_child (group1, 1, group2);
+  def_splay_tree::insert_child (group, 1, group2);
+  def_splay_tree::insert_child (group, 1, group1);
+
+  // Invalidate the old group.
+  group->set_last_clobber (nullptr);
 
   return prev;
 }
@@ -952,6 +945,8 @@ function_info::add_def (def_info *def)
            }
          prev = split_clobber_group (group, insn);
          next = prev->next_def ();
+         tree.remove_root ();
+         last->set_splay_root (tree.root ());
        }
       // COMPARISON is < 0 if DEF comes before ROOT or > 0 if DEF comes
       // after ROOT.

diff --git a/gcc/rtl-ssa/accesses.h b/gcc/rtl-ssa/accesses.h
index 7d2916d00c2807e60ca82f28e139bf147a6fc9bd..27810a02063fd4eca3e5cc698ba4dfb3837725fc 100644 (file)
--- a/gcc/rtl-ssa/accesses.h
+++ b/gcc/rtl-ssa/accesses.h
@@ -937,7 +937,8 @@ public:
   void print (pretty_printer *pp) const;
 
 private:
-  clobber_group (clobber_info *clobber);
+  clobber_group (clobber_info *);
+  clobber_group (clobber_info *, clobber_info *, clobber_info *);
 
   // Set the values of first_clobber () and last_clobber ().
   void set_first_clobber (clobber_info *c) { m_clobber_or_set = c; }

diff --git a/gcc/rtl-ssa/internals.inl b/gcc/rtl-ssa/internals.inl
index c736877479e3a4393fceb5b3b3616dfca0c2930a..9bc40bf35c5b614c7c1801f648bec34e6162c6a8 100644 (file)
--- a/gcc/rtl-ssa/internals.inl
+++ b/gcc/rtl-ssa/internals.inl
@@ -380,6 +380,20 @@ inline clobber_group::clobber_group (clobber_info *clobber)
   clobber->m_group = this;
 }
 
+// Construct a new group of clobber_infos that spans [FIRST_CLOBBER,
+// LAST_CLOBBER].  Set the root of the splay tree to CLOBBER_TREE.
+inline clobber_group::clobber_group (clobber_info *first_clobber,
+                                    clobber_info *last_clobber,
+                                    clobber_info *clobber_tree)
+  : def_node (first_clobber),
+    m_last_clobber (last_clobber),
+    m_clobber_tree (clobber_tree)
+{
+  first_clobber->m_group = this;
+  last_clobber->m_group = this;
+  clobber_tree->m_group = this;
+}
+
 // Construct a node for the instruction with uid UID.
 inline insn_info::order_node::order_node (int uid)
   : insn_note (kind),

diff --git a/gcc/testsuite/gcc.dg/torture/pr115928.c b/gcc/testsuite/gcc.dg/torture/pr115928.c
new file mode 100644 (file)
index 0000000..4379fa9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr115928.c
@@ -0,0 +1,23 @@
+/* { dg-additional-options "-fgcse-sm" } */
+
+int a[1], b, c;
+struct {
+  int d;
+  int e;
+  int : 8;
+} f[1];
+static int g;
+char h, i, j;
+void k(int l) { b = 5 ^ a[b ^ (l & 5)]; }
+void m(long l) { k(c >> 6); }
+int main() {
+  g++;
+  if (b) {
+    h = 5 && j;
+    if (h)
+      h -= i;
+    m(f[g].d);
+    m(f[g].e);
+  }
+  return 0;
+}