Jakub Jelinek <ja...@redhat.com> writes:
> On Fri, Jun 13, 2025 at 10:05:14AM +0200, Jakub Jelinek wrote:
>> On Fri, Jun 13, 2025 at 08:52:55AM +0100, Richard Sandiford wrote:
>> > > 2025-06-12 Jakub Jelinek <ja...@redhat.com>
>> > >
>> > > * cfgexpand.cc (construct_init_block): If first_block isn't BB_RTL,
>> > > has any PHI nodes and false_edge->dest_idx before redirection is
>> > > different from make_single_succ_edge result's dest_idx, swap the
>> > > latter with the former last pred edge and their dest_idx members.
>> >
>> > OK, thanks. But now that there are two instances, I wonder if it would
>> > be worth hiding this detail in a helper function?
>>
>> Will try to come up with something incrementally if that is ok.
>
> Here it is, so far lightly tested. Ok for trunk if it passes full
> bootstrap/regtest?
>
> 2025-06-13 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/120629
> * cfgexpand.cc (expand_split_edge): New function.
> (expand_gimple_cond, construct_init_block): Use it.
OK. Thanks for doing this!
Richard
> --- gcc/cfgexpand.cc.jj 2025-06-13 14:01:15.168637257 +0200
> +++ gcc/cfgexpand.cc 2025-06-13 14:17:14.896303981 +0200
> @@ -2830,6 +2830,31 @@ expand_remove_edge (edge e)
> remove_edge (e);
> }
>
> +/* Split edge E during expansion and instead of creating a new
> + bb on that edge, add there BB. FLAGS should be flags on the
> + new edge from BB to former E->dest. */
> +
> +static void
> +expand_split_edge (edge e, basic_block bb, int flags)
> +{
> + unsigned int dest_idx = e->dest_idx;
> + basic_block dest = e->dest;
> + redirect_edge_succ (e, bb);
> + e = make_single_succ_edge (bb, dest, flags);
> + if ((dest->flags & BB_RTL) == 0
> + && phi_nodes (dest)
> + && e->dest_idx != dest_idx)
> + {
> + /* If there are any PHI nodes on dest, swap the new succ edge
> + with the one moved into false_edge's former position, so that
> + PHI arguments don't need adjustment. */
> + edge e2 = EDGE_PRED (dest, dest_idx);
> + std::swap (e->dest_idx, e2->dest_idx);
> + std::swap (EDGE_PRED (dest, e->dest_idx),
> + EDGE_PRED (dest, e2->dest_idx));
> + }
> +}
> +
>
> /* A subroutine of expand_gimple_cond. Given E, a fallthrough edge
> of a basic block where we just expanded the conditional at the end,
> @@ -3013,8 +3038,7 @@ expand_gimple_cond (basic_block bb, gcon
>
> new_bb = create_basic_block (NEXT_INSN (last), get_last_insn (), bb);
> dest = false_edge->dest;
> - unsigned int dest_idx = false_edge->dest_idx;
> - redirect_edge_succ (false_edge, new_bb);
> + expand_split_edge (false_edge, new_bb, 0);
> false_edge->flags |= EDGE_FALLTHRU;
> new_bb->count = false_edge->count ();
> loop_p loop = find_common_loop (bb->loop_father, dest->loop_father);
> @@ -3022,19 +3046,6 @@ expand_gimple_cond (basic_block bb, gcon
> if (loop->latch == bb
> && loop->header == dest)
> loop->latch = new_bb;
> - edge e = make_single_succ_edge (new_bb, dest, 0);
> - if ((dest->flags & BB_RTL) == 0
> - && phi_nodes (dest)
> - && e->dest_idx != dest_idx)
> - {
> - /* If there are any PHI nodes on dest, swap the new succ edge
> - with the one moved into false_edge's former position, so that
> - PHI arguments don't need adjustment. */
> - edge e2 = EDGE_PRED (dest, dest_idx);
> - std::swap (e->dest_idx, e2->dest_idx);
> - std::swap (EDGE_PRED (dest, e->dest_idx),
> - EDGE_PRED (dest, e2->dest_idx));
> - }
> if (BARRIER_P (BB_END (new_bb)))
> BB_END (new_bb) = PREV_INSN (BB_END (new_bb));
> update_bb_for_insn (new_bb);
> @@ -6538,7 +6549,7 @@ expand_gimple_basic_block (basic_block b
> static basic_block
> construct_init_block (void)
> {
> - basic_block init_block, first_block;
> + basic_block init_block;
> edge e = NULL;
> int flags;
>
> @@ -6569,24 +6580,7 @@ construct_init_block (void)
> init_block->count = ENTRY_BLOCK_PTR_FOR_FN (cfun)->count;
> add_bb_to_loop (init_block, ENTRY_BLOCK_PTR_FOR_FN (cfun)->loop_father);
> if (e)
> - {
> - unsigned int dest_idx = e->dest_idx;
> - first_block = e->dest;
> - redirect_edge_succ (e, init_block);
> - e = make_single_succ_edge (init_block, first_block, flags);
> - if ((first_block->flags & BB_RTL) == 0
> - && phi_nodes (first_block)
> - && e->dest_idx != dest_idx)
> - {
> - /* If there are any PHI nodes on dest, swap the new succ edge
> - with the one moved into false_edge's former position, so that
> - PHI arguments don't need adjustment. */
> - edge e2 = EDGE_PRED (first_block, dest_idx);
> - std::swap (e->dest_idx, e2->dest_idx);
> - std::swap (EDGE_PRED (first_block, e->dest_idx),
> - EDGE_PRED (first_block, e2->dest_idx));
> - }
> - }
> + expand_split_edge (e, init_block, flags);
> else
> make_single_succ_edge (init_block, EXIT_BLOCK_PTR_FOR_FN (cfun),
> EDGE_FALLTHRU);
>
>
> Jakub
