On 1/7/23 15:00, Mark Cave-Ayland wrote:
  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?

Mm.  I suppose so.  Otherwise, compute into a local variable:

    floatx80 fp_rem = floatx80_rem(val1->d, val0->d, &env->fp_status);
    if (!floatx80_is_any_nan(fp_rem)) {
        float_status scratch = env->fp_status;
        floatx80 fp_quot;
        uint32_t int_quot;
        int sign;

        set_float_rounding_mode(float_round_nearest_even, &scratch);
        fp_quot = floatx80_div(val1->d, val0->d, &scratch);

        sign = ...
        int_quot = ...
        ...
    }
    res->d = fp_rem;

?


r~

Reply via email to