Hi Jeff & Kito,
Thanks for all your review.
We'll Fixed these in next patch version.
Kito Cheng <kito.ch...@gmail.com> 於 2025年7月30日 週三 上午9:48寫道：
>
> On Tue, Jul 22, 2025 at 6:40 AM Jeff Law <jeffreya...@gmail.com> wrote:
> >
> >
> >
> > On 7/11/25 2:57 AM, Kuan-Lin Chen wrote:
> > > This extension defines instructions to perform scalar floating-point
> > > conversion between the BFLOAT16 floating-point data and the IEEE-754
> > > 32-bit single-precision floating-point (SP) data in a scalar
> > > floating point register.
> > >
> > > gcc/ChangeLog:
> > >
> > > * config/riscv/andes.def: Add nds_fcvt_s_bf16 and nds_fcvt_bf16_s.
> > > * config/riscv/andes.md (riscv_nds_fcvt_bf16_s): New pattern.
> > > (riscv_nds_fcvt_s_bf16): New pattern.
> > > * config/riscv/riscv-builtins.cc: New AVAIL andesbfhcvt.
> > > Add new define RISCV_ATYPE_BF and RISCV_ATYPE_SF.
> > > * config/riscv/riscv-ftypes.def: New DEF_RISCV_FTYPE.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/riscv/xandesbfhcvt-1.c: New test.
> > > * gcc.target/riscv/xandesbfhcvt-2.c: New test.
> > > ---
> > > gcc/config/riscv/andes.def | 4 +++
> > > gcc/config/riscv/andes.md | 26 +++++++++++++++++++
> > > gcc/config/riscv/riscv-builtins.cc | 3 +++
> > > gcc/config/riscv/riscv-ftypes.def | 2 ++
> > > .../gcc.target/riscv/xandesbfhcvt-1.c | 11 ++++++++
> > > .../gcc.target/riscv/xandesbfhcvt-2.c | 11 ++++++++
> > > 6 files changed, 57 insertions(+)
> > > create mode 100644 gcc/testsuite/gcc.target/riscv/xandesbfhcvt-1.c
> > > create mode 100644 gcc/testsuite/gcc.target/riscv/xandesbfhcvt-2.c
> > >
> > > diff --git a/gcc/config/riscv/andes.def b/gcc/config/riscv/andes.def
> > > index b864ae712c1d..5b5fb76bfe0e 100644
> > > --- a/gcc/config/riscv/andes.def
> > > +++ b/gcc/config/riscv/andes.def
> > > @@ -8,3 +8,7 @@ RISCV_BUILTIN (nds_ffmismsi, "nds_ffmism",
> > > RISCV_BUILTIN_DIRECT, RISCV_LONG_FTYP
> > > RISCV_BUILTIN (nds_ffmismdi, "nds_ffmism", RISCV_BUILTIN_DIRECT,
> > > RISCV_LONG_FTYPE_ULONG_ULONG, andesperf64),
> > > RISCV_BUILTIN (nds_flmismsi, "nds_flmism", RISCV_BUILTIN_DIRECT,
> > > RISCV_LONG_FTYPE_ULONG_ULONG, andesperf32),
> > > RISCV_BUILTIN (nds_flmismdi, "nds_flmism", RISCV_BUILTIN_DIRECT,
> > > RISCV_LONG_FTYPE_ULONG_ULONG, andesperf64),
> > > +
> > > +/* Andes Scalar BFLOAT16 Conversion Extension */
> > > +RISCV_BUILTIN (nds_fcvt_s_bf16, "nds_fcvt_s_bf16", RISCV_BUILTIN_DIRECT,
> > > RISCV_SF_FTYPE_BF, andesbfhcvt),
> > > +RISCV_BUILTIN (nds_fcvt_bf16_s, "nds_fcvt_bf16_s", RISCV_BUILTIN_DIRECT,
> > > RISCV_BF_FTYPE_SF, andesbfhcvt),
> > > diff --git a/gcc/config/riscv/andes.md b/gcc/config/riscv/andes.md
> > > index 51f61e58e244..22aa5e5150d5 100644
> > > --- a/gcc/config/riscv/andes.md
> > > +++ b/gcc/config/riscv/andes.md
> > > @@ -441,3 +441,29 @@
> > > "nds.flmism\t%0, %z1, %z2"
> > > [(set_attr "mode" "<MODE>")
> > > (set_attr "type" "arith")])
> > > +
> > > +;;
> > > +;; ....................
> > > +;;
> > > +;; Bfloat16
> > > +;;
> > > +;; ....................
> > > +;;
> > > +
> > > +(define_insn "riscv_nds_fcvt_bf16_s"
> > > + [(set (match_operand:BF 0 "register_operand" "=f")
> > > + (float_truncate:BF
> > > + (match_operand:SF 1 "register_operand" " f")))]
> > > + "TARGET_XANDESBFHCVT"
> > > + "nds.fcvt.bf16.s\t%0,%1"
> > > + [(set_attr "type" "fcvt")
> > > + (set_attr "mode" "BF")])
> > > +
> > > +(define_insn "riscv_nds_fcvt_s_bf16"
> > > + [(set (match_operand:SF 0 "register_operand" "=f")
> > > + (float_extend:SF
> > > + (match_operand:BF 1 "register_operand" " f")))]
> > > + "TARGET_XANDESBFHCVT"
> > > + "nds.fcvt.s.bf16\t%0,%1"
> > > + [(set_attr "type" "fcvt")
> > > + (set_attr "mode" "SF")])
> > So these are equivalent to the BF16 bits in riscv.md. Given we've got a
> > number of instructions that have the same semantics I wonder if we
> > should just bite the bullet and use the assembler dialect functionality
> > to select between the standard opcodes and the Andes specific opcodes.
> >
> > My worry with the duplicated insns is long term maintenance. If we can
> > cleanly write a merged pattern, then we should try and do so.
>
> Same opinion here:
>
> maybe something like:
>
> (define_insn "truncsfbf2"
> [(set (match_operand:BF 0 "register_operand" "=f")
> (float_truncate:BF
> (match_operand:SF 1 "register_operand" " f")))]
> "TARGET_ZFBFMIN || TARGET_XANDESBFHCVT"
> {
> if (TARGET_ZFBFMIN)
> return "fcvt.bf16.s\t%0,%1";
> else /* TARGET_XANDESBFHCVT */
> return "nds.fcvt.s.bf16"
> }
> [(set_attr "type" "fcvt")
> (set_attr "mode" "BF")])
