From: Andrew Pinski Date: Sun, 14 Dec 2025 04:48:51 +0000 (-0800) Subject: final_cleanup: Don't create forwarder blocks for degenerate_phis that are ifconvertab... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4204d3c46581bff5461e7b837e8bd3e1f2c24a2e;p=thirdparty%2Fgcc.git final_cleanup: Don't create forwarder blocks for degenerate_phis that are ifconvertable [PR123111] So it turns out creating a forwarder block in some cases causes a regression. The rtl ifcvt does like the case were the middle bb does not have a single predecessor. So in some cases ifcvt does not happen any more. So the way to fix this is not to create a forwarder block for those edges which causes out of ssa to create a middle bb which has the constant or the copy in it. I tried to do a simple ifcvt on the gimple level with the last phiopt but that introduces regressions as then `v += cmp - 43;` is not optimized and produces an extra subtraction. This is the best workaround I could come up with until that is fixed. I filed PR 123116 for that issue. Bootstrapped and tested on x86_64-linux-gnu. PR tree-optimization/123111 gcc/ChangeLog: * tree-cfg.cc (ifconvertable_edge): New function. (make_forwarders_with_degenerate_phis): Add skip_ifcvtable argument, check ifconvertable_edge if skip_ifcvtable is true. * tree-cfg.h (make_forwarders_with_degenerate_phis): New argument with default of false. * tree-cfgcleanup.cc (execute_cleanup_cfg_post_optimizing): Update argument to make_forwarders_with_degenerate_phis. Signed-off-by: Andrew Pinski --- diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc index 8ee208b3032..ec3be8125f0 100644 --- a/gcc/tree-cfg.cc +++ b/gcc/tree-cfg.cc @@ -10195,13 +10195,77 @@ sort_phi_args (const void *a_, const void *b_) return 0; } +/* Returns true if edge E comes from a possible ifconvertable branch. + That is: + ``` + BB0: + if (a) goto BB1; else goto BB2; + BB1: + BB2: + ``` + Returns true for the edge from BB0->BB2 or BB1->BB2. + This function assumes we only have one middle block. + And the middle block is empty. */ + +static bool +ifconvertable_edge (edge e) +{ + basic_block bb2 = e->dest; + basic_block bb0 = e->src; + basic_block bb1 = nullptr, rbb1; + if (e->src == e->dest) + return false; + if (EDGE_COUNT (bb0->succs) > 2) + return false; + if (single_succ_p (bb0)) + { + if (!single_pred_p (bb0)) + return false; + /* The middle block can only be empty, + otherwise the phis will be + different anyways. */ + if (!empty_block_p (bb0)) + return false; + bb1 = bb0; + bb0 = single_pred (bb0); + if (EDGE_COUNT (bb0->succs) != 2) + return false; + } + + /* If convertables are only for conditionals. */ + if (!is_a(*gsi_last_nondebug_bb (bb0))) + return false; + + /* Find the other basic block. */ + if (EDGE_SUCC (bb0, 0)->dest == bb2) + rbb1 = EDGE_SUCC (bb0, 1)->dest; + else if (EDGE_SUCC (bb0, 1)->dest == bb2) + rbb1 = EDGE_SUCC (bb0, 0)->dest; + else + return false; + + /* If we already know bb1, then just test it. */ + if (bb1) + return rbb1 == bb1; + + if (!single_succ_p (rbb1) || !single_pred_p (rbb1)) + return false; + /* The middle block can only be empty, + otherwise the phis will be + different anyways. */ + if (!empty_block_p (rbb1)) + return false; + + return single_succ (rbb1) == bb2; +} + /* Look for a non-virtual PHIs and make a forwarder block when all PHIs have the same argument on a set of edges. This is to not consider control dependences of individual edges for same values but only for the common set. Returns true if changed the CFG. */ bool -make_forwarders_with_degenerate_phis (function *fn) +make_forwarders_with_degenerate_phis (function *fn, bool skip_ifcvtable) { bool didsomething = false; @@ -10245,6 +10309,13 @@ make_forwarders_with_degenerate_phis (function *fn) && loop_exit_edge_p (e->src->loop_father, e)) continue; + /* Skip ifconvertable edges when asked as we want the + copy/constant on that edge still when going out of ssa. + FIXME: phiopt should produce COND_EXPR but there + are regressions with that. */ + if (skip_ifcvtable && ifconvertable_edge (e)) + continue; + tree arg = gimple_phi_arg_def (phi, i); if (!CONSTANT_CLASS_P (arg) && TREE_CODE (arg) != SSA_NAME) need_resort = true; diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h index 2e01762f04b..56d40406d1f 100644 --- a/gcc/tree-cfg.h +++ b/gcc/tree-cfg.h @@ -114,7 +114,7 @@ extern edge gimple_switch_edge (function *, gswitch *, unsigned); extern edge gimple_switch_default_edge (function *, gswitch *); extern bool cond_only_block_p (basic_block); extern void copy_phi_arg_into_existing_phi (edge, edge, bool = false); -extern bool make_forwarders_with_degenerate_phis (function *); +extern bool make_forwarders_with_degenerate_phis (function *, bool = false); /* Return true if the LHS of a call should be removed. */ diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc index 2521e6d09fd..8060e90be79 100644 --- a/gcc/tree-cfgcleanup.cc +++ b/gcc/tree-cfgcleanup.cc @@ -1418,7 +1418,7 @@ execute_cleanup_cfg_post_optimizing (void) /* When optimizing undo the merging of forwarder blocks that phis for better out of ssa expansion. */ if (optimize) - make_forwarders_with_degenerate_phis (cfun); + make_forwarders_with_degenerate_phis (cfun, true); /* Make sure todo does not have cleanup cfg as we don't want remove the forwarder blocks we just created. cleanup cfg