On 10/17/23 03:19, Richard Biener wrote:
On Thu, Oct 12, 2023 at 10:15 AM Richard Sandiford
<richard.sandif...@arm.com> wrote:

Richard Biener <richard.guent...@gmail.com> writes:
On Tue, Aug 22, 2023 at 12:42 PM Szabolcs Nagy via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:

From: Richard Sandiford <richard.sandif...@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.

Sorry for the late reply, but does this look OK?  Tested on
aarch64-linux-gnu and x86_64-linux-gnu.

LGTM, not sure if I'm qualified enough to approve though (I think you
are more qualified here, so ..)
It looks quite sensible to me.

Jeff

Reply via email to