On 04/01/2023 18:04, Richard Henderson wrote:

On 1/4/23 05:45, Mark Cave-Ayland wrote:
The FPSR quotient byte should be set to the value of the quotient and not the
result. Manually calculate the quotient in the frem helper in round to nearest
even mode (note this is different from the quotient calculated internally for
fmod), and use it to set the quotient byte accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1314
Reviewed-by: Laurent Vivier <laur...@vivier.eu>
---
  target/m68k/fpu_helper.c | 14 +++++++++++---
  1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 5fd094a33c..56f7400140 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -538,17 +538,25 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
  void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
  {
+    float_status fp_status;
+    FPReg fp_quot;
      uint32_t quotient;
      int sign;
+    /* Calculate quotient directly using round to nearest mode */
+    set_float_rounding_mode(float_round_nearest_even, &fp_status);
+    set_floatx80_rounding_precision(
+        get_floatx80_rounding_precision(&env->fp_status), &fp_status);
+    fp_quot.d = floatx80_div(val1->d, val0->d, &fp_status);
+
      res->d = floatx80_rem(val1->d, val0->d, &env->fp_status);
-    if (floatx80_is_any_nan(res->d)) {
+    if (floatx80_is_any_nan(fp_quot.d)) {

I think you should leave this line unchanged, and move the div afterward.
I also think you should completely initialize the local fp_status = { }.

With that,
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>

I can leave the floatx80_is_any_nan() line above unchanged and also initialise the local fp_status, however the floatx80_div() has to happen before floatx80_rem() function is called. This is because the fmod and frem instructions write the result back to one of the input registers, which then causes the subsequent floatx80_div() to return an incorrect result.

Would just these 2 changes be enough to keep your R-B tag for a v3?


ATB,

Mark.

Reply via email to