Re: [PATCH] RISC-V: setjmp: reduce code size for register load/store with Zilsd

Hi Kito,
I generally tested the Zilsd implementation by allowing misaligned memory
accesses in spike, so guarding the implementation with *__riscv_misaligned_fast
*didn't strike my mind. I will send in the fixed version.
Thanks for the feedback,
Vinit Puranik

On Wed, 28 May 2025 at 11:58, Kito Cheng <[email protected]> wrote:

> Hi puranikvinit:
>
> Thanks for your patch! it's generally LGTM, but I am concerned about
> the alignment issue, zilsd spec only guarantees that 16-byte alignment
> won't cause a misaligned trap, so it may be guarded with
> __riscv_misaligned_fast?
>
> [1]
> https://github.com/riscv/riscv-zilsd/blob/main/zilsd.adoc#loadstore-pair-instructions-zilsd
>
> On Wed, May 28, 2025 at 1:45 PM puranikvinit <[email protected]>
> wrote:
> >
> > This patch optimizes the RISC-V setjmp implementation in
> > newlib/libc/machine/riscv/setjmp.S for 32-bit targets. It reduces code
> > size by using doubleword store/load instructions (sd/ld) when the Zilsd
> > or Zclsd extensions are available for saving and
> > restoring s0-s11 registers, while preserving the original
> > single-word instructions (REG_S/REG_L) for compatibility with other
> > configurations.
> >
> > Signed-off-by: puranikvinit <[email protected]>
> > Reviewed-by: Christian Herber <[email protected]>
> >
> > RISC-V: setjmp: remove redundant checks for Zclsd
> > ---
> >  newlib/libc/machine/riscv/setjmp.S | 74 ++++++++++++++++++++----------
> >  1 file changed, 49 insertions(+), 25 deletions(-)
> >
> > diff --git a/newlib/libc/machine/riscv/setjmp.S
> b/newlib/libc/machine/riscv/setjmp.S
> > index 2d4ab6cfc..d5b132466 100644
> > --- a/newlib/libc/machine/riscv/setjmp.S
> > +++ b/newlib/libc/machine/riscv/setjmp.S
> > @@ -16,21 +16,33 @@
> >    .type   setjmp, @function
> >  setjmp:
> >         REG_S ra,  0*SZREG(a0)
> > -       REG_S s0,  1*SZREG(a0)
> > -       REG_S s1,  2*SZREG(a0)
> > +  #if __riscv_xlen == 32 && (__riscv_zilsd)
> > +         sd    s0,  1*SZREG(a0)
> > +  #else
> > +         REG_S s0,  1*SZREG(a0)
> > +         REG_S s1,  2*SZREG(a0)
> > +  #endif
> >
> >  #ifndef __riscv_abi_rve
> > -       REG_S s2,  3*SZREG(a0)
> > -       REG_S s3,  4*SZREG(a0)
> > -       REG_S s4,  5*SZREG(a0)
> > -       REG_S s5,  6*SZREG(a0)
> > -       REG_S s6,  7*SZREG(a0)
> > -       REG_S s7,  8*SZREG(a0)
> > -       REG_S s8,  9*SZREG(a0)
> > -       REG_S s9, 10*SZREG(a0)
> > -       REG_S s10,11*SZREG(a0)
> > -       REG_S s11,12*SZREG(a0)
> > -       REG_S sp, 13*SZREG(a0)
> > +  #if __riscv_xlen == 32 && (__riscv_zilsd)
> > +         sd    s2,  3*SZREG(a0)
> > +         sd    s4,  5*SZREG(a0)
> > +         sd    s6,  7*SZREG(a0)
> > +         sd    s8,  9*SZREG(a0)
> > +         sd    s10,11*SZREG(a0)
> > +  #else
> > +         REG_S s2,  3*SZREG(a0)
> > +         REG_S s3,  4*SZREG(a0)
> > +         REG_S s4,  5*SZREG(a0)
> > +         REG_S s5,  6*SZREG(a0)
> > +         REG_S s6,  7*SZREG(a0)
> > +         REG_S s7,  8*SZREG(a0)
> > +         REG_S s8,  9*SZREG(a0)
> > +         REG_S s9, 10*SZREG(a0)
> > +         REG_S s10,11*SZREG(a0)
> > +         REG_S s11,12*SZREG(a0)
> > +  #endif
> > +  REG_S sp, 13*SZREG(a0)
> >  #else
> >         REG_S sp, 3*SZREG(a0)
> >  #endif
> > @@ -59,19 +71,31 @@ setjmp:
> >    .type   longjmp, @function
> >  longjmp:
> >         REG_L ra,  0*SZREG(a0)
> > -       REG_L s0,  1*SZREG(a0)
> > -       REG_L s1,  2*SZREG(a0)
> > +  #if __riscv_xlen == 32 && (__riscv_zilsd)
> > +    ld s0, 1*SZREG(a0)
> > +  #else
> > +    REG_L s0,  1*SZREG(a0)
> > +    REG_L s1,  2*SZREG(a0)
> > +  #endif
> >  #ifndef __riscv_abi_rve
> > -       REG_L s2,  3*SZREG(a0)
> > -       REG_L s3,  4*SZREG(a0)
> > -       REG_L s4,  5*SZREG(a0)
> > -       REG_L s5,  6*SZREG(a0)
> > -       REG_L s6,  7*SZREG(a0)
> > -       REG_L s7,  8*SZREG(a0)
> > -       REG_L s8,  9*SZREG(a0)
> > -       REG_L s9, 10*SZREG(a0)
> > -       REG_L s10,11*SZREG(a0)
> > -       REG_L s11,12*SZREG(a0)
> > +  #if __riscv_xlen == 32 && (__riscv_zilsd)
> > +         ld    s2,  3*SZREG(a0)
> > +         ld    s4,  5*SZREG(a0)
> > +         ld    s6,  7*SZREG(a0)
> > +         ld    s8,  9*SZREG(a0)
> > +         ld    s10,11*SZREG(a0)
> > +  #else
> > +         REG_L s2,  3*SZREG(a0)
> > +         REG_L s3,  4*SZREG(a0)
> > +         REG_L s4,  5*SZREG(a0)
> > +         REG_L s5,  6*SZREG(a0)
> > +         REG_L s6,  7*SZREG(a0)
> > +         REG_L s7,  8*SZREG(a0)
> > +         REG_L s8,  9*SZREG(a0)
> > +         REG_L s9, 10*SZREG(a0)
> > +         REG_L s10,11*SZREG(a0)
> > +         REG_L s11,12*SZREG(a0)
> > +  #endif
> >         REG_L sp, 13*SZREG(a0)
> >  #else
> >         REG_L sp, 3*SZREG(a0)
> > --
> > 2.49.0
> >
>