public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Lulu Cheng <chenglulu@loongson.cn>
To: gcc-patches@gcc.gnu.org
Cc: xry111@xry111.site, i@xen0n.name, xuchenghua@loongson.cn,
	Yang Yujie <yangyujie@loongson.cn>
Subject: Re: [pushed][PATCH][gcc-13] LoongArch: Fix eh_return epilogue for normal returns.
Date: Tue, 30 Apr 2024 09:15:06 +0800	[thread overview]
Message-ID: <ac3b335f-ada4-ff2b-af9a-cda3a1a0a054@loongson.cn> (raw)
In-Reply-To: <20240429080949.32621-1-chenglulu@loongson.cn>

Pushed to r13-8661.

在 2024/4/29 下午4:09, Lulu Cheng 写道:
> From: Yang Yujie <yangyujie@loongson.cn>
>
> On LoongArch, the regitsters $r4 - $r7 (EH_RETURN_DATA_REGNO) will be saved
> and restored in the function prologue and epilogue if the given function calls
> __builtin_eh_return.  This causes the return value to be overwritten on normal
> return paths and breaks a rare case of libgcc's _Unwind_RaiseException.
>
> gcc/ChangeLog:
>
> 	PR target/114848
> 	* config/loongarch/loongarch.cc: Do not restore the saved eh_return
> 	data registers ($r4-$r7) for a normal return of a function that calls
> 	__builtin_eh_return elsewhere.
> 	* config/loongarch/loongarch-protos.h: Same.
> 	* config/loongarch/loongarch.md: Same.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/loongarch/eh_return-normal-return.c: New test.
>
> (cherry picked from commit 4b421728289e6f1caa0dfaa953a11698ab95d37d)
> ---
>   gcc/config/loongarch/loongarch-protos.h       |  2 +-
>   gcc/config/loongarch/loongarch.cc             | 35 ++++++++++++-----
>   gcc/config/loongarch/loongarch.md             | 23 ++++++++++-
>   .../loongarch/eh_return-normal-return.c       | 38 +++++++++++++++++++
>   4 files changed, 85 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/loongarch/eh_return-normal-return.c
>
> diff --git a/gcc/config/loongarch/loongarch-protos.h b/gcc/config/loongarch/loongarch-protos.h
> index 35cc77c7367..0f608ee5179 100644
> --- a/gcc/config/loongarch/loongarch-protos.h
> +++ b/gcc/config/loongarch/loongarch-protos.h
> @@ -60,7 +60,7 @@ enum loongarch_symbol_type {
>   extern rtx loongarch_emit_move (rtx, rtx);
>   extern HOST_WIDE_INT loongarch_initial_elimination_offset (int, int);
>   extern void loongarch_expand_prologue (void);
> -extern void loongarch_expand_epilogue (bool);
> +extern void loongarch_expand_epilogue (int);
>   extern bool loongarch_can_use_return_insn (void);
>   \f
>   extern bool loongarch_symbolic_constant_p (rtx, enum loongarch_symbol_type *);
> diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
> index f47a5fc2ad7..2238858cd6a 100644
> --- a/gcc/config/loongarch/loongarch.cc
> +++ b/gcc/config/loongarch/loongarch.cc
> @@ -1012,7 +1012,8 @@ loongarch_save_restore_reg (machine_mode mode, int regno, HOST_WIDE_INT offset,
>   
>   static void
>   loongarch_for_each_saved_reg (HOST_WIDE_INT sp_offset,
> -			      loongarch_save_restore_fn fn)
> +			      loongarch_save_restore_fn fn,
> +			      bool skip_eh_data_regs_p)
>   {
>     HOST_WIDE_INT offset;
>   
> @@ -1021,7 +1022,15 @@ loongarch_for_each_saved_reg (HOST_WIDE_INT sp_offset,
>     for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++)
>       if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
>         {
> -	loongarch_save_restore_reg (word_mode, regno, offset, fn);
> +	/* Special care needs to be taken for $r4-$r7 (EH_RETURN_DATA_REGNO)
> +	   when returning normally from a function that calls
> +	   __builtin_eh_return.  In this case, these registers are saved but
> +	   should not be restored, or the return value may be clobbered.  */
> +
> +	if (!(skip_eh_data_regs_p
> +	      && GP_ARG_FIRST <= regno && regno < GP_ARG_FIRST + 4))
> +	  loongarch_save_restore_reg (word_mode, regno, offset, fn);
> +
>   	offset -= UNITS_PER_WORD;
>         }
>   
> @@ -1290,7 +1299,7 @@ loongarch_expand_prologue (void)
>   			    GEN_INT (-step1));
>         RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
>         size -= step1;
> -      loongarch_for_each_saved_reg (size, loongarch_save_reg);
> +      loongarch_for_each_saved_reg (size, loongarch_save_reg, false);
>       }
>   
>     /* Set up the frame pointer, if we're using one.  */
> @@ -1375,11 +1384,13 @@ loongarch_can_use_return_insn (void)
>     return reload_completed && cfun->machine->frame.total_size == 0;
>   }
>   
> -/* Expand an "epilogue" or "sibcall_epilogue" pattern; SIBCALL_P
> -   says which.  */
> +/* Expand function epilogue using the following insn patterns:
> +   "epilogue"	      (style == NORMAL_RETURN)
> +   "sibcall_epilogue" (style == SIBCALL_RETURN)
> +   "eh_return"	      (style == EXCEPTION_RETURN) */
>   
>   void
> -loongarch_expand_epilogue (bool sibcall_p)
> +loongarch_expand_epilogue (int style)
>   {
>     /* Split the frame into two.  STEP1 is the amount of stack we should
>        deallocate before restoring the registers.  STEP2 is the amount we
> @@ -1396,7 +1407,8 @@ loongarch_expand_epilogue (bool sibcall_p)
>     bool need_barrier_p
>       = (get_frame_size () + cfun->machine->frame.arg_pointer_offset) != 0;
>   
> -  if (!sibcall_p && loongarch_can_use_return_insn ())
> +  /* Handle simple returns.  */
> +  if (style == NORMAL_RETURN && loongarch_can_use_return_insn ())
>       {
>         emit_jump_insn (gen_return ());
>         return;
> @@ -1472,7 +1484,9 @@ loongarch_expand_epilogue (bool sibcall_p)
>   
>     /* Restore the registers.  */
>     loongarch_for_each_saved_reg (frame->total_size - step2,
> -				loongarch_restore_reg);
> +				loongarch_restore_reg,
> +				crtl->calls_eh_return
> +				&& style != EXCEPTION_RETURN);
>   
>     if (need_barrier_p)
>       loongarch_emit_stack_tie ();
> @@ -1493,11 +1507,12 @@ loongarch_expand_epilogue (bool sibcall_p)
>       }
>   
>     /* Add in the __builtin_eh_return stack adjustment.  */
> -  if (crtl->calls_eh_return)
> +  if (crtl->calls_eh_return && style == EXCEPTION_RETURN)
>       emit_insn (gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
>   			      EH_RETURN_STACKADJ_RTX));
>   
> -  if (!sibcall_p)
> +  /* Emit return unless doing sibcall.  */
> +  if (style != SIBCALL_RETURN)
>       emit_jump_insn (gen_simple_return_internal (ra));
>   }
>   
> diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md
> index 830eaaea90e..acf1269b402 100644
> --- a/gcc/config/loongarch/loongarch.md
> +++ b/gcc/config/loongarch/loongarch.md
> @@ -115,6 +115,11 @@ (define_constants
>      (T1_REGNUM			13)
>      (S0_REGNUM			23)
>   
> +   ;; Return path styles
> +   (NORMAL_RETURN		0)
> +   (SIBCALL_RETURN		1)
> +   (EXCEPTION_RETURN		2)
> +
>      ;; PIC long branch sequences are never longer than 100 bytes.
>      (MAX_PIC_BRANCH_LENGTH	100)
>   ])
> @@ -2947,7 +2952,7 @@ (define_expand "epilogue"
>     [(const_int 2)]
>     ""
>   {
> -  loongarch_expand_epilogue (false);
> +  loongarch_expand_epilogue (NORMAL_RETURN);
>     DONE;
>   })
>   
> @@ -2955,7 +2960,7 @@ (define_expand "sibcall_epilogue"
>     [(const_int 2)]
>     ""
>   {
> -  loongarch_expand_epilogue (true);
> +  loongarch_expand_epilogue (SIBCALL_RETURN);
>     DONE;
>   })
>   
> @@ -3012,6 +3017,20 @@ (define_expand "eh_return"
>       emit_insn (gen_eh_set_ra_di (operands[0]));
>     else
>       emit_insn (gen_eh_set_ra_si (operands[0]));
> +
> +  emit_jump_insn (gen_eh_return_internal ());
> +  emit_barrier ();
> +  DONE;
> +})
> +
> +(define_insn_and_split "eh_return_internal"
> +  [(eh_return)]
> +  ""
> +  "#"
> +  "epilogue_completed"
> +  [(const_int 0)]
> +{
> +  loongarch_expand_epilogue (EXCEPTION_RETURN);
>     DONE;
>   })
>   
> diff --git a/gcc/testsuite/gcc.target/loongarch/eh_return-normal-return.c b/gcc/testsuite/gcc.target/loongarch/eh_return-normal-return.c
> new file mode 100644
> index 00000000000..f8f3965f894
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/loongarch/eh_return-normal-return.c
> @@ -0,0 +1,38 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include <stdlib.h>
> +
> +int foo ()  __attribute__((noinline));
> +int main ();
> +
> +int
> +foo () {
> +
> +  int t;
> +
> +  /* prevent optimization using asm */
> +  asm ("" : "=r" (t) : "0" (-1));
> +  asm ("" : "=r" (t) : "0" (t ? 1 : 0));
> +
> +  if (t == 0)
> +    /* never reached */
> +    __builtin_eh_return (0, __builtin_return_address (0));
> +
> +  else if (t == 1)
> +    /* return here */
> +    return 202312;
> +
> +  else
> +    /* never reached: prevent vrp optimization in main */
> +    return 0;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo() == 202312)
> +    return 0;
> +  else
> +    abort ();
> +}


      reply	other threads:[~2024-04-30  1:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29  8:09 [PATCH][gcc-13] " Lulu Cheng
2024-04-30  1:15 ` Lulu Cheng [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac3b335f-ada4-ff2b-af9a-cda3a1a0a054@loongson.cn \
    --to=chenglulu@loongson.cn \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=i@xen0n.name \
    --cc=xry111@xry111.site \
    --cc=xuchenghua@loongson.cn \
    --cc=yangyujie@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).