| Message ID | 20191009211541.9937-1-palmer@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: fcvt can set fflags, so set FS accordingly | expand |
On 10/9/19 5:15 PM, Palmer Dabbelt wrote: > 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(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
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; }
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(+)