[PATCH] RISC-V: fcvt can set fflags, so set FS accordingly

* [PATCH] RISC-V: fcvt can set fflags, so set FS accordingly
@ 2019-10-09 21:15 Palmer Dabbelt
  2019-10-09 22:26 ` Richard Henderson
  0 siblings, 1 reply; 2+ messages in thread
From: Palmer Dabbelt @ 2019-10-09 21:15 UTC (permalink / raw)
  To: qemu-riscv; +Cc: Palmer Dabbelt, qemu-devel

A user pinged me to say "my floating point heavy code works in user mode
but not system mode", which I'm guessing is the result of a lazy FP
save/restore issue as those still crop up from time to time as long tail
bugs.  I figured it was worth giving the FP stuff a look to see if
anything jumps out, and it turns out that there is a bug: converting
float to integer can set the invalid flag, which is supposed to mark FS
as dirty, but the emulation routine doesn't do so.

This patch unconditionally marks FS as dirty for fcvt instructions that
convert into X registers (fcvt into F registers already did so).  I
haven't actually tried to manifest a bug here, but as far as I can tell
the soft float stuff does set the invalid flag.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/insn_trans/trans_rvd.inc.c | 2 ++
 target/riscv/insn_trans/trans_rvf.inc.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/target/riscv/insn_trans/trans_rvd.inc.c b/target/riscv/insn_trans/trans_rvd.inc.c
index 393fa0248c..8611e95486 100644
--- a/target/riscv/insn_trans/trans_rvd.inc.c
+++ b/target/riscv/insn_trans/trans_rvd.inc.c
@@ -371,6 +371,7 @@ static bool trans_fcvt_l_d(DisasContext *ctx, arg_fcvt_l_d *a)
     gen_helper_fcvt_l_d(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
     return true;
 }
 
@@ -384,6 +385,7 @@ static bool trans_fcvt_lu_d(DisasContext *ctx, arg_fcvt_lu_d *a)
     gen_helper_fcvt_lu_d(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
     return true;
 }
 
diff --git a/target/riscv/insn_trans/trans_rvf.inc.c b/target/riscv/insn_trans/trans_rvf.inc.c
index 172dbfa919..87a250a3f2 100644
--- a/target/riscv/insn_trans/trans_rvf.inc.c
+++ b/target/riscv/insn_trans/trans_rvf.inc.c
@@ -237,6 +237,7 @@ static bool trans_fcvt_w_s(DisasContext *ctx, arg_fcvt_w_s *a)
     gen_helper_fcvt_w_s(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
 
     return true;
 }
@@ -251,6 +252,7 @@ static bool trans_fcvt_wu_s(DisasContext *ctx, arg_fcvt_wu_s *a)
     gen_helper_fcvt_wu_s(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
 
     return true;
 }
@@ -389,6 +391,7 @@ static bool trans_fcvt_l_s(DisasContext *ctx, arg_fcvt_l_s *a)
     gen_helper_fcvt_l_s(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
     return true;
 }
 
@@ -402,6 +405,7 @@ static bool trans_fcvt_lu_s(DisasContext *ctx, arg_fcvt_lu_s *a)
     gen_helper_fcvt_lu_s(t0, cpu_env, cpu_fpr[a->rs1]);
     gen_set_gpr(a->rd, t0);
     tcg_temp_free(t0);
+    mark_fs_dirty(ctx);
     return true;
 }
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 2+ messages in thread