On Mon, Dec 19, 2022 at 10:26 AM Kito Cheng wrote: > Something like this: > > static unsigned int > riscv_next_saved_reg (unsigned int regno, unsigned int limit, > HOST_WIDE_INT *offset, bool inc = true) > { > if (inc) > regno++; > > while (regno <= limit) > { > if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > { > *offset = *offset - UNITS_PER_WORD; > break; > } > > regno++; > } > if (regno >= limit) > return INVALID_REGNUM; > else > return regno; > } > ... > > for (regno = riscv_next_saved_reg (start, limit, &offset, false); > regno != INVALID_REGNUM; > regno = riscv_next_saved_reg (regno, limit, &offset)) > { > ... > > Ok, I see. I changed it as follows (it will be retested before committing): @@ -5531,7 +5531,8 @@ riscv_save_restore_reg (machine_mode mode, int regno, /* Return the next register up from REGNO up to LIMIT for the callee to save or restore. OFFSET will be adjusted accordingly. - If INC is set, then REGNO will be incremented first. */ + If INC is set, then REGNO will be incremented first. + Returns INVALID_REGNUM if there is no such next register. */ static unsigned int riscv_next_saved_reg (unsigned int regno, unsigned int limit, @@ -5545,12 +5546,12 @@ riscv_next_saved_reg (unsigned int regno, unsigned int limit, if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) { *offset = *offset - UNITS_PER_WORD; - break; + return regno; } regno++; } - return regno; + return INVALID_REGNUM; } /* Return TRUE if provided REGNO is eh return data register. */ @@ -5589,7 +5590,7 @@ riscv_for_each_saved_reg (poly_int64 sp_offset, riscv_save_restore_fn fn, offset = (cfun->machine->frame.gp_sp_offset - sp_offset).to_constant () + UNITS_PER_WORD; for (regno = riscv_next_saved_reg (start, limit, &offset, false); - regno <= limit; + regno != INVALID_REGNUM; Thanks! > On Mon, Dec 19, 2022 at 5:21 PM Christoph Müllner > wrote: > > > > > > > > On Mon, Dec 19, 2022 at 7:30 AM Kito Cheng > wrote: > >> > >> just one more nit: Use INVALID_REGNUM as sentinel value for > >> riscv_next_saved_reg, otherwise LGTM, and feel free to commit that > >> separately :) > > > > > > Would this change below be ok? > > > > @@ -5540,7 +5540,7 @@ riscv_next_saved_reg (unsigned int regno, unsigned > int limit, > > if (inc) > > regno++; > > > > - while (regno <= limit) > > + while (regno <= limit && regno != INVALID_REGNUM) > > { > > if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > > { > > > > Thanks, > > Christoph > > > > > >> > >> > >> On Mon, Dec 19, 2022 at 9:08 AM Christoph Muellner > >> wrote: > >> > > >> > From: Christoph Müllner > >> > > >> > This patch restructures the loop over the GP registers > >> > which saves/restores then as part of the prologue/epilogue. > >> > No functional change is intended by this patch, but it > >> > offers the possibility to use load-pair/store-pair instructions. > >> > > >> > gcc/ChangeLog: > >> > > >> > * config/riscv/riscv.cc (riscv_next_saved_reg): New function. > >> > (riscv_is_eh_return_data_register): New function. > >> > (riscv_for_each_saved_reg): Restructure loop. > >> > > >> > Signed-off-by: Christoph Müllner > >> > --- > >> > gcc/config/riscv/riscv.cc | 94 > +++++++++++++++++++++++++++------------ > >> > 1 file changed, 66 insertions(+), 28 deletions(-) > >> > > >> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > >> > index 6dd2ab2d11e..a8d5e1dac7f 100644 > >> > --- a/gcc/config/riscv/riscv.cc > >> > +++ b/gcc/config/riscv/riscv.cc > >> > @@ -4835,6 +4835,49 @@ riscv_save_restore_reg (machine_mode mode, int > regno, > >> > fn (gen_rtx_REG (mode, regno), mem); > >> > } > >> > > >> > +/* Return the next register up from REGNO up to LIMIT for the callee > >> > + to save or restore. OFFSET will be adjusted accordingly. > >> > + If INC is set, then REGNO will be incremented first. */ > >> > + > >> > +static unsigned int > >> > +riscv_next_saved_reg (unsigned int regno, unsigned int limit, > >> > + HOST_WIDE_INT *offset, bool inc = true) > >> > +{ > >> > + if (inc) > >> > + regno++; > >> > + > >> > + while (regno <= limit) > >> > + { > >> > + if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > >> > + { > >> > + *offset = *offset - UNITS_PER_WORD; > >> > + break; > >> > + } > >> > + > >> > + regno++; > >> > + } > >> > + return regno; > >> > +} > >> > + > >> > +/* Return TRUE if provided REGNO is eh return data register. */ > >> > + > >> > +static bool > >> > +riscv_is_eh_return_data_register (unsigned int regno) > >> > +{ > >> > + unsigned int i, regnum; > >> > + > >> > + if (!crtl->calls_eh_return) > >> > + return false; > >> > + > >> > + for (i = 0; (regnum = EH_RETURN_DATA_REGNO (i)) != INVALID_REGNUM; > i++) > >> > + if (regno == regnum) > >> > + { > >> > + return true; > >> > + } > >> > + > >> > + return false; > >> > +} > >> > + > >> > /* Call FN for each register that is saved by the current function. > >> > SP_OFFSET is the offset of the current stack pointer from the > start > >> > of the frame. */ > >> > @@ -4844,36 +4887,31 @@ riscv_for_each_saved_reg (poly_int64 > sp_offset, riscv_save_restore_fn fn, > >> > bool epilogue, bool maybe_eh_return) > >> > { > >> > HOST_WIDE_INT offset; > >> > + unsigned int regno; > >> > + unsigned int start = GP_REG_FIRST; > >> > + unsigned int limit = GP_REG_LAST; > >> > > >> > /* Save the link register and s-registers. */ > >> > - offset = (cfun->machine->frame.gp_sp_offset - > sp_offset).to_constant (); > >> > - for (unsigned int regno = GP_REG_FIRST; regno <= GP_REG_LAST; > regno++) > >> > - if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST)) > >> > - { > >> > - bool handle_reg = > !cfun->machine->reg_is_wrapped_separately[regno]; > >> > - > >> > - /* If this is a normal return in a function that calls the > eh_return > >> > - builtin, then do not restore the eh return data registers > as that > >> > - would clobber the return value. But we do still need to > save them > >> > - in the prologue, and restore them for an exception return, > so we > >> > - need special handling here. */ > >> > - if (epilogue && !maybe_eh_return && crtl->calls_eh_return) > >> > - { > >> > - unsigned int i, regnum; > >> > - > >> > - for (i = 0; (regnum = EH_RETURN_DATA_REGNO (i)) != > INVALID_REGNUM; > >> > - i++) > >> > - if (regno == regnum) > >> > - { > >> > - handle_reg = FALSE; > >> > - break; > >> > - } > >> > - } > >> > - > >> > - if (handle_reg) > >> > - riscv_save_restore_reg (word_mode, regno, offset, fn); > >> > - offset -= UNITS_PER_WORD; > >> > - } > >> > + offset = (cfun->machine->frame.gp_sp_offset - > sp_offset).to_constant () > >> > + + UNITS_PER_WORD; > >> > + for (regno = riscv_next_saved_reg (start, limit, &offset, false); > >> > + regno <= limit; > >> > + regno = riscv_next_saved_reg (regno, limit, &offset)) > >> > + { > >> > + if (cfun->machine->reg_is_wrapped_separately[regno]) > >> > + continue; > >> > + > >> > + /* If this is a normal return in a function that calls the > eh_return > >> > + builtin, then do not restore the eh return data registers as > that > >> > + would clobber the return value. But we do still need to > save them > >> > + in the prologue, and restore them for an exception return, > so we > >> > + need special handling here. */ > >> > + if (epilogue && !maybe_eh_return > >> > + && riscv_is_eh_return_data_register (regno)) > >> > + continue; > >> > + > >> > + riscv_save_restore_reg (word_mode, regno, offset, fn); > >> > + } > >> > > >> > /* This loop must iterate over the same space as its companion in > >> > riscv_compute_frame_info. */ > >> > -- > >> > 2.38.1 > >> > >