On Thu, Apr 3, 2025 at 7:10 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> builtin_unreachable_bb_p exacts empty basic blocks to have only one successor
> edge but
> in the case after inliing a noreturn function that actually returns, we end
> up with an
> empty basic block with no successor edge. fixup_cfg fixes this up by placing
> a call to
> __builtin_unreachable but we don't do that before getting the function summary
> (after einiline). So the fix is to have the inliner insert the call
> to __builtin_unreachable (or the trap version) and not depend on the fixup if
> the return
> block is reachable after inlining a noreturn function.
>
> A small optimization is done not to insert the call if the block is NOT
> simplely reachable.
> This should prevent inserting the call just to remove it during cleanupcfg.
>
> Note this shows up only at -O0 with always_inline because when optimizations
> are turned on
> returns are turned into __builtin_unreachable inside noreturn functions.
Where's that done? IMO we should do this at -O0 as well (possibly defaulting to
use the trapping variant there).
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR ipa/119599
>
> gcc/ChangeLog:
>
> * tree-inline.cc (expand_call_inline): Add a __builtin_unreachable
> call
> to the return block if it is still reachable and the call that was
> inlined
> was a noreturn function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr119599-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/testsuite/gcc.dg/torture/pr119599-1.c | 27 +++++++++++++++++++++++
> gcc/tree-inline.cc | 15 +++++++++++++
> 2 files changed, 42 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr119599-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> new file mode 100644
> index 00000000000..4fbd228a359
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-einline" } */
> +
> +/* PR ipa/119599 */
> +/* inlining a noreturn function which returns
> + can cause an ICE when dealing finding an unreachable block.
> + We should get a __builtin_unreachable after the inliing. */
> +
> +
> +void baz (void);
> +
> +static inline __attribute__((always_inline, noreturn)) void
> +bar (void)
> +{
> + static volatile int t = 0;
> + if (t == 0)
> + baz ();
> +} /* { dg-warning "function does return" } */
> +
> +void
> +foo (void)
> +{
> + bar ();
> +}
> +
> +/* After inlining, we should have call to __builtin_unreachable now. */
> +/* { dg-final { scan-tree-dump "__builtin_unreachable " "einline" } } */
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index 05843b8ccf0..da403cd1456 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -4832,6 +4832,7 @@ expand_call_inline (basic_block bb, gimple *stmt,
> copy_body_data *id,
> gimple *simtenter_stmt = NULL;
> vec<tree> *simtvars_save;
> tree save_stack = NULL_TREE;
> + bool was_noreturn;
>
> /* The gimplifier uses input_location in too many places, such as
> internal_get_tmp_var (). */
> @@ -4843,6 +4844,8 @@ expand_call_inline (basic_block bb, gimple *stmt,
> copy_body_data *id,
> if (!call_stmt)
> goto egress;
>
> + was_noreturn = gimple_call_noreturn_p (call_stmt);
> +
> cg_edge = id->dst_node->get_edge (stmt);
> gcc_checking_assert (cg_edge);
> /* First, see if we can figure out what function is being called.
> @@ -5376,6 +5379,18 @@ expand_call_inline (basic_block bb, gimple *stmt,
> copy_body_data *id,
> if (purge_dead_abnormal_edges)
> bitmap_set_bit (to_purge, return_block->index);
>
> + /* If this was a noreturn function and if the return block is still
> + reachable, then add a call to __builtin_unreachable(). */
> + if (was_noreturn && EDGE_COUNT (return_block->preds) != 0)
> + {
> + gcall *call_stmt = gimple_build_builtin_unreachable (input_location);
> + gimple_stmt_iterator gsi = gsi_last_bb (return_block);
> + gsi_insert_after (&gsi, call_stmt, GSI_NEW_STMT);
> + tree fndecl = gimple_call_fndecl (call_stmt);
> + cg_edge->caller->create_edge (cgraph_node::get_create (fndecl),
> + call_stmt, return_block->count);
> + }
> +
> /* If the value of the new expression is ignored, that's OK. We
> don't warn about this for CALL_EXPRs, so we shouldn't warn about
> the equivalent inlined version either. */
> --
> 2.43.0
>
