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.
r~
