Re: [PATCH 02/11] Handle epilogues that contain jumps

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Aug 22, 2023 at 12:42 PM Szabolcs Nagy via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Richard Sandiford <richard.sandiford@arm.com>
>
> The prologue/epilogue pass allows the prologue sequence
> to contain jumps.  The sequence is then partitioned into
> basic blocks using find_many_sub_basic_blocks.
>
> This patch treats epilogues in the same way.  It's needed for
> a follow-on aarch64 patch that adds conditional code to both
> the prologue and the epilogue.
>
> Tested on aarch64-linux-gnu (including with a follow-on patch)
> and x86_64-linux-gnu.  OK to install?
>
> Richard
>
> gcc/
>         * function.cc (thread_prologue_and_epilogue_insns): Handle
>         epilogues that contain jumps.
> ---
>
> This is a previously approved patch that was not committed
> because it was not needed at the time, but i'd like to commit
> it as it is needed for the followup aarch64 eh_return changes:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605769.html
>
> ---
>  gcc/function.cc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/gcc/function.cc b/gcc/function.cc
> index dd2c1136e07..70d1cd65303 100644
> --- a/gcc/function.cc
> +++ b/gcc/function.cc
> @@ -6120,6 +6120,11 @@ thread_prologue_and_epilogue_insns (void)
>                   && returnjump_p (BB_END (e->src)))
>                 e->flags &= ~EDGE_FALLTHRU;
>             }
> +
> +         auto_sbitmap blocks (last_basic_block_for_fn (cfun));
> +         bitmap_clear (blocks);
> +           bitmap_set_bit (blocks, BLOCK_FOR_INSN (epilogue_seq)->index);
> +         find_many_sub_basic_blocks (blocks);
>         }
>        else if (next_active_insn (BB_END (exit_fallthru_edge->src)))
>         {
> @@ -6218,6 +6223,11 @@ thread_prologue_and_epilogue_insns (void)
>           set_insn_locations (seq, epilogue_location);
>
>           emit_insn_before (seq, insn);
> +
> +         auto_sbitmap blocks (last_basic_block_for_fn (cfun));
> +         bitmap_clear (blocks);
> +         bitmap_set_bit (blocks, BLOCK_FOR_INSN (insn)->index);
> +         find_many_sub_basic_blocks (blocks);

I'll note that clearing a full sbitmap to pass down a single basic block
to find_many_sub_basic_blocks is a quite expensive operation.  May I suggest
to add an overload operating on a single basic block?  It's only

  FOR_EACH_BB_FN (bb, cfun)
    SET_STATE (bb,
               bitmap_bit_p (blocks, bb->index) ? BLOCK_TO_SPLIT :
BLOCK_ORIGINAL);

using the bitmap, so factoring the rest of the function and customizing this
walk would do the trick.  Note that the whole function could be refactored to
handle single blocks more efficiently.

>         }
>      }
>
> --
> 2.25.1
>