[PATCH] riscv: Add helper to make NaN-boxing for FP register

Ian Jiang posted 1 patch 4 years, 3 months ago
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200127141051.12543-1-ianjiang.ict@gmail.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Alistair Francis <Alistair.Francis@wdc.com>
There is a newer version of this series
target/riscv/insn_trans/trans_rvf.inc.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
[PATCH] riscv: Add helper to make NaN-boxing for FP register
Posted by Ian Jiang 4 years, 3 months ago
The function that makes NaN-boxing when a 32-bit value is assigned
to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
Then it is applied in translating of the FLW instruction.

This also applies for other instructions when the RVD extension is
present, such as FMV.W.W, FADD.S, FSUB.S and so on.

Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>
---
 target/riscv/insn_trans/trans_rvf.inc.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvf.inc.c b/target/riscv/insn_trans/trans_rvf.inc.c
index e23cd639a6..3bfd8881e7 100644
--- a/target/riscv/insn_trans/trans_rvf.inc.c
+++ b/target/riscv/insn_trans/trans_rvf.inc.c
@@ -23,6 +23,20 @@
         return false;                       \
 } while (0)
 
+/*
+ * RISC-V requires NaN-boxing of narrower width floating
+ * point values.  This applies when a 32-bit value is
+ * assigned to a 64-bit FP register.  Thus this does not
+ * apply when the RVD extension is not present.
+ */
+static void gen_nanbox_fpr(DisasContext *ctx, int regno)
+{
+    if (has_ext(ctx, RVD)) {
+        tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno],
+                        MAKE_64BIT_MASK(32, 32));
+    }
+}
+
 static bool trans_flw(DisasContext *ctx, arg_flw *a)
 {
     TCGv t0 = tcg_temp_new();
@@ -32,8 +46,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
     tcg_gen_addi_tl(t0, t0, a->imm);
 
     tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
-    /* RISC-V requires NaN-boxing of narrower width floating point values */
-    tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], 0xffffffff00000000ULL);
+    gen_nanbox_fpr(ctx, a->rd);
 
     tcg_temp_free(t0);
     mark_fs_dirty(ctx);
-- 
2.17.1


Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register
Posted by Alistair Francis 4 years, 3 months ago
On Mon, Jan 27, 2020 at 6:11 AM Ian Jiang <ianjiang.ict@gmail.com> wrote:
>
> The function that makes NaN-boxing when a 32-bit value is assigned
> to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> Then it is applied in translating of the FLW instruction.
>
> This also applies for other instructions when the RVD extension is
> present, such as FMV.W.W, FADD.S, FSUB.S and so on.
>
> Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvf.inc.c b/target/riscv/insn_trans/trans_rvf.inc.c
> index e23cd639a6..3bfd8881e7 100644
> --- a/target/riscv/insn_trans/trans_rvf.inc.c
> +++ b/target/riscv/insn_trans/trans_rvf.inc.c
> @@ -23,6 +23,20 @@
>          return false;                       \
>  } while (0)
>
> +/*
> + * RISC-V requires NaN-boxing of narrower width floating
> + * point values.  This applies when a 32-bit value is
> + * assigned to a 64-bit FP register.  Thus this does not
> + * apply when the RVD extension is not present.
> + */
> +static void gen_nanbox_fpr(DisasContext *ctx, int regno)
> +{
> +    if (has_ext(ctx, RVD)) {
> +        tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno],
> +                        MAKE_64BIT_MASK(32, 32));
> +    }
> +}
> +
>  static bool trans_flw(DisasContext *ctx, arg_flw *a)
>  {
>      TCGv t0 = tcg_temp_new();
> @@ -32,8 +46,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
>      tcg_gen_addi_tl(t0, t0, a->imm);
>
>      tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
> -    /* RISC-V requires NaN-boxing of narrower width floating point values */
> -    tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], 0xffffffff00000000ULL);
> +    gen_nanbox_fpr(ctx, a->rd);
>
>      tcg_temp_free(t0);
>      mark_fs_dirty(ctx);
> --
> 2.17.1
>
>

Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register
Posted by Richard Henderson 4 years, 3 months ago
On 1/27/20 6:10 AM, Ian Jiang wrote:
> The function that makes NaN-boxing when a 32-bit value is assigned
> to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> Then it is applied in translating of the FLW instruction.
> 
> This also applies for other instructions when the RVD extension is
> present, such as FMV.W.W, FADD.S, FSUB.S and so on.

I wouldn't mention this yet, as it begs the question of what you are doing
about it.  Which is nothing, yet, since that will apply to a follow-up patch.


> 
> Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>
> ---
>  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register
Posted by Ian Jiang 4 years, 3 months ago
The patch description is modified and a new version is committed.
--
Ian Jiang

Richard Henderson <richard.henderson@linaro.org> 于2020年1月28日周二 上午1:38写道:
>
> On 1/27/20 6:10 AM, Ian Jiang wrote:
> > The function that makes NaN-boxing when a 32-bit value is assigned
> > to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> > Then it is applied in translating of the FLW instruction.
> >
> > This also applies for other instructions when the RVD extension is
> > present, such as FMV.W.W, FADD.S, FSUB.S and so on.
>
> I wouldn't mention this yet, as it begs the question of what you are doing
> about it.  Which is nothing, yet, since that will apply to a follow-up patch.
>
>
> >
> > Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>
> > ---
> >  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~