| Message ID | mpt5xt5jthe.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [pushed] rtl-ssa: Fix removal of order_nodes [PR115929] | expand |
On 7/16/24 8:34 AM, Richard Sandiford wrote: > order_nodes are used to implement ordered comparisons between > two insns with the same program point number. remove_insn would > remove an order_node from its splay tree, but didn't remove it > from the insn. This caused confusion if the insn was later > reinserted somewhere else that also needed an order_node. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. Pushed as obvious. > > Richard > > > gcc/ > PR rtl-optimization/115929 > * rtl-ssa/insns.cc (function_info::remove_insn): Remove an > order_node from the instruction as well as from the splay tree. > > gcc/testsuite/ > PR rtl-optimization/115929 > * gcc.dg/torture/pr115929-1.c: New test. OK. I do find myself wondering if you should just be pushing these patches. You know this code far better than anyone. jeff
Am 16.07.24 um 16:34 schrieb Richard Sandiford: > order_nodes are used to implement ordered comparisons between > two insns with the same program point number. remove_insn would > remove an order_node from its splay tree, but didn't remove it > from the insn. This caused confusion if the insn was later > reinserted somewhere else that also needed an order_node. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. Pushed as obvious. > > Richard > > > gcc/ > PR rtl-optimization/115929 > * rtl-ssa/insns.cc (function_info::remove_insn): Remove an > order_node from the instruction as well as from the splay tree. > > gcc/testsuite/ > PR rtl-optimization/115929 > * gcc.dg/torture/pr115929-1.c: New test. > --- > gcc/rtl-ssa/insns.cc | 5 ++- > gcc/testsuite/gcc.dg/torture/pr115929-1.c | 45 +++++++++++++++++++++++ > 2 files changed, 49 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr115929-1.c > > diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc > index 7e26bfd978f..bc30734df89 100644 > --- a/gcc/rtl-ssa/insns.cc > +++ b/gcc/rtl-ssa/insns.cc > @@ -393,7 +393,10 @@ void > function_info::remove_insn (insn_info *insn) > { > if (insn_info::order_node *order = insn->get_order_node ()) > - insn_info::order_splay_tree::remove_node (order); > + { > + insn_info::order_splay_tree::remove_node (order); > + insn->remove_note (order); > + } > > if (auto *note = insn->find_note<insn_call_clobbers_note> ()) > { > diff --git a/gcc/testsuite/gcc.dg/torture/pr115929-1.c b/gcc/testsuite/gcc.dg/torture/pr115929-1.c > new file mode 100644 > index 00000000000..19b831ab99e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr115929-1.c > @@ -0,0 +1,45 @@ > +/* { dg-require-effective-target lp64 } */ > +/* { dg-options "-fno-gcse -fschedule-insns -fno-guess-branch-probability -fno-tree-fre -fno-tree-ch" } */ Hi, this fails on machines that don't support scheduling: cc1: warning: instruction scheduling not supported on this target machine FAIL: gcc.dg/torture/pr115929-2.c -O0 (test for excess errors) Excess errors: cc1: warning: instruction scheduling not supported on this target machine Better do /* { dg-require-effective-target scheduling } */ Johann > + > +int printf(const char *, ...); > +int a[6], b, c; > +char d, l; > +struct { > + char e; > + int f; > + int : 8; > + long g; > + long h; > +} i[1][9] = {0}; > +unsigned j; > +void n(char p) { b = b >> 8 ^ a[b ^ p]; } > +int main() { > + int k, o; > + while (b) { > + k = 0; > + for (; k < 9; k++) { > + b = b ^ a[l]; > + n(j); > + if (o) > + printf(&d); > + long m = i[c][k].f; > + b = b >> 8 ^ a[l]; > + n(m >> 32); > + n(m); > + if (o) > + printf("%d", d); > + b = b >> 8 ^ l; > + n(2); > + n(0); > + if (o) > + printf(&d); > + b = b ^ a[l]; > + n(i[c][k].g >> 2); > + n(i[c][k].g); > + if (o) > + printf(&d); > + printf("%d", i[c][k].f); > + } > + } > + return 0; > +}
diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc index 7e26bfd978f..bc30734df89 100644 --- a/gcc/rtl-ssa/insns.cc +++ b/gcc/rtl-ssa/insns.cc @@ -393,7 +393,10 @@ void function_info::remove_insn (insn_info *insn) { if (insn_info::order_node *order = insn->get_order_node ()) - insn_info::order_splay_tree::remove_node (order); + { + insn_info::order_splay_tree::remove_node (order); + insn->remove_note (order); + } if (auto *note = insn->find_note<insn_call_clobbers_note> ()) { diff --git a/gcc/testsuite/gcc.dg/torture/pr115929-1.c b/gcc/testsuite/gcc.dg/torture/pr115929-1.c new file mode 100644 index 00000000000..19b831ab99e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr115929-1.c @@ -0,0 +1,45 @@ +/* { dg-require-effective-target lp64 } */ +/* { dg-options "-fno-gcse -fschedule-insns -fno-guess-branch-probability -fno-tree-fre -fno-tree-ch" } */ + +int printf(const char *, ...); +int a[6], b, c; +char d, l; +struct { + char e; + int f; + int : 8; + long g; + long h; +} i[1][9] = {0}; +unsigned j; +void n(char p) { b = b >> 8 ^ a[b ^ p]; } +int main() { + int k, o; + while (b) { + k = 0; + for (; k < 9; k++) { + b = b ^ a[l]; + n(j); + if (o) + printf(&d); + long m = i[c][k].f; + b = b >> 8 ^ a[l]; + n(m >> 32); + n(m); + if (o) + printf("%d", d); + b = b >> 8 ^ l; + n(2); + n(0); + if (o) + printf(&d); + b = b ^ a[l]; + n(i[c][k].g >> 2); + n(i[c][k].g); + if (o) + printf(&d); + printf("%d", i[c][k].f); + } + } + return 0; +}