Richard Henderson <richard.hender...@linaro.org> 于2020年1月23日周四 上午11:07写道：
>
> On 1/22/20 2:48 PM, Ian Jiang wrote:
> > Richard Henderson <richard.hender...@linaro.org> 于2020年1月23日周四 上午1:11写道：
> >>
> >> On 1/21/20 11:53 PM, Ian Jiang wrote:
> >>> --
> >>> Ian Jiang
> >>>
> >>> Richard Henderson <richard.hender...@linaro.org> 于2020年1月22日周三 下午4:53写道：
> >>>>
> >>>> On 1/21/20 9:24 PM, Ian Jiang wrote:
> >>>>> For FMV.W.X that moves the lower 32 bits of an integer register to a
> >>>>> floating-point register, Rd should encoded with NoN-boxing scheme.
> >>>>> Note: This applies to RV64 only.
> >>>>>
> >>>>> Signed-off-by: Ian Jiang <ianjiang....@gmail.com>
> >>>>> ---
> >>>>> target/riscv/insn_trans/trans_rvf.inc.c | 1 +
> >>>>> 1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/target/riscv/insn_trans/trans_rvf.inc.c
> >>>>> b/target/riscv/insn_trans/trans_rvf.inc.c
> >>>>> index 172dbfa919..62b7a36567 100644
> >>>>> --- a/target/riscv/insn_trans/trans_rvf.inc.c
> >>>>> +++ b/target/riscv/insn_trans/trans_rvf.inc.c
> >>>>> @@ -368,6 +368,7 @@ static bool trans_fmv_w_x(DisasContext *ctx,
> >>>>> arg_fmv_w_x *a)
> >>>>>
> >>>>> #if defined(TARGET_RISCV64)
> >>>>> tcg_gen_mov_i64(cpu_fpr[a->rd], t0);
> >>>>> + tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> >>>>> 0xffffffff00000000ULL);
> >>>>> #else
> >>>>> tcg_gen_extu_i32_i64(cpu_fpr[a->rd], t0);
> >>>>> #endif
> >>>>>
> >>>>
> >>>> This doesn't look right. There's nothing in the spec that says the
> >>>> nan-boxing
> >>>> is restricted to rv64. NaN-boxing is all about FLEN, not XLEN.
> >>>>
> >>>
> >>> Why the translation of FLW has a NaN-boxing?
> >>>
> >>> file ./target/riscv/insn_trans/trans_rvf.inc.c
> >>> 26 static bool trans_flw(DisasContext *ctx, arg_flw *a)
> >>> 27 {
> >>> ...
> >>> 34 tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
> >>> 35 /* RISC-V requires NaN-boxing of narrower width floating point
> >>> values */
> >>> 36 tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> >>> 0xffffffff00000000ULL);
> >>> ...
> >>> 41 }
> >>
> >> Eh? Obviously because we're loading a 32-bit value into a FLEN=64 bit
> >> register.
> > FMV.W.X is the same with FLW at this point that filling a 64 bits
> > float register based on 32 bits value.
> > Besides, the RISCV simulator Spike makes NaN-boxing for FLW, FMV.W.W,
> > FADD.S, FSUB.S, and others.
> > It might be better that they have a coincident behavior? I am not sure
> > about this and just want a discussion.
>
> So, I wonder if we're talking past one another.
>
> In reply to the patch at the beginning of this thread, I was suggesting that
> the correct solution is
>
> #if defined(TARGET_RISCV64)
> tcg_gen_mov_i64(cpu_fpr[a->rd], t0);
> #else
> tcg_gen_extu_i32_i64(cpu_fpr[a->rd], t0);
> #endif
> + tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> + 0xffffffff00000000ULL);
>
> However, after the previous follow-up I wonder if it might be better to split
> out the nan-boxing to a helper:
>
> /*
> * 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));
> }
> }
>
> which could elide this when only RVF is enabled, and thus
> the upper 32-bits of the register are inaccessible.
>
> This helper would then be propagated to the existing uses within translate.c
> and insn_trans/trans_rvf.inc.c.
>
> As another cleanup, the ifdef above may be replaced with
>
> tcg_gen_extu_tl_i64(cpu_fpr[a->rd], t0);
>
> which will handle both RISCV64 and RISCV32.
Good point.
But I am not clear where to call this new helper gen_nanbox_fpr(). Is
there a position that could affect all floating-point instructions? So
that we don't have to modify so many translating functions. Please
give more details.
