[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
