[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 30/62] target/riscv: Update fp_status when float rounding
From: |
LIU Zhiwei |
Subject: |
Re: [PATCH v8 30/62] target/riscv: Update fp_status when float rounding mode changes |
Date: |
Fri, 5 Jun 2020 10:50:46 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 |
On 2020/6/5 4:15, Richard Henderson wrote:
On 6/2/20 10:46 PM, LIU Zhiwei wrote:
I think you are right. Maybe I should transmit frm to ctx->frm, and check
ctx->frm in vector fp ops.
We can set ctx->frm = env->frm instead of ctx->frm = -1 in
riscv_tr_init_disas_context.
And remove the sentence ctx->frm = rm; from gen_set_rm.
Is it right?
If we record frm in tb_flags, then we also have to reset
env->fp_status.rounding_mode for scalar fp insns which encode a rm != 7.
Depending on the exact mix of fp insns, that could result in more changes to
rounding_mode than we do presently. This is something that you'd want to look
at closely to make sure you're not making scalar code worse.
Hi Richard,
I don't think so. It will occupy three bits in tb_flags.
Another reason is the scalar float codes have fixed rounding mode. The
frm field in tb_flags is useless without vector extension.
If we really have to put it in the tb_flags, one bit INVALID_FRM is
enough. The scalar code will still be the same.
The vector code will check the INVALID_FRM.
I think the easiest solution in the short term is to have the translation of
any fp vector insn call gen_set_rm(ctx, 7), so that we are certain to install
frm as rounding_mode. This will happen at most once per translation block,
assuming no scalar insns that would also require changes to rounding_mode.
Yes. Only some csr insns can change the rounding mode. And they will
exit the tb immediately.
So no scalar insns will require changes within a translation block.
I think there is a error in gen_set_rm
static void gen_set_rm(DisasContext *ctx, int rm)
{
TCGv_i32 t0;
if (ctx->frm == rm) {
return;
}
ctx->frm = rm;
t0 = tcg_const_i32(rm);
gen_helper_set_rounding_mode(cpu_env, t0);
tcg_temp_free_i32(t0);
}
I don't know why updating ctx->frm in this function.
You can work with the risc-v folk to examine frm handling more generally
separate from this vector work.
OK. I will send a separate RFC patch to handle frm. Then merge it to the
vector patch set.
Best Regards,
Zhiwei
r~