Re: [PATCH v9] RISC-V: Add the 'zfa' extension, version 0.2

[Kito Cheng <kito.cheng@gmail.com>]

zfa requires/depend f, it means zfa implies f in current toolchain
implementation, could you add that into riscv-common.cc?

Also that means zfa is exclusive with Z[FDH]INX.

Ref: https://github.com/riscv/riscv-isa-manual/issues/1020

On Tue, May 16, 2023 at 3:06 PM jinma <jinma@linux.alibaba.com> wrote:
>
> On 5/15/23 07:16, Jin Ma wrote:
> > > This patch adds the 'Zfa' extension for riscv, which is based on:
> > > https://github.com/riscv/riscv-isa-manual/commits/zfb
> > >
> > > The binutils-gdb for 'Zfa' extension:
> > > https://sourceware.org/pipermail/binutils/2023-April/127060.html
> > >
> > > What needs special explanation is:
> > > 1, The immediate number of the instructions FLI.H/S/D is represented in the assembly as a
> > >    floating-point value, with scientific counting when rs1 is 2,3, and decimal numbers for
> > >    the rest.
> > >
> > >    Related llvm link:
> > >      https://reviews.llvm.org/D145645
> > >    Related discussion link:
> > >      https://github.com/riscv/riscv-isa-manual/issues/980
> > >
> > > 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added principally to
> > >    accelerate the processing of JavaScript Numbers.", so it seems that no implementation
> > >    is required.
> > >
> > > 3, The instructions FMINM and FMAXM correspond to C23 library function fminimum and fmaximum.
> > >    Therefore, this patch has simply implemented the pattern of fminm<hf\sf\df>3 and
> > >    fmaxm<hf\sf\df>3 to prepare for later.
> > >
> > > gcc/ChangeLog:
> > >
> > >  * common/config/riscv/riscv-common.cc: Add zfa extension version.
> > >  * config/riscv/constraints.md (zfli): Constrain the floating point number that the
> > >  instructions FLI.H/S/D can load.
> > >  * config/riscv/iterators.md (ceil): New.
> > >  (rup): New.
> > >  * config/riscv/riscv-opts.h (MASK_ZFA): New.
> > >  (TARGET_ZFA): New.
> > >  * config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): New.
> > >  * config/riscv/riscv.cc (riscv_float_const_rtx_index_for_fli): New.
> > >  (riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, memory is not applicable.
> > >  (riscv_const_insns): Likewise.
> > >  (riscv_legitimize_const_move): Likewise.
> > >  (riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no split is required.
> > >  (riscv_split_doubleword_move): Likewise.
> > >  (riscv_output_move): Output the mov instructions in zfa extension.
> > >  (riscv_print_operand): Output the floating-point value of the FLI.H/S/D immediate in assembly
> > >  (riscv_secondary_memory_needed): Likewise.
> > >  * config/riscv/riscv.md (fminm<mode>3): New.
> > >  (fmaxm<mode>3): New.
> > >  (movsidf2_low_rv32): New.
> > >  (movsidf2_high_rv32): New.
> > >  (movdfsisi3_rv32): New.
> > >  (f<quiet_pattern>_quiet<ANYF:mode><X:mode>4_zfa): Likewise.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >  * gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
> > >  * gcc.target/riscv/zfa-fleq-fltq.c: New test.
> > >  * gcc.target/riscv/zfa-fli-rv32.c: New test.
> > >  * gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
> > >  * gcc.target/riscv/zfa-fli-zfh.c: New test.
> > >  * gcc.target/riscv/zfa-fli.c: New test.
> > >  * gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
> > >  * gcc.target/riscv/zfa-fround-rv32.c: New test.
> > >  * gcc.target/riscv/zfa-fround.c: New test.
> > > ---
> > >   gcc/common/config/riscv/riscv-common.cc       |   4 +
> > >   gcc/config/riscv/constraints.md               |  21 +-
> > >   gcc/config/riscv/iterators.md                 |   5 +
> > >   gcc/config/riscv/riscv-opts.h                 |   3 +
> > >   gcc/config/riscv/riscv-protos.h               |   1 +
> > >   gcc/config/riscv/riscv.cc                     | 204 +++++++++++++++++-
> > >   gcc/config/riscv/riscv.md                     | 145 +++++++++++--
> > >   .../gcc.target/riscv/zfa-fleq-fltq-rv32.c     |  19 ++
> > >   .../gcc.target/riscv/zfa-fleq-fltq.c          |  19 ++
> > >   gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c |  79 +++++++
> > >   .../gcc.target/riscv/zfa-fli-zfh-rv32.c       |  41 ++++
> > >   gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c  |  41 ++++
> > >   gcc/testsuite/gcc.target/riscv/zfa-fli.c      |  79 +++++++
> > >   .../gcc.target/riscv/zfa-fmovh-fmovp-rv32.c   |  10 +
> > >   .../gcc.target/riscv/zfa-fround-rv32.c        |  42 ++++
> > >   gcc/testsuite/gcc.target/riscv/zfa-fround.c   |  42 ++++
> > >   16 files changed, 719 insertions(+), 36 deletions(-)
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq-rv32.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh-rv32.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fmovh-fmovp-rv32.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround-rv32.c
> > >   create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround.c
> > >
> >
> >
> > > +
> > > +/* Return index of the FLI instruction table if rtx X is an immediate constant that can
> > > +   be moved using a single FLI instruction in zfa extension. Return -1 if not found.  */
> > > +
> > > +int
> > > +riscv_float_const_rtx_index_for_fli (rtx x)
> > > +{
> > > +  unsigned HOST_WIDE_INT *fli_value_array;
> > > +
> > > +  machine_mode mode = GET_MODE (x);
> > > +
> > > +  if (!TARGET_ZFA
> > > +      || !CONST_DOUBLE_P(x)
> > > +      || mode == VOIDmode
> > > +      || (mode == HFmode && !TARGET_ZFH)
> > > +      || (mode == SFmode && !TARGET_HARD_FLOAT)
> > > +      || (mode == DFmode && !TARGET_DOUBLE_FLOAT))
> > > +    return -1;
> > Do we also need to check Z[FDH]INX too?
> >
> > Otherwise it looks pretty good.  We just need to wait for everything to
> > freeze and finalization on the assembler interface.
> >
> > jeff
>
> Yes, you are right, we also need to check Z[FDH]INX. I will send a patch
> again to fix it after others give some review comments.
>
> Jin