https://gcc.gnu.org/g:1d8e2d51e5c5cbdb6ad6f455a050af869103dad4
commit r16-5258-g1d8e2d51e5c5cbdb6ad6f455a050af869103dad4 Author: Andrew Pinski <[email protected]> Date: Tue Nov 11 11:29:38 2025 -0800 cfgcleanup: Support merging forwarder blocks with phis [PR122493] This adds support for merging forwarder blocks with phis in cleanupcfg. This patch might seem small but that is because the previous patches were done to build up to make it easier to add this support. There is still one more patch to merge remove_forwarder_block and remove_forwarder_block_with_phi since remove_forwarder_block_with_phi supports splitting an edge which is not supported as an option in remove_forwarder_block. The splitting edge option should not be enabled for cfgcleanup but only for mergephi. Note r8-338-ge7d70c6c3bccb2 added always creating a preheader for loops so we should protect them if we have a phi node as it goes back and forth here. And both the gimple and RTL loop code likes to have this preheader in the case of having the same constant value being starting of the loop. explaination on testcase changes gcc.target/i386/pr121062-1.c needed a small change because there is a basic block which is not duplicated so only one `movq reg, -1` is there instead of 2. uninit-pred-7_a.c is xfailed and filed as PR122660, some analysis in the PR already of the difference now. uninit-pred-5.C was actually a false positive because when m_best_candidate is non-NULL, m_best_candidate_len is always initialized. The log message on the testcase is wrong if you manually fall the path you can notice that. With an extra jump threading after the merging of some bbs, the false positive is now no longer happening. So change the dg-warning to dg-bogus. ssa-dom-thread-7.c now jump threads 12 times in thread2 instead of 8 Bootstrapped and tested on x86_64-linux-gnu. PR tree-optimization/122493 gcc/ChangeLog: * tree-cfgcleanup.cc (tree_forwarder_block_p): Change bool argument to a must have phi and allow phis if it is false. (remove_forwarder_block): Add support for merging of forwarder blocks with phis. gcc/testsuite/ChangeLog: * gcc.target/i386/pr121062-1.c: Update count. * gcc.dg/uninit-pred-7_a.c: xfail line 23. * g++.dg/uninit-pred-5.C: Change dg-warning to dg-bogus. * gcc.dg/tree-ssa/ssa-dom-thread-7.c: Update count of jump thread. Signed-off-by: Andrew Pinski <[email protected]> Diff: --- gcc/testsuite/g++.dg/uninit-pred-5.C | 2 +- gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c | 4 +-- gcc/testsuite/gcc.dg/uninit-pred-7_a.c | 2 +- gcc/testsuite/gcc.target/i386/pr121062-1.c | 2 +- gcc/tree-cfgcleanup.cc | 43 +++++++++++++++++++----- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/gcc/testsuite/g++.dg/uninit-pred-5.C b/gcc/testsuite/g++.dg/uninit-pred-5.C index 8dfd9874f65b..cffac6b0c1ea 100644 --- a/gcc/testsuite/g++.dg/uninit-pred-5.C +++ b/gcc/testsuite/g++.dg/uninit-pred-5.C @@ -41,7 +41,7 @@ public: edit_distance_t __trans_tmp_1; if (m_best_candidate) { size_t candidate_len = m_best_candidate_len; - __trans_tmp_1 = get_edit_distance_cutoff(candidate_len); // { dg-warning "may be used uninitialized" } + __trans_tmp_1 = get_edit_distance_cutoff(candidate_len); // { dg-bogus "may be used uninitialized" } } edit_distance_t cutoff = __trans_tmp_1; if (cutoff) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c index 1c2cfa4fa8d5..81bb7fc9d1e0 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c @@ -11,8 +11,8 @@ to change decisions in switch expansion which in turn can expose new jump threading opportunities. Skip the later tests on aarch64. */ /* { dg-final { scan-tree-dump-not "Jumps threaded" "dom3" { target { ! aarch64*-*-* } } } } */ -/* { dg-final { scan-tree-dump "Jumps threaded: 8" "thread2" { target { ! aarch64*-*-* } } } } */ -/* { dg-final { scan-tree-dump "Jumps threaded: 8" "thread2" { target { aarch64*-*-* } } } } */ +/* { dg-final { scan-tree-dump "Jumps threaded: 12" "thread2" { target { ! aarch64*-*-* } } } } */ +/* { dg-final { scan-tree-dump "Jumps threaded: 12" "thread2" { target { aarch64*-*-* } } } } */ enum STATE { S0=0, diff --git a/gcc/testsuite/gcc.dg/uninit-pred-7_a.c b/gcc/testsuite/gcc.dg/uninit-pred-7_a.c index c2ba2a4248d7..7aaadf7def75 100644 --- a/gcc/testsuite/gcc.dg/uninit-pred-7_a.c +++ b/gcc/testsuite/gcc.dg/uninit-pred-7_a.c @@ -20,7 +20,7 @@ int foo (int n, int l, int m, int r) blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */ if ( n ) - blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */ + blah(v); /* { dg-bogus "uninitialized" "bogus warning" { xfail *-*-* } } */ if ( l ) blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */ diff --git a/gcc/testsuite/gcc.target/i386/pr121062-1.c b/gcc/testsuite/gcc.target/i386/pr121062-1.c index 799f8562c9f7..c334df64cdca 100644 --- a/gcc/testsuite/gcc.target/i386/pr121062-1.c +++ b/gcc/testsuite/gcc.target/i386/pr121062-1.c @@ -31,4 +31,4 @@ render_result_from_bake_h(int tx) } } -/* { dg-final { scan-assembler-times "movq\[ \\t\]+\\\$-1, %r\[a-z0-9\]+" 2 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times "movq\[ \\t\]+\\\$-1, %r\[a-z0-9\]+" 1 { target { ! ia32 } } } } */ diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc index 803d8599b577..636a546a3dc9 100644 --- a/gcc/tree-cfgcleanup.cc +++ b/gcc/tree-cfgcleanup.cc @@ -386,16 +386,13 @@ cleanup_control_flow_bb (basic_block bb) the entry block. */ static bool -tree_forwarder_block_p (basic_block bb, bool phi_wanted) +tree_forwarder_block_p (basic_block bb, bool must_have_phis) { gimple_stmt_iterator gsi; location_t locus; /* BB must have a single outgoing edge. */ if (!single_succ_p (bb) - /* If PHI_WANTED is false, BB must not have any PHI nodes. - Otherwise, BB must have PHI nodes. */ - || gimple_seq_empty_p (phi_nodes (bb)) == phi_wanted /* BB may not be a predecessor of the exit block. */ || single_succ (bb) == EXIT_BLOCK_PTR_FOR_FN (cfun) /* Nor should this be an infinite loop. */ @@ -404,6 +401,10 @@ tree_forwarder_block_p (basic_block bb, bool phi_wanted) || (single_succ_edge (bb)->flags & EDGE_ABNORMAL)) return false; + /* If MUST_HAVE_PHIS is true and we don't have any phis, return false. */ + if (must_have_phis && gimple_seq_empty_p (phi_nodes (bb))) + return false; + gcc_checking_assert (bb != ENTRY_BLOCK_PTR_FOR_FN (cfun)); locus = single_succ_edge (bb)->goto_locus; @@ -520,8 +521,10 @@ tree_forwarder_block_p (basic_block bb, bool phi_wanted) || loops_state_satisfies_p (LOOPS_MAY_HAVE_MULTIPLE_LATCHES)); } + /* cleanup_tree_cfg_noloop just created the loop preheader, don't + remove it if it has phis. */ else if (bb->loop_father == loop_outer (dest->loop_father)) - return !loops_state_satisfies_p (LOOPS_HAVE_PREHEADERS); + return gimple_seq_empty_p (phi_nodes (bb)); /* Always preserve other edges into loop headers that are not simple latches or preheaders. */ return false; @@ -625,6 +628,7 @@ remove_forwarder_block (basic_block bb) basic_block dest = succ->dest; gimple *stmt; gimple_stmt_iterator gsi, gsi_to; + bool has_phi = !gimple_seq_empty_p (phi_nodes (bb)); /* If there is an abnormal edge to basic block BB, but not into dest, problems might occur during removal of the phi node at out @@ -636,15 +640,24 @@ remove_forwarder_block (basic_block bb) does not like it. So if there is an abnormal edge to BB, proceed only if there is - no abnormal edge to DEST and there are no phi nodes in DEST. */ + no abnormal edge to DEST and there are no phi nodes in DEST. + If the BB has phi, we don't want to deal with abnormal edges either. */ if (bb_has_abnormal_pred (bb) && (bb_has_abnormal_pred (dest) - || !gimple_seq_empty_p (phi_nodes (dest)))) + || !gimple_seq_empty_p (phi_nodes (dest)) + || has_phi)) + return false; + + /* When we have a phi, we have to feed into another + basic block with PHI nodes. */ + if (has_phi && gimple_seq_empty_p (phi_nodes (dest))) return false; /* If there are phi nodes in DEST, and some of the blocks that are predecessors of BB are also predecessors of DEST, check that the - phi node arguments match. */ + phi node arguments match. + Otherwise we have to split the edge and that becomes + a "forwarder" again. */ if (!gimple_seq_empty_p (phi_nodes (dest))) { edge_iterator ei; @@ -680,9 +693,21 @@ remove_forwarder_block (basic_block bb) if (s == e) { + /* If we merge the forwarder with phis into a loop header + verify if we are creating another loop latch edge. + If so, reset number of iteration information of the loop. */ + if (has_phi + && dest->loop_father + && dest->loop_father->header == dest + && dominated_by_p (CDI_DOMINATORS, e->src, dest)) + { + dest->loop_father->any_upper_bound = false; + dest->loop_father->any_likely_upper_bound = false; + free_numbers_of_iterations_estimates (dest->loop_father); + } /* Copy arguments for the phi nodes, since the edge was not here before. */ - copy_phi_arg_into_existing_phi (succ, s); + copy_phi_arg_into_existing_phi (succ, s, has_phi); } }
