https://gcc.gnu.org/g:9b671c0fdb80d9fd8a60030e7cc9550e903c4fa3
commit 9b671c0fdb80d9fd8a60030e7cc9550e903c4fa3
Author: Alexandre Oliva <ol...@gnu.org>
Date: Thu Nov 28 21:56:34 2024 -0300
ifcombine: avoid forwarders with intervening blocks
Diff:
---
gcc/testsuite/gcc.dg/ifcmb-1.c | 60 ++++++++++++++++++++++++++++++++++++
gcc/tree-ssa-ifcombine.cc | 69 ++++++++++++++++++++++++++++++++++++------
2 files changed, 119 insertions(+), 10 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/ifcmb-1.c b/gcc/testsuite/gcc.dg/ifcmb-1.c
new file mode 100644
index 000000000000..9aaba4de5328
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ifcmb-1.c
@@ -0,0 +1,60 @@
+/* { dg-do run } */
+
+[[gnu::noinline]]
+int f0 (int a, int b) {
+ if ((a & 1))
+ return 0;
+ if (b)
+ return 1;
+ if (!(a & 2))
+ return 0;
+ else
+ return 1;
+}
+
+[[gnu::noinline]]
+int f1 (int a, int b) {
+ if (!(a & 1))
+ return 0;
+ if (b)
+ return 1;
+ if ((a & 2))
+ return 1;
+ else
+ return 0;
+}
+
+[[gnu::noinline]]
+int f2 (int a, int b) {
+ if ((a & 1))
+ return 0;
+ if (b)
+ return 1;
+ if (!(a & 2))
+ return 0;
+ else
+ return 1;
+}
+
+[[gnu::noinline]]
+int f3 (int a, int b) {
+ if (!(a & 1))
+ return 0;
+ if (b)
+ return 1;
+ if ((a & 2))
+ return 1;
+ else
+ return 0;
+}
+
+int main() {
+ if (f0 (0, 1) != 1)
+ __builtin_abort();
+ if (f1 (1, 1) != 1)
+ __builtin_abort();
+ if (f2 (2, 1) != 1)
+ __builtin_abort();
+ if (f3 (3, 1) != 1)
+ __builtin_abort();
+}
diff --git a/gcc/tree-ssa-ifcombine.cc b/gcc/tree-ssa-ifcombine.cc
index b58f63f4707a..fd7f3c245254 100644
--- a/gcc/tree-ssa-ifcombine.cc
+++ b/gcc/tree-ssa-ifcombine.cc
@@ -1089,7 +1089,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb,
basic_block outer_cond_bb,
}
/* The || form is characterized by a common then_bb with the
- two edges leading to it mergable. The latter is guaranteed
+ two edges leading to it mergeable. The latter is guaranteed
by matching PHI arguments in the then_bb and the inner cond_bb
having no side-effects. */
if (phi_pred_bb != then_bb
@@ -1100,7 +1100,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb,
basic_block outer_cond_bb,
<outer_cond_bb>
if (q) goto then_bb; else goto inner_cond_bb;
<inner_cond_bb>
- if (q) goto then_bb; else goto ...;
+ if (p) goto then_bb; else goto ...;
<then_bb>
...
*/
@@ -1116,7 +1116,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb,
basic_block outer_cond_bb,
<outer_cond_bb>
if (q) goto inner_cond_bb; else goto then_bb;
<inner_cond_bb>
- if (q) goto then_bb; else goto ...;
+ if (p) goto then_bb; else goto ...;
<then_bb>
...
*/
@@ -1139,6 +1139,10 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
if (!recognize_if_then_else (inner_cond_bb, &then_bb, &else_bb))
return ret;
+ /* FWD_WHICH will be set along with EXIT_BB to take note of whether THEN_BB
+ (1), ELSE_BB (-1) or neither (0) is a forwarder block to EXIT_BB. */
+ int fwd_which;
+
/* Recognize && and || of two conditions with a common
then/else block which entry edges we can merge. That is:
if (a || b)
@@ -1198,10 +1202,14 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
checking of same phi args. */
if (known_succ_p (outer_cond_bb))
changed = false;
- else if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb,
- then_bb, else_bb, inner_cond_bb, bb))
- changed = true;
- else if (forwarder_block_to (else_bb, then_bb))
+ else if (exit_bb
+ ? fwd_which == 0
+ : tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb,
+ then_bb, else_bb, inner_cond_bb, bb))
+ changed = true, fwd_which = 0;
+ else if (exit_bb
+ ? fwd_which < 0
+ : forwarder_block_to (else_bb, then_bb))
{
/* Other possibilities for the && form, if else_bb is
empty forwarder block to then_bb. Compared to the above simpler
@@ -1211,9 +1219,11 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
edge from outer_cond_bb and the forwarder block. */
if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb, else_bb,
then_bb, else_bb, bb))
- changed = true;
+ changed = true, fwd_which = -1;
}
- else if (forwarder_block_to (then_bb, else_bb))
+ else if (exit_bb
+ ? fwd_which > 0
+ : forwarder_block_to (then_bb, else_bb))
{
/* Other possibilities for the || form, if then_bb is
empty forwarder block to else_bb. Compared to the above simpler
@@ -1223,7 +1233,7 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
edge from outer_cond_bb and the forwarder block. */
if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb, else_bb,
then_bb, then_bb, bb))
- changed = true;
+ changed = true, fwd_which = 1;
}
if (changed)
@@ -1243,6 +1253,45 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
exit_bb = else_bb;
else
break;
+
+ /* Find out which path from INNER_COND_BB shares PHI args with the
+ edge (OUTER_COND_BB->EXIT_BB). That path may involve a forwarder
+ block, whether THEN_BB or ELSE_BB, and we need to know which one
+ satisfies the condition to avoid combinations that could use
+ different forwarding arrangements, because they would be unsound.
+ E.g., given (a ? 0 : b ? 1 : c ? 1 : 0), after trying to merge b
+ and c, we test that both share the same exit block, with the same
+ value 1. Whether or not that involves a forwarder block, if we
+ don't go through the same (possibly absent) forwarder block in
+ subsequent attempted combinations, e.g. a with c, we could find
+ that a and inverted c share the same exit block with a different
+ value, namely 0, which would enable an unsound merge. We need all
+ of inner, intervening and outer blocks to reach the same exit with
+ the same value for the transformation to be sound. So here we
+ determine how to get to EXIT_BB from outer and inner with the same
+ PHI values, record that in FWD_WHICH, and then subsequent
+ combination attempts that have OUTER_COND_BB as an intervening
+ block will ensure the same path to exit is taken, skipping unsound
+ transformations. */
+ if (changed)
+ /* FWD_WHICH was set along with CHANGED, and the successful
+ combination already checked for the same PHI args. */;
+ else if (same_phi_args_p (outer_cond_bb, inner_cond_bb, exit_bb))
+ fwd_which = 0;
+ else if (then_bb == exit_bb
+ && forwarder_block_to (else_bb, then_bb)
+ && same_phi_args_p (outer_cond_bb, else_bb, exit_bb))
+ fwd_which = -1;
+ else if (else_bb == exit_bb
+ && forwarder_block_to (then_bb, else_bb)
+ && same_phi_args_p (outer_cond_bb, then_bb, exit_bb))
+ fwd_which = 1;
+ else
+ /* If none of the paths share the same PHI args, no combination is
+ viable. */
+ break;
+ /* Skip the PHI args test below. */
+ continue;
}
/* Before trying an earlier block, make sure INNER_COND_BB and the
