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. Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install? Richard 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. --- gcc/rtl-ssa/accesses.cc | 37 +++++++++++-------------- gcc/rtl-ssa/accesses.h | 3 +- gcc/rtl-ssa/internals.inl | 14 ++++++++++ gcc/testsuite/gcc.dg/torture/pr115928.c | 23 +++++++++++++++ 4 files changed, 55 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr115928.c diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc index 3f1304fc5bf..5cc05cb4be7 100644 --- 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 7d2916d00c2..27810a02063 100644 --- 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 c736877479e..9bc40bf35c5 100644 --- 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 index 00000000000..4379fa968ad --- /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; +} -- 2.25.1