On Thu, May 16, 2024, 4:09 AM Kewen.Lin <li...@linux.ibm.com> wrote:
> Hi,
>
> As the associated test case in PR114846 shows, currently
> with eh_return involved some register restoring for EH
> RETURN DATA in epilogue can clobber the one which holding
> the return value. Referring to the existing handlings in
> some other targets, this patch makes eh_return expander
> call one new define_insn_and_split eh_return_internal which
> directly calls rs6000_emit_epilogue with epilogue_type
> EPILOGUE_TYPE_EH_RETURN instead of the previous treating
> normal return with crtl->calls_eh_return specially.
>
> Bootstrapped and regtested on powerpc64-linux-gnu P8/P9 and
> powerpc64le-linux-gnu P9 and P10.
>
> I'm going to push this next week if no objections.
>
Thanks for fixing this for powerpc. I hope my patch for aarch64 gets
reviewed soon and it will contain many more testcases. Hopefully someone
will fix the arm target too.
Thanks,
Andrew
> BR,
> Kewen
> -----
> PR target/114846
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-logue.cc (rs6000_emit_epilogue): As
> EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
> now, adjust the relevant handlings on it.
> * config/rs6000/rs6000.md (eh_return expander): Append by calling
> gen_eh_return_internal and emit_barrier.
> (eh_return_internal): New define_insn_and_split, call function
> rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/powerpc/pr114846.c: New test.
> ---
> gcc/config/rs6000/rs6000-logue.cc | 7 +++----
> gcc/config/rs6000/rs6000.md | 15 +++++++++++++++
> gcc/testsuite/gcc.target/powerpc/pr114846.c | 20 ++++++++++++++++++++
> 3 files changed, 38 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr114846.c
>
> diff --git a/gcc/config/rs6000/rs6000-logue.cc
> b/gcc/config/rs6000/rs6000-logue.cc
> index 60ba15a8bc3..bd5d56ba002 100644
> --- a/gcc/config/rs6000/rs6000-logue.cc
> +++ b/gcc/config/rs6000/rs6000-logue.cc
> @@ -4308,9 +4308,6 @@ rs6000_emit_epilogue (enum epilogue_type
> epilogue_type)
>
> rs6000_stack_t *info = rs6000_stack_info ();
>
> - if (epilogue_type == EPILOGUE_TYPE_NORMAL && crtl->calls_eh_return)
> - epilogue_type = EPILOGUE_TYPE_EH_RETURN;
> -
> int strategy = info->savres_strategy;
> bool using_load_multiple = !!(strategy & REST_MULTIPLE);
> bool restoring_GPRs_inline = !!(strategy & REST_INLINE_GPRS);
> @@ -4788,7 +4785,9 @@ rs6000_emit_epilogue (enum epilogue_type
> epilogue_type)
>
> /* In the ELFv2 ABI we need to restore all call-saved CR fields from
> *separate* slots if the routine calls __builtin_eh_return, so
> - that they can be independently restored by the unwinder. */
> + that they can be independently restored by the unwinder. Since
> + it is for CR fields restoring, it should be done for any epilogue
> + types (not EPILOGUE_TYPE_EH_RETURN specific). */
> if (DEFAULT_ABI == ABI_ELFv2 && crtl->calls_eh_return)
> {
> int i, cr_off = info->ehcr_offset;
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index ac5651d7420..d4120c3b9ce 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -14281,6 +14281,8 @@ (define_expand "eh_return"
> ""
> {
> emit_insn (gen_eh_set_lr (Pmode, operands[0]));
> + emit_jump_insn (gen_eh_return_internal ());
> + emit_barrier ();
> DONE;
> })
>
> @@ -14297,6 +14299,19 @@ (define_insn_and_split "@eh_set_lr_<mode>"
> DONE;
> })
>
> +(define_insn_and_split "eh_return_internal"
> + [(eh_return)]
> + ""
> + "#"
> + "epilogue_completed"
> + [(const_int 0)]
> +{
> + if (!TARGET_SCHED_PROLOG)
> + emit_insn (gen_blockage ());
> + rs6000_emit_epilogue (EPILOGUE_TYPE_EH_RETURN);
> + DONE;
> +})
> +
> (define_insn "prefetch"
> [(prefetch (match_operand 0 "indexed_or_indirect_address" "a")
> (match_operand:SI 1 "const_int_operand" "n")
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr114846.c
> b/gcc/testsuite/gcc.target/powerpc/pr114846.c
> new file mode 100644
> index 00000000000..efe2300b73a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr114846.c
> @@ -0,0 +1,20 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target builtin_eh_return } */
> +
> +/* Ensure it runs successfully. */
> +
> +__attribute__ ((noipa))
> +int f (int *a, long offset, void *handler)
> +{
> + if (*a == 5)
> + return 5;
> + __builtin_eh_return (offset, handler);
> +}
> +
> +int main ()
> +{
> + int t = 5;
> + if (f (&t, 0, 0) != 5)
> + __builtin_abort ();
> + return 0;
> +}
> --
> 2.39.3
>
