Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
Hi Alistair,
On Wed, Jul 8, 2020 at 1:33 AM Alistair Francis wrote:
>
> On Thu, Jul 2, 2020 at 6:25 PM Bin Meng wrote:
> >
> > On Wed, Jul 1, 2020 at 4:23 AM 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")
> >
> > 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
Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
On Thu, Jul 2, 2020 at 6:25 PM Bin Meng wrote:
>
> On Wed, Jul 1, 2020 at 4:23 AM 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")
>
> 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
> > ---
> > target/riscv/translate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Regards,
> Bin
Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
On Wed, Jul 1, 2020 at 4:23 AM 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")
I can't find this commit id.
> Signed-off-by: Alistair Francis
> ---
> target/riscv/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Regards,
Bin
Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
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
---
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~
Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
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
> ---
> 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~
