Re: [PATCH v8 30/62] target/riscv: Update fp_status when float rounding mode changes

[LIU Zhiwei]

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~