committed to trunk :)
On Mon, May 19, 2025 at 11:49 AM Kito Cheng <kito.ch...@gmail.com> wrote:
> 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
> >
>
