On Sun, Oct 29, 2023 at 11:25 PM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
> On 10/20/23 03:53, Christoph Muellner wrote:
> > From: Christoph Müllner <christoph.muell...@vrull.eu>
> >
> > The XTheadFMemIdx ISA extension provides additional load and store
> > instructions for floating-point registers with new addressing modes.
> >
> > The following memory accesses types are supported:
> > * load/store: [w,d] (single-precision FP, double-precision FP)
> >
> > The following addressing modes are supported:
> > * register offset with additional immediate offset (4 instructions):
> > flr<type>, fsr<type>
> > * zero-extended register offset with additional immediate offset
> > (4 instructions): flur<type>, fsur<type>
> >
> > These addressing modes are also part of the similar XTheadMemIdx
> > ISA extension support, whose code is reused and extended to support
> > floating-point registers.
> >
> > One challenge that this patch needs to solve are GP registers in FP-mode
> > (e.g. "(reg:DF a2)"), which cannot be handled by the XTheadFMemIdx
> > instructions. Such registers are the result of independent
> > optimizations, which can happen after register allocation.
> > This patch uses a simple but efficient method to address this:
> > add a dependency for XTheadMemIdx to XTheadFMemIdx optimizations.
> > This allows to use the instructions from XTheadMemIdx in case
> > of such registers.
> Or alternately define secondary reloads so that you can get a scratch
> register to reload the address into a GPR. Your call on whether or not
> to try to implement that. I guess it largely depends on how likely it
> is you'll have one extension defined, but not the other.
I started doing this but I thought it is not worth the effort,
given all cores that implement one extension also support the other.
> > The added tests ensure that this feature won't regress without notice.
> > Testing: GCC regression test suite and SPEC CPU 2017 intrate (base&peak).
> >
> > Signed-off-by: Christoph Müllner <christoph.muell...@vrull.eu>
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv.cc (riscv_index_reg_class):
> > Return GR_REGS for XTheadFMemIdx.
> > (riscv_regno_ok_for_index_p): Add support for XTheadFMemIdx.
> > * config/riscv/riscv.h (HARDFP_REG_P): New macro.
> > * config/riscv/thead.cc (is_fmemidx_mode): New function.
> > (th_memidx_classify_address_index): Add support for XTheadFMemIdx.
> > (th_fmemidx_output_index): New function.
> > (th_output_move): Add support for XTheadFMemIdx.
> > * config/riscv/thead.md (TH_M_ANYF): New mode iterator.
> > (TH_M_NOEXTF): Likewise.
> > (*th_fmemidx_movsf_hardfloat): New INSN.
> > (*th_fmemidx_movdf_hardfloat_rv64): Likewise.
> > (*th_fmemidx_I_a): Likewise.
> > (*th_fmemidx_I_c): Likewise.
> > (*th_fmemidx_US_a): Likewise.
> > (*th_fmemidx_US_c): Likewise.
> > (*th_fmemidx_UZ_a): Likewise.
> > (*th_fmemidx_UZ_c): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/xtheadfmemidx-index-update.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-index-xtheadbb-update.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-index-xtheadbb.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-index.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-uindex-update.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-uindex-xtheadbb-update.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-uindex-xtheadbb.c: New test.
> > * gcc.target/riscv/xtheadfmemidx-uindex.c: New test.
> > ---
> Same note as with the prior patch WRT wrapping assembly instructions
> when using scan-assembler.
Will do.
>
>
>
> > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> > index eb162abcb92..1e9813b4f39 100644
> > --- a/gcc/config/riscv/riscv.h
> > +++ b/gcc/config/riscv/riscv.h
> > @@ -372,6 +372,8 @@ ASM_MISA_SPEC
> > ((unsigned int) ((int) (REGNO) - GP_REG_FIRST) < GP_REG_NUM)
> > #define FP_REG_P(REGNO) \
> > ((unsigned int) ((int) (REGNO) - FP_REG_FIRST) < FP_REG_NUM)
> > +#define HARDFP_REG_P(REGNO) \
> > + ((REGNO) >= FP_REG_FIRST && (REGNO) <= FP_REG_LAST)
> > #define V_REG_P(REGNO) \
> > ((unsigned int) ((int) (REGNO) - V_REG_FIRST) < V_REG_NUM)
> > #define VL_REG_P(REGNO) ((REGNO) == VL_REGNUM)
>
> > @@ -755,6 +768,40 @@ th_memidx_output_index (rtx x, machine_mode mode, bool
> > load)
> > return buf;
> > }
> >
> > +/* Provide a buffer for a th.flX/th.fluX/th.fsX/th.fsuX instruction
> > + for the given MODE. If LOAD is true, a load instruction will be
> > + provided (otherwise, a store instruction). If X is not suitable
> > + return NULL. */
> > +
> > +static const char *
> > +th_fmemidx_output_index (rtx x, machine_mode mode, bool load)
> > +{
> > + struct riscv_address_info info;
> > + static char buf[128] = {0};
> Same comment WRT static buffers as in the previous patch.
>
> OK for the trunk after fixing the testcases and potentially adjusting
> the static buffer. No need to get another review round, post for for
> the archiver and commit.
Thanks!
>
> jeff
