| Message ID | ea4f280e6f77e734c8e555e3c98d10085ce9f5b6.1593547870.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | A few RISC-V fixes | expand |
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. r~
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~
On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis <alistair.francis@wdc.com> 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") I can't find this commit id. > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Regards, Bin
On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis > <alistair.francis@wdc.com> 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") > > I can't find this commit id. It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350 Alistair > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > target/riscv/translate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Regards, > Bin
Hi Alistair, On Wed, Jul 8, 2020 at 1:33 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis > > <alistair.francis@wdc.com> 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") > > > > I can't find this commit id. > > It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350 > My understanding is that the convention used in "Fixes" tag is for commit id. 1885350 is not a commit id but a QEMU bug report id. Regards, Bin
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; } ctx->frm = rm;
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(-)