On Sun, Apr 27, 2025 at 1:07 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> When we have an empty function, things can go wrong with
> cfi_startproc/cfi_endproc and a few other
> things like exceptions. So if the only thing the function does is a call to
> __builtin_unreachable,
> let's expand that to a __builtin_trap instead. For most targets that is one
> instruction wide so it
> won't hurt things that much and we get correct behavior for exceptions and
> some linkers will be better
> for it.
Doing this at RTL expansion is a bit of an awkward spot since it never
appears in any dump.
We have pass_cleanup_cfg_post_optimizing aka ".optimized", so I wonder
if we could do
this there instead?
>From a QOI perspective we'd really want to turn any unreachable() that
falls off the function
into a trap (also consider hot/cold partitioning here). ISTR we
talked about all this in detail
somewhen back, but I don't remember the issues with doing this after BB reorder.
Richard.
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR middle-end/109267
>
> gcc/ChangeLog:
>
> * cfgexpand.cc (expand_gimple_basic_block): If the first non debug
> statement in the
> first (and only) basic block is a call to __builtin_unreachable
> change it to a call
> to __builtin_trap.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/pr109267-1.c: New test.
> * gcc.dg/pr109267-2.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/cfgexpand.cc | 8 ++++++++
> gcc/testsuite/gcc.dg/pr109267-1.c | 14 ++++++++++++++
> gcc/testsuite/gcc.dg/pr109267-2.c | 13 +++++++++++++
> 3 files changed, 35 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/pr109267-1.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109267-2.c
>
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index e84f12a5e93..e14df760b7a 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -6206,6 +6206,14 @@ expand_gimple_basic_block (basic_block bb, bool
> disable_tail_calls)
> basic_block new_bb;
>
> stmt = gsi_stmt (gsi);
> +
> + /* If we are expanding the first (and only) bb and the only non debug
> + statement is __builtin_unreachable call, then replace it with a trap
> + so the function is at least one instruction in size. */
> + if (!nondebug_stmt_seen && bb->index == NUM_FIXED_BLOCKS
> + && gimple_call_builtin_p (stmt, BUILT_IN_UNREACHABLE))
> + gimple_call_set_fndecl(stmt, builtin_decl_implicit (BUILT_IN_TRAP));
> +
> if (!is_gimple_debug (stmt))
> nondebug_stmt_seen = true;
>
> diff --git a/gcc/testsuite/gcc.dg/pr109267-1.c
> b/gcc/testsuite/gcc.dg/pr109267-1.c
> new file mode 100644
> index 00000000000..4f1da8b41e3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr109267-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-rtl-expand-details" } */
> +
> +/* PR middle-end/109267 */
> +
> +int f(void)
> +{
> + __builtin_unreachable();
> +}
> +
> +/* This unreachable should expand as trap. */
> +
> +/* { dg-final { scan-rtl-dump-times "__builtin_trap " 1 "expand"} } */
> +/* { dg-final { scan-rtl-dump-times "__builtin_unreachable " 1 "expand"} } */
> diff --git a/gcc/testsuite/gcc.dg/pr109267-2.c
> b/gcc/testsuite/gcc.dg/pr109267-2.c
> new file mode 100644
> index 00000000000..e6da4860998
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr109267-2.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-rtl-expand-details" } */
> +
> +/* PR middle-end/109267 */
> +void g(void);
> +int f(int *t)
> +{
> + g();
> + __builtin_unreachable();
> +}
> +
> +/* This should be expanded to unreachable so it should show up twice. */
> +/* { dg-final { scan-rtl-dump-times "__builtin_unreachable " 2 "expand"} } */
> --
> 2.43.0
>
