Re: [PATCH v2 1/2] RISC-V: Support _Float16 type.
Hi Maciej:
It’s not intentionally, I suspect that is because I port from our internal
old gcc branch, will send patch to fix that later, thanks for catching this!
Maciej W. Rozycki 於 2022年12月5日 週一，21:05寫道：
> Hi Kito,
>
> I came across this issue while inspecting code and I have been wondering
> what the reason was to downgrade current FMV.X.W and FMW.W.X instructions
> to their older FMV.S.W and FMV.W.S variants here:
>
> On Wed, 10 Aug 2022, Kito Cheng wrote:
>
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 5a0adffb5ce..47e6110767c 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -2308,10 +2310,19 @@ riscv_output_move (rtx dest, rtx src)
> >if (dest_code == REG && GP_REG_P (REGNO (dest)))
> > {
> >if (src_code == REG && FP_REG_P (REGNO (src)))
> > - return dbl_p ? "fmv.x.d\t%0,%1" : "fmv.x.w\t%0,%1";
> > + switch (width)
> > + {
> > + case 2:
> > + /* Using fmv.x.s + sign-extend to emulate fmv.x.h. */
> > + return "fmv.x.s\t%0,%1;slli\t%0,%0,16;srai\t%0,%0,16";
> > + case 4:
> > + return "fmv.x.s\t%0,%1";
> > + case 8:
> > + return "fmv.x.d\t%0,%1";
> > + }
>
> and here:
>
> > @@ -2353,18 +2364,24 @@ riscv_output_move (rtx dest, rtx src)
> > return "mv\t%0,%z1";
> >
> > if (FP_REG_P (REGNO (dest)))
> > - {
> > - if (!dbl_p)
> > - return "fmv.w.x\t%0,%z1";
> > - if (TARGET_64BIT)
> > - return "fmv.d.x\t%0,%z1";
> > - /* in RV32, we can emulate fmv.d.x %0, x0 using fcvt.d.w */
> > - gcc_assert (src == CONST0_RTX (mode));
> > - return "fcvt.d.w\t%0,x0";
> > - }
> > + switch (width)
> > + {
> > + case 2:
> > + /* High 16 bits should be all-1, otherwise HW will treated
> > +as a n-bit canonical NaN, but isn't matter for
> softfloat. */
> > + return "fmv.s.x\t%0,%1";
> > + case 4:
> > + return "fmv.s.x\t%0,%z1";
> > + case 8:
> > + if (TARGET_64BIT)
> > + return "fmv.d.x\t%0,%z1";
> > + /* in RV32, we can emulate fmv.d.x %0, x0 using fcvt.d.w */
>
> (Incorrect comment formatting here as well.)
>
> > + gcc_assert (src == CONST0_RTX (mode));
> > + return "fcvt.d.w\t%0,x0";
> > + }
>
> Was it intentional or just an oversight in review? If intentional, I'd
> expect such a change to happen on its own rather than sneaked in with a
> large functional update.
>
> Maciej
>
Re: [PATCH v2 1/2] RISC-V: Support _Float16 type.
Hi Kito,
I came across this issue while inspecting code and I have been wondering
what the reason was to downgrade current FMV.X.W and FMW.W.X instructions
to their older FMV.S.W and FMV.W.S variants here:
On Wed, 10 Aug 2022, Kito Cheng wrote:
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 5a0adffb5ce..47e6110767c 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -2308,10 +2310,19 @@ riscv_output_move (rtx dest, rtx src)
>if (dest_code == REG && GP_REG_P (REGNO (dest)))
> {
>if (src_code == REG && FP_REG_P (REGNO (src)))
> - return dbl_p ? "fmv.x.d\t%0,%1" : "fmv.x.w\t%0,%1";
> + switch (width)
> + {
> + case 2:
> + /* Using fmv.x.s + sign-extend to emulate fmv.x.h. */
> + return "fmv.x.s\t%0,%1;slli\t%0,%0,16;srai\t%0,%0,16";
> + case 4:
> + return "fmv.x.s\t%0,%1";
> + case 8:
> + return "fmv.x.d\t%0,%1";
> + }
and here:
> @@ -2353,18 +2364,24 @@ riscv_output_move (rtx dest, rtx src)
> return "mv\t%0,%z1";
>
> if (FP_REG_P (REGNO (dest)))
> - {
> - if (!dbl_p)
> - return "fmv.w.x\t%0,%z1";
> - if (TARGET_64BIT)
> - return "fmv.d.x\t%0,%z1";
> - /* in RV32, we can emulate fmv.d.x %0, x0 using fcvt.d.w */
> - gcc_assert (src == CONST0_RTX (mode));
> - return "fcvt.d.w\t%0,x0";
> - }
> + switch (width)
> + {
> + case 2:
> + /* High 16 bits should be all-1, otherwise HW will treated
> +as a n-bit canonical NaN, but isn't matter for softfloat. */
> + return "fmv.s.x\t%0,%1";
> + case 4:
> + return "fmv.s.x\t%0,%z1";
> + case 8:
> + if (TARGET_64BIT)
> + return "fmv.d.x\t%0,%z1";
> + /* in RV32, we can emulate fmv.d.x %0, x0 using fcvt.d.w */
(Incorrect comment formatting here as well.)
> + gcc_assert (src == CONST0_RTX (mode));
> + return "fcvt.d.w\t%0,x0";
> + }
Was it intentional or just an oversight in review? If intentional, I'd
expect such a change to happen on its own rather than sneaked in with a
large functional update.
Maciej
[PATCH v2 1/2] RISC-V: Support _Float16 type.
RISC-V decide use _Float16 as primary IEEE half precision type, and this
already become part of psABI, this patch has added folloing support for
_Float16:
- Soft-float support for _Float16.
- Make sure _Float16 available on C++ mode.
- Name mangling for _Float16 on C++ mode.
gcc/ChangeLog
* config/riscv/riscv-builtins.cc: include stringpool.h
(riscv_float16_type_node): New.
(riscv_init_builtin_types): Ditto.
(riscv_init_builtins): Call riscv_init_builtin_types.
* config/riscv/riscv-modes.def (HF): New.
* gcc/config/riscv/riscv.cc (riscv_output_move): Handle HFmode.
(riscv_mangle_type): New.
(riscv_scalar_mode_supported_p): Ditto.
(riscv_libgcc_floating_mode_supported_p): Ditto.
(riscv_excess_precision): Ditto.
(riscv_floatn_mode): Ditto.
(riscv_init_libfuncs): Ditto.
(TARGET_MANGLE_TYPE): Ditto.
(TARGET_SCALAR_MODE_SUPPORTED_P): Ditto.
(TARGET_LIBGCC_FLOATING_MODE_SUPPORTED_P): Ditto.
(TARGET_INIT_LIBFUNCS): Ditto.
(TARGET_C_EXCESS_PRECISION): Ditto.
(TARGET_FLOATN_MODE): Ditto.
* gcc/config/riscv/riscv.md (mode): Add HF.
(softload): Add HF.
(softstore): Ditto.
(fmt): Ditto.
(UNITMODE): Ditto.
(movhf): New.
(*movhf_softfloat): New.
libgcc/ChangeLog:
* config/riscv/sfp-machine.h (_FP_NANFRAC_H): New.
(_FP_NANFRAC_H): Ditto.
(_FP_NANSIGN_H): Ditto.
* config/riscv/t-softfp32 (softfp_extensions): Add HF related
routines.
(softfp_truncations): Ditto.
(softfp_extras): Ditto.
* config/riscv/t-softfp64 (softfp_extras): Add HF related routines.
gcc/testsuite/ChangeLog:
* gcc/testsuite/g++.target/riscv/_Float16.C: New.
* gcc/testsuite/gcc.target/riscv/_Float16-soft-1.c: Ditto.
* gcc/testsuite/gcc.target/riscv/_Float16-soft-2.c: Ditto.
* gcc/testsuite/gcc.target/riscv/_Float16-soft-3.c: Ditto.
* gcc/testsuite/gcc.target/riscv/_Float16-soft-4.c: Ditto.
* gcc/testsuite/gcc.target/riscv/_Float16.c: Ditto.
---
gcc/config/riscv/riscv-builtins.cc| 24 +++
gcc/config/riscv/riscv-modes.def | 1 +
gcc/config/riscv/riscv.cc | 171 --
gcc/config/riscv/riscv.md | 30 ++-
gcc/testsuite/g++.target/riscv/_Float16.C | 18 ++
.../gcc.target/riscv/_Float16-soft-1.c| 9 +
.../gcc.target/riscv/_Float16-soft-2.c| 13 ++
.../gcc.target/riscv/_Float16-soft-3.c| 12 ++
.../gcc.target/riscv/_Float16-soft-4.c| 12 ++
gcc/testsuite/gcc.target/riscv/_Float16.c | 19 ++
libgcc/config/riscv/sfp-machine.h | 3 +
libgcc/config/riscv/t-softfp32| 5 +
libgcc/config/riscv/t-softfp64| 1 +
13 files changed, 300 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/g++.target/riscv/_Float16.C
create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-soft-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-soft-2.c
create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-soft-3.c
create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16-soft-4.c
create mode 100644 gcc/testsuite/gcc.target/riscv/_Float16.c
diff --git a/gcc/config/riscv/riscv-builtins.cc
b/gcc/config/riscv/riscv-builtins.cc
index 1218fdfc67d..3009311604d 100644
--- a/gcc/config/riscv/riscv-builtins.cc
+++ b/gcc/config/riscv/riscv-builtins.cc
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "recog.h"
#include "diagnostic-core.h"
#include "stor-layout.h"
+#include "stringpool.h"
#include "expr.h"
#include "langhooks.h"
@@ -160,6 +161,8 @@ static GTY(()) int riscv_builtin_decl_index[NUM_INSN_CODES];
#define GET_BUILTIN_DECL(CODE) \
riscv_builtin_decls[riscv_builtin_decl_index[(CODE)]]
+tree riscv_float16_type_node = NULL_TREE;
+
/* Return the function type associated with function prototype TYPE. */
static tree
@@ -185,11 +188,32 @@ riscv_build_function_type (enum riscv_function_type type)
return types[(int) type];
}
+static void
+riscv_init_builtin_types (void)
+{
+ /* Provide the _Float16 type and float16_type_node if needed. */
+ if (!float16_type_node)
+{
+ riscv_float16_type_node = make_node (REAL_TYPE);
+ TYPE_PRECISION (riscv_float16_type_node) = 16;
+ SET_TYPE_MODE (riscv_float16_type_node, HFmode);
+ layout_type (riscv_float16_type_node);
+}
+ else
+riscv_float16_type_node = float16_type_node;
+
+ if (!maybe_get_identifier ("_Float16"))
+lang_hooks.types.register_builtin_type (riscv_float16_type_node,
+ "_Float16");
+}
+
/* Implement TARGET_INIT_BUILTINS. */
void
riscv_init_builtins (void)
{
+ riscv_init_builtin_types ();
+
for (size_t i = 0; i < ARRAY_SIZE (riscv_builtins); i++)
{
const struct
