[PATCH][v2] tree-optimization/115144 - improve sinking destination choice
When sinking code closer to its uses we already try to minimize the distance we move by inserting at the start of the basic-block. The following makes sure to sink closest to the control dependence check of the region we want to sink to as well as make sure to ignore control dependences that are only guarding exceptional code. This restores somewhat the old profile check but without requiring nearly even probabilities. The patch also makes sure to not give up completely when the best sink location is one we do not want to sink to but possibly then choose the next best one. This addresses fallout observed in building libgo. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/115144 * tree-ssa-sink.cc (do_not_sink): New function, split out from ... (select_best_block): Here. First pick valid block to sink to. From that search for the best valid block, avoiding sinking across conditions to exceptional code. (sink_code_in_bb): When updating vuses of stores in paths we do not sink a store to make sure we didn't pick a dominating sink location. * gcc.dg/tree-ssa/ssa-sink-22.c: New testcase. --- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c | 14 +++ gcc/tree-ssa-sink.cc| 106 +--- 2 files changed, 86 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c new file mode 100644 index 000..e35626d4070 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-sink1-details" } */ + +extern void abort (void); + +int foo (int x, int y, int f) +{ + int tem = x / y; + if (f) +abort (); + return tem; +} + +/* { dg-final { scan-tree-dump-not "Sinking" "sink1" } } */ diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc index 2188b7523c7..b0fe871cf1e 100644 --- a/gcc/tree-ssa-sink.cc +++ b/gcc/tree-ssa-sink.cc @@ -172,6 +172,39 @@ nearest_common_dominator_of_uses (def_operand_p def_p, bool *debug_stmts) return commondom; } +/* Return whether sinking STMT from EARLY_BB to BEST_BB should be avoided. */ + +static bool +do_not_sink (gimple *stmt, basic_block early_bb, basic_block best_bb) +{ + /* Placing a statement before a setjmp-like function would be invalid + (it cannot be reevaluated when execution follows an abnormal edge). + If we selected a block with abnormal predecessors, just punt. */ + if (bb_has_abnormal_pred (best_bb)) +return true; + + /* If the latch block is empty, don't make it non-empty by sinking + something into it. */ + if (best_bb == early_bb->loop_father->latch + && empty_block_p (best_bb)) +return true; + + /* Avoid turning an unconditional read into a conditional one when we + still might want to perform vectorization. */ + if (best_bb->loop_father == early_bb->loop_father + && loop_outer (best_bb->loop_father) + && !best_bb->loop_father->inner + && gimple_vuse (stmt) + && !gimple_vdef (stmt) + && flag_tree_loop_vectorize + && !(cfun->curr_properties & PROP_loop_opts_done) + && dominated_by_p (CDI_DOMINATORS, best_bb->loop_father->latch, early_bb) + && !dominated_by_p (CDI_DOMINATORS, best_bb->loop_father->latch, best_bb)) +return true; + + return false; +} + /* Given EARLY_BB and LATE_BB, two blocks in a path through the dominator tree, return the best basic block between them (inclusive) to place statements. @@ -185,54 +218,57 @@ select_best_block (basic_block early_bb, basic_block late_bb, gimple *stmt) { + /* First pick a block we do not disqualify. */ + while (late_bb != early_bb +&& do_not_sink (stmt, early_bb, late_bb)) +late_bb = get_immediate_dominator (CDI_DOMINATORS, late_bb); + basic_block best_bb = late_bb; basic_block temp_bb = late_bb; - while (temp_bb != early_bb) { /* Walk up the dominator tree, hopefully we'll find a shallower loop nest. */ temp_bb = get_immediate_dominator (CDI_DOMINATORS, temp_bb); + /* Do not consider blocks we do not want to sink to. */ + if (temp_bb != early_bb && do_not_sink (stmt, early_bb, temp_bb)) + ; + /* If we've moved into a lower loop nest, then that becomes our best block. */ - if (bb_loop_depth (temp_bb) < bb_loop_depth (best_bb)) + else if (bb_loop_depth (temp_bb) < bb_loop_depth (best_bb)) best_bb = temp_bb; -} - /* Placing a statement before a setjmp-like function would be invalid - (it cannot be reevaluated when execution follows an abnormal edge). - If we selected a block with abnormal predecessors, just punt. */ - if (bb_has_abnormal_pred (best_bb)) -return early_bb; - - /* If we found a
RE: [PATCH][v2] tree-optimization/115144 - improve sinking destination choice
Hi Richard, Looks this commit may result one ICE similar as below when build the newlib, feel free to ping me if you need one PR for this. CC RISC-V port for awareness. In file included from /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/stdlib/setenv_r.c:26: /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h: In function '_setenv_r': /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h:212:9: error: stmt with wrong VUSE 212 | int _setenv_r (struct _reent *, const char *__string, const char *__value, int __overwrite); | ^ # .MEM_109 = VDEF <.MEM_67> *C_59 = 61; expected .MEM_106 during GIMPLE pass: sink /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h:212:9: internal compiler error: verify_ssa failed Pan -Original Message- From: Richard Biener Sent: Friday, May 24, 2024 7:01 PM To: gcc-patches@gcc.gnu.org Subject: [PATCH][v2] tree-optimization/115144 - improve sinking destination choice When sinking code closer to its uses we already try to minimize the distance we move by inserting at the start of the basic-block. The following makes sure to sink closest to the control dependence check of the region we want to sink to as well as make sure to ignore control dependences that are only guarding exceptional code. This restores somewhat the old profile check but without requiring nearly even probabilities. The patch also makes sure to not give up completely when the best sink location is one we do not want to sink to but possibly then choose the next best one. This addresses fallout observed in building libgo. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/115144 * tree-ssa-sink.cc (do_not_sink): New function, split out from ... (select_best_block): Here. First pick valid block to sink to. From that search for the best valid block, avoiding sinking across conditions to exceptional code. (sink_code_in_bb): When updating vuses of stores in paths we do not sink a store to make sure we didn't pick a dominating sink location. * gcc.dg/tree-ssa/ssa-sink-22.c: New testcase. --- gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c | 14 +++ gcc/tree-ssa-sink.cc| 106 +--- 2 files changed, 86 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c new file mode 100644 index 000..e35626d4070 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-sink1-details" } */ + +extern void abort (void); + +int foo (int x, int y, int f) +{ + int tem = x / y; + if (f) +abort (); + return tem; +} + +/* { dg-final { scan-tree-dump-not "Sinking" "sink1" } } */ diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc index 2188b7523c7..b0fe871cf1e 100644 --- a/gcc/tree-ssa-sink.cc +++ b/gcc/tree-ssa-sink.cc @@ -172,6 +172,39 @@ nearest_common_dominator_of_uses (def_operand_p def_p, bool *debug_stmts) return commondom; } +/* Return whether sinking STMT from EARLY_BB to BEST_BB should be avoided. */ + +static bool +do_not_sink (gimple *stmt, basic_block early_bb, basic_block best_bb) +{ + /* Placing a statement before a setjmp-like function would be invalid + (it cannot be reevaluated when execution follows an abnormal edge). + If we selected a block with abnormal predecessors, just punt. */ + if (bb_has_abnormal_pred (best_bb)) +return true; + + /* If the latch block is empty, don't make it non-empty by sinking + something into it. */ + if (best_bb == early_bb->loop_father->latch + && empty_block_p (best_bb)) +return true; + + /* Avoid turning an unconditional read into a conditional one when we + still might want to perform vectorization. */ + if (best_bb->loop_father == early_bb->loop_father + && loop_outer (best_bb->loop_father) + && !best_bb->loop_father->inner + && gimple_vuse (stmt) + && !gimple_vdef (stmt) + && flag_tree_loop_vectorize + && !(cfun->curr_properties & PROP_loop_opts_done) + && dominated_by_p (CDI_DOMINATORS, best_bb->loop_father->latch, early_bb) + && !dominated_by_p (CDI_DOMINATORS, best_bb->loop_father->latch, best_bb)) +return true; + + return false; +} + /* Given EARLY_BB and LATE_BB, two blocks in a path through the dominator tree, return the best basic block between them (inclusive) to place statements. @@ -185,54 +218,57 @@ select_best_block (basic_block early_bb, basic_block late_bb,
Re: [PATCH][v2] tree-optimization/115144 - improve sinking destination choice
Relevant bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115220 Patrick On Sun, May 26, 2024 at 2:30 AM Li, Pan2 wrote: > > Hi Richard, > > Looks this commit may result one ICE similar as below when build the newlib, > feel free to ping me if you need one PR for this. > > CC RISC-V port for awareness. > > In file included from > /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/stdlib/setenv_r.c:26: > /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h: In > function '_setenv_r': > /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h:212:9: > error: stmt with wrong VUSE > 212 | int _setenv_r (struct _reent *, const char *__string, const char > *__value, int __overwrite); > | ^ > # .MEM_109 = VDEF <.MEM_67> > *C_59 = 61; > expected .MEM_106 > during GIMPLE pass: sink > /home/pli/gcc/111/riscv-gnu-toolchain/newlib/newlib/libc/include/stdlib.h:212:9: > internal compiler error: verify_ssa failed > > Pan > > > -Original Message- > From: Richard Biener > Sent: Friday, May 24, 2024 7:01 PM > To: gcc-patches@gcc.gnu.org > Subject: [PATCH][v2] tree-optimization/115144 - improve sinking destination > choice > > When sinking code closer to its uses we already try to minimize the > distance we move by inserting at the start of the basic-block. The > following makes sure to sink closest to the control dependence > check of the region we want to sink to as well as make sure to > ignore control dependences that are only guarding exceptional code. > This restores somewhat the old profile check but without requiring > nearly even probabilities. The patch also makes sure to not give > up completely when the best sink location is one we do not want to > sink to but possibly then choose the next best one. > > This addresses fallout observed in building libgo. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. > > PR tree-optimization/115144 > * tree-ssa-sink.cc (do_not_sink): New function, split out > from ... > (select_best_block): Here. First pick valid block to > sink to. From that search for the best valid block, > avoiding sinking across conditions to exceptional code. > (sink_code_in_bb): When updating vuses of stores in > paths we do not sink a store to make sure we didn't > pick a dominating sink location. > > * gcc.dg/tree-ssa/ssa-sink-22.c: New testcase. > --- > gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c | 14 +++ > gcc/tree-ssa-sink.cc| 106 +--- > 2 files changed, 86 insertions(+), 34 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c > new file mode 100644 > index 000..e35626d4070 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-22.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-sink1-details" } */ > + > +extern void abort (void); > + > +int foo (int x, int y, int f) > +{ > + int tem = x / y; > + if (f) > +abort (); > + return tem; > +} > + > +/* { dg-final { scan-tree-dump-not "Sinking" "sink1" } } */ > diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc > index 2188b7523c7..b0fe871cf1e 100644 > --- a/gcc/tree-ssa-sink.cc > +++ b/gcc/tree-ssa-sink.cc > @@ -172,6 +172,39 @@ nearest_common_dominator_of_uses (def_operand_p def_p, > bool *debug_stmts) >return commondom; > } > > +/* Return whether sinking STMT from EARLY_BB to BEST_BB should be avoided. > */ > + > +static bool > +do_not_sink (gimple *stmt, basic_block early_bb, basic_block best_bb) > +{ > + /* Placing a statement before a setjmp-like function would be invalid > + (it cannot be reevaluated when execution follows an abnormal edge). > + If we selected a block with abnormal predecessors, just punt. */ > + if (bb_has_abnormal_pred (best_bb)) > +return true; > + > + /* If the latch block is empty, don't make it non-empty by sinking > + something into it. */ > + if (best_bb == early_bb->loop_father->latch > + && empty_block_p (best_bb)) > +return true; > + > + /* Avoid turning an unconditional read into a conditional one when we > + still might want to perform vectorization. */ > + if (best_bb->loop_father == early_bb->loop_father > + && loop_outer (best_bb->loop_father) > + && !best_bb->loop_father->i