Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode

[LIU Zhiwei]

On 2020/7/1 5:51, Richard Henderson wrote:
> On 6/30/20 1:12 PM, Alistair Francis wrote:
> > When a guest specificies the the rounding mode should be dynamic 0b111
> > then we want to re-caclulate the rounding mode on each instruction. The
> > gen_helper_set_rounding_mode() function will correctly check the
> > rounding mode and handle a dynamic rounding, we just need to make sure
> > it's always called if dynamic rounding is selected.
> >
> > Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   target/riscv/translate.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index ce71ca7a92..a39eba679a 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
> >   {
> >       TCGv_i32 t0;
> >   
> > -    if (ctx->frm == rm) {
> > +    if (ctx->frm == rm && rm != 7) {
> >           return;
> This should not be necessary.
>
> It was my understanding that after the set to the csr, that we would end the
> TB.  That's certainly what I see in RISCV_OP_CSR_POST.
>
> The next TB will begin wiht ctx->frm = -1, so we will reset the rounding mode
> with 7.  It would be good to understand what's really going on here.
Agree. I think the 'bug '  is false positive.
Although the round mode process code is  confusing, it it right.

Zhiwei
> r~