On Sat, May 17, 2025 at 9:34 PM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
> On 5/14/25 9:14 PM, Kito Cheng wrote:
> > This commit adds the code gen support for Zilsd, which is a
> > newly added extension for RISC-V. The Zilsd extension allows
> > for loading and storing 64-bit values using even-odd register
> > pairs.
> >
> > We only try to do miminal code gen support for that, which means only
> > use the new instructions when the load store is 64 bits data, we can use
> > that to optimize the code gen of memcpy/memset/memmove and also the
> > prologue and epilogue of functions, but I think that probably should be
> > done in a follow up patch.
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv.cc (riscv_legitimize_move): Handle
> > load/store with odd-even reg pair.
> > (riscv_split_64bit_move_p): Don't split load/store if zilsd enabled.
> > (riscv_hard_regno_mode_ok): Only allow even reg can be used for
> > 64 bits mode for zilsd.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/zilsd-code-gen.c: New test.
> > ---
> > gcc/config/riscv/riscv.cc | 38 +++++++++++++++++++
> > .../gcc.target/riscv/zilsd-code-gen.c | 18 +++++++++
> > 2 files changed, 56 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.target/riscv/zilsd-code-gen.c
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index d28aee4b439..f5ee3ce9034 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -3742,6 +3742,25 @@ riscv_legitimize_move (machine_mode mode, rtx dest,
> > rtx src)
> > return true;
> > }
> >
> > + if (TARGET_ZILSD
> > + && (GET_MODE_UNIT_SIZE (mode) == (UNITS_PER_WORD * 2))
> > + && ((REG_P (dest) && MEM_P (src))
> > + || (MEM_P (dest) && REG_P (src))))
> > + {
> > + rtx reg = REG_P (dest) ? dest : src;
> > + unsigned regno = REGNO (reg);
> > + /* ZILSD require even-odd register pair, let RA to
> > + fix the constraint if the reg is hard reg and not even reg. */
> > + if ((regno < FIRST_PSEUDO_REGISTER)
> > + && (regno % 2) != 0)
> > + {
> > + rtx tmp = gen_reg_rtx (GET_MODE (reg));
> > + emit_move_insn (tmp, src);
> > + emit_move_insn (dest, tmp);
> > + return true;
> > + }
> AFAICT this will only ever be called by the various movXX expanders, but
> those can be called during IRA, so we probably should a bit safer here.
>
> We could either add can_create_pseudo_p to the guard or we could assert
> it's true. The former would be most appropriate if the rest of the code
> will still do the right thing, the latter if not.
Good suggestion, I guess just adding can_create_pseudo_p in the
if-condition would be fine,
we already have a splitter later to handle those cases we can't handle here.
>
>
> > @@ -9799,6 +9831,12 @@ riscv_hard_regno_mode_ok (unsigned int regno,
> > machine_mode mode)
> > if (riscv_v_ext_mode_p (mode))
> > return false;
> >
> > + /* Zilsd require load/store with even-odd reg pair. */
> > + if (TARGET_ZILSD
> > + && (GET_MODE_UNIT_SIZE (mode) == (UNITS_PER_WORD * 2))
> > + && ((regno % 2) != 0))
> > + return false;
> Do you need to check that you're working with a GPR here?
We have checked that few lines before, so we are safe here, but it's
not easy to observe from the diff since we have only 3 lines before,
hope one day we can migrate to something like github...
>
> At a higher level, my understanding is zilsd is only for rv32. Do we
> want to be extra safe and check TARGET_32BIT alongside TARGET_ZILSD?
We have checked that during arch string parsing, so I'm inclined not
to check that again in other places :)
>
> Take the action you feel is appropriate on the above issues and the
> result is pre-approved for the trunk.
Thanks for reviewing, I will commit after applying those small changes
and testing :)
>
> Thanks,
> jeff
>
