Re: [PATCH v4] LoongArch: Fix eh_return epilogue for normal returns
Sorry, this is the wrong patch. I will post it again. On Fri, Dec 08, 2023 at 05:57:12PM +0800, Yang Yujie wrote: > Updates: > v1 -> v2: Add a test case. > v2 -> v3: Fix code format. > v3 -> v4: Fix code format. Avoid unwanted optimization in the test. > > On Fri, Dec 08, 2023 at 05:54:46PM +0800, Yang Yujie wrote: > > 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: > > > > * 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. > > --- > > gcc/config/loongarch/loongarch-protos.h | 2 +- > > gcc/config/loongarch/loongarch.cc | 41 --- > > gcc/config/loongarch/loongarch.md | 18 +++- > > .../loongarch/eh_return-normal-return.c | 32 +++ > > 4 files changed, 76 insertions(+), 17 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 cb8fc36b086..af20b5d7132 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); > > > > 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 3545e66a10e..9c0e0dd1b73 100644 > > --- a/gcc/config/loongarch/loongarch.cc > > +++ b/gcc/config/loongarch/loongarch.cc > > @@ -1015,20 +1015,30 @@ 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; > > > >/* Save the link register and s-registers. */ > >offset = cfun->machine->frame.gp_sp_offset - sp_offset; > >for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++) > > -if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > > - { > > - if (!cfun->machine->reg_is_wrapped_separately[regno]) > > - 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. */ > > > > - offset -= UNITS_PER_WORD; > > - } > > + if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > > + { > > + if (!(cfun->machine->reg_is_wrapped_separately[regno] > > + || (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; > > + } > > +} > > > >/* This loop must iterate over the same space as its companion in > > loongarch_compute_frame_info. */ > > @@ -1297,7 +1307,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. */ > > @@ -1382,11 +1392,11 @@ 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 for the following insn patterns: > > + "epilogue" (style == 0) / "sibcall_epilogue" (1) / "eh_return" (2). */ > > > > void > > -loongarch_expand_epilogue (bool
Re: [PATCH v4] LoongArch: Fix eh_return epilogue for normal returns
Updates: v1 -> v2: Add a test case. v2 -> v3: Fix code format. v3 -> v4: Fix code format. Avoid unwanted optimization in the test. On Fri, Dec 08, 2023 at 05:54:46PM +0800, Yang Yujie wrote: > 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: > > * 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. > --- > gcc/config/loongarch/loongarch-protos.h | 2 +- > gcc/config/loongarch/loongarch.cc | 41 --- > gcc/config/loongarch/loongarch.md | 18 +++- > .../loongarch/eh_return-normal-return.c | 32 +++ > 4 files changed, 76 insertions(+), 17 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 cb8fc36b086..af20b5d7132 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); > > 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 3545e66a10e..9c0e0dd1b73 100644 > --- a/gcc/config/loongarch/loongarch.cc > +++ b/gcc/config/loongarch/loongarch.cc > @@ -1015,20 +1015,30 @@ 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; > >/* Save the link register and s-registers. */ >offset = cfun->machine->frame.gp_sp_offset - sp_offset; >for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++) > -if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > - { > - if (!cfun->machine->reg_is_wrapped_separately[regno]) > - 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. */ > > - offset -= UNITS_PER_WORD; > - } > + if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > + { > + if (!(cfun->machine->reg_is_wrapped_separately[regno] > + || (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; > + } > +} > >/* This loop must iterate over the same space as its companion in > loongarch_compute_frame_info. */ > @@ -1297,7 +1307,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. */ > @@ -1382,11 +1392,11 @@ 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 for the following insn patterns: > + "epilogue" (style == 0) / "sibcall_epilogue" (1) / "eh_return" (2). */ > > 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 > @@ -1403,7 +1413,8 @@ loongarch_expand_epilogue (bool sibcall_p) >bool need_barrier_p >
[PATCH v4] LoongArch: Fix eh_return epilogue for normal returns
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: * 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. --- gcc/config/loongarch/loongarch-protos.h | 2 +- gcc/config/loongarch/loongarch.cc | 41 --- gcc/config/loongarch/loongarch.md | 18 +++- .../loongarch/eh_return-normal-return.c | 32 +++ 4 files changed, 76 insertions(+), 17 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 cb8fc36b086..af20b5d7132 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); 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 3545e66a10e..9c0e0dd1b73 100644 --- a/gcc/config/loongarch/loongarch.cc +++ b/gcc/config/loongarch/loongarch.cc @@ -1015,20 +1015,30 @@ 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; /* Save the link register and s-registers. */ offset = cfun->machine->frame.gp_sp_offset - sp_offset; for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++) -if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) - { - if (!cfun->machine->reg_is_wrapped_separately[regno]) - 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. */ - offset -= UNITS_PER_WORD; - } + if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) + { + if (!(cfun->machine->reg_is_wrapped_separately[regno] + || (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; + } +} /* This loop must iterate over the same space as its companion in loongarch_compute_frame_info. */ @@ -1297,7 +1307,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. */ @@ -1382,11 +1392,11 @@ 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 for the following insn patterns: + "epilogue" (style == 0) / "sibcall_epilogue" (1) / "eh_return" (2). */ 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 @@ -1403,7 +1413,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 == 0 && loongarch_can_use_return_insn ()) { emit_jump_insn (gen_return ()); return; @@ -1479,7 +1490,8 @@ loongarch_expand_epilogue (bool sibcall_p) /* Restore the