Hi Jakub, all,
> On 14 May 2019, at 23:57, Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Tue, May 14, 2019 at 01:08:27PM -0600, Jeff Law wrote:
>>> In https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00484.html I've posted a
>>> patch that would set it earlier (or it could be set during gimplification
>>> and propagated during inlining, would need to be in cfun->calls_eh_return
>>> instead of crtl->calls_eh_return) and then targets for which we do not want
>>> to bother with it or where it is not beneficial to have tail calls in
>>> functions that call __builtin_eh_return (as I said in the thread, e.g. on
>>> x86_64-linux it is both smaller and faster), the targets which we expect to
>>> fail currently or even have a proof of that can just punt.
>> I would go with a patch that got the info set earlier and just punt in
>> the generic code. I just don't see this case as terribly important to
>> optimize.
>
> So like this? Bootstrapped/regtested on x86_64-linux and i686-linux,
> verified _Unwind_Resume_or_Rethrow no longer has a tail call.
JFTR, this fixes bootstrap/90418 for powerpc-darwin9
I don’t have a strong policy view: I’m happy with either this, or some variant
that provides ??->calls_eh_return at <target>_function_ok_for_sibcall time.
it would be nice to get consensus and apply a fix.
Iain
>
> 2019-05-14 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/59813
> PR target/90418
> * function.h (struct function): Add calls_eh_return member.
> * gimplify.c (gimplify_call_expr): Set cfun->calls_eh_return when
> gimplifying __builtin_eh_return call.
> * tree-inline.c (initialize_cfun): Copy calls_eh_return from src_cfun
> to cfun.
> (expand_call_inline): Or in src_cfun->calls_eh_return into
> dst_cfun->calls_eh_return.
> * tree-tailcall.c (suitable_for_tail_call_opt_p): Return false if
> cfun->calls_eh_return.
> * lto-streamer-in.c (input_struct_function_base): Read calls_eh_return.
> * lto-streamer-out.c (output_struct_function_base): Write
> calls_eh_return.
>
> --- gcc/function.h.jj 2019-01-01 12:37:15.383004075 +0100
> +++ gcc/function.h 2019-05-14 21:40:13.837715081 +0200
> @@ -327,6 +327,9 @@ struct GTY(()) function {
> either as a subroutine or builtin. */
> unsigned int calls_alloca : 1;
>
> + /* Nonzero if function being compiled can call __builtin_eh_return. */
> + unsigned int calls_eh_return : 1;
> +
> /* Nonzero if function being compiled receives nonlocal gotos
> from nested functions. */
> unsigned int has_nonlocal_label : 1;
> --- gcc/gimplify.c.jj 2019-05-03 15:22:07.416401170 +0200
> +++ gcc/gimplify.c 2019-05-14 21:51:38.700288873 +0200
> @@ -3297,6 +3297,10 @@ gimplify_call_expr (tree *expr_p, gimple
> break;
> }
>
> + case BUILT_IN_EH_RETURN:
> + cfun->calls_eh_return = true;
> + break;
> +
> default:
> ;
> }
> --- gcc/tree-inline.c.jj 2019-05-10 10:15:40.704283180 +0200
> +++ gcc/tree-inline.c 2019-05-14 21:49:19.038617963 +0200
> @@ -2662,6 +2662,7 @@ initialize_cfun (tree new_fndecl, tree c
> cfun->va_list_gpr_size = src_cfun->va_list_gpr_size;
> cfun->va_list_fpr_size = src_cfun->va_list_fpr_size;
> cfun->has_nonlocal_label = src_cfun->has_nonlocal_label;
> + cfun->calls_eh_return = src_cfun->calls_eh_return;
> cfun->stdarg = src_cfun->stdarg;
> cfun->after_inlining = src_cfun->after_inlining;
> cfun->can_throw_non_call_exceptions
> @@ -4778,6 +4779,7 @@ expand_call_inline (basic_block bb, gimp
> src_properties = id->src_cfun->curr_properties & prop_mask;
> if (src_properties != prop_mask)
> dst_cfun->curr_properties &= src_properties | ~prop_mask;
> + dst_cfun->calls_eh_return |= id->src_cfun->calls_eh_return;
>
> gcc_assert (!id->src_cfun->after_inlining);
>
> --- gcc/tree-tailcall.c.jj 2019-05-10 23:20:33.849768476 +0200
> +++ gcc/tree-tailcall.c 2019-05-14 21:54:34.733353248 +0200
> @@ -140,6 +140,7 @@ suitable_for_tail_opt_p (void)
>
> return true;
> }
> +
> /* Returns false when the function is not suitable for tail call optimization
> for some reason (e.g. if it takes variable number of arguments).
> This test must pass in addition to suitable_for_tail_opt_p in order to make
> @@ -168,6 +169,11 @@ suitable_for_tail_call_opt_p (void)
> if (cfun->calls_setjmp)
> return false;
>
> + /* Various targets don't handle tail calls correctly in functions
> + that call __builtin_eh_return. */
> + if (cfun->calls_eh_return)
> + return false;
> +
> /* ??? It is OK if the argument of a function is taken in some cases,
> but not in all cases. See PR15387 and PR19616. Revisit for 4.1. */
> for (param = DECL_ARGUMENTS (current_function_decl);
> --- gcc/lto-streamer-in.c.jj 2019-03-08 11:43:35.062317743 +0100
> +++ gcc/lto-streamer-in.c 2019-05-14 21:41:48.900128559 +0200
> @@ -1005,6 +1005,7 @@ input_struct_function_base (struct funct
> fn->has_forced_label_in_static = bp_unpack_value (&bp, 1);
> fn->calls_alloca = bp_unpack_value (&bp, 1);
> fn->calls_setjmp = bp_unpack_value (&bp, 1);
> + fn->calls_eh_return = bp_unpack_value (&bp, 1);
> fn->has_force_vectorize_loops = bp_unpack_value (&bp, 1);
> fn->has_simduid_loops = bp_unpack_value (&bp, 1);
> fn->va_list_fpr_size = bp_unpack_value (&bp, 8);
> --- gcc/lto-streamer-out.c.jj 2019-03-08 11:43:35.062317743 +0100
> +++ gcc/lto-streamer-out.c 2019-05-14 21:42:05.262855481 +0200
> @@ -2029,6 +2029,7 @@ output_struct_function_base (struct outp
> bp_pack_value (&bp, fn->has_forced_label_in_static, 1);
> bp_pack_value (&bp, fn->calls_alloca, 1);
> bp_pack_value (&bp, fn->calls_setjmp, 1);
> + bp_pack_value (&bp, fn->calls_eh_return, 1);
> bp_pack_value (&bp, fn->has_force_vectorize_loops, 1);
> bp_pack_value (&bp, fn->has_simduid_loops, 1);
> bp_pack_value (&bp, fn->va_list_fpr_size, 8);
>
>
> Jakub
