Re: [Qemu-devel] [RFC] alpha qemu arithmetic exceptions

[Al Viro]

On Wed, Jun 25, 2014 at 08:01:17AM +0100, Al Viro wrote:
> On Tue, Jun 24, 2014 at 02:32:46PM -0700, Richard Henderson wrote:
> > On 06/24/2014 02:24 PM, Al Viro wrote:
> > > Al, off to figure out the black magic TCG is using to generate calls...
> > 
> > If you've a helper
> > 
> > DEF_HELPER_1(halt, void, i64)
> > 
> > then
> > 
> >   gen_helper_halt(...)
> > 
> > will generate the tcg ops that result in the call.
> 
> Another fun issue:
> 
> CVTTQ is both underreporting the overflow *AND* reports the wrong kind - FOV
> instead of IOV.
> 
>       * it misses reporting overflows for case when it *knows* that
>         overflow will happen - the need to shift up by more than 63 bits.
>         Trivially fixed, of course.  There overflow cases leave wrong
>         result as well - should be 0.
>       * it also misses reporting overflows for case when value is in
>         ranges 2^63..2^64-1 and -2^64+1..-2^63-1.  And yes, it's
>         asymmetric - 2^63 is an overflow, -2^63 isn't.
>       * overflow is reported by float_raise(float_flag_overflow, &FP_STATUS).
>         Wrong flag - it should be IOV, not FOV.  And it should be set
>         in FPCR regardless of the trap modifier (IOV, this VI thing is
>         wrong - we should deal with that only when we generate a trap).
>       * All overflow cases should raise INE as well.
> 
> Could we steal bit 1 in float_exception_flags for IOV?  It is (currently?)
> unused -
> enum {
>     float_flag_invalid   =  1,
>     float_flag_divbyzero =  4,
>     float_flag_overflow  =  8,
>     float_flag_underflow = 16,
>     float_flag_inexact   = 32,
>     float_flag_input_denormal = 64,
>     float_flag_output_denormal = 128
> };
> 
> That would allow to deal with that crap nicely - we could have it raise
> the new flag, then have helper_fp_exc_raise_... for default trap mode
> mask it out (and yes, we need to set FPCR flags in default mode, as well
> as /U and /V - confirmed by direct experiment *and* by TFM).

OK, I've managed to resurrect UP1000 box (FSVO resurrect - the southbridge
DMA controller has always been buggered, with intermittent noise on one of
the data lines; fans in CPU module are FUBAR as well - 17 and 20 RPM resp.,
so I don't risk keeping it running for long, etc.)

Still, that allows to test EV67 and I hope to resurrect a DS10 box as well,
which will allow for saner testing environment.

Current delta follows, fixing gcc and libc testcases *and* AFAICS getting
CVTTQ handling in line with what actual EV67 is doing.  It's a dirty hack
wrt float_raise() - relies on bit 1 never being raised by softfpu.c.  I'll
look into separating that bit, but it'll probably have non-zero costs ;-/
We need two flags - "has IOV been raised during this insn" (in this patch
it's bit 1 of fp_status.float_exception_flags, cleaned along with those)
and something to keep FPCR.IOV in (in this patch - bit 1 of fpcr_exc_status).
Sure, we can add another uint8_t or two in struct CPUAlphaState, but that'll
mean extra PITA in code and extra memory accesses...

Review would be welcome.

diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index d9b861f..047b9a2 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -152,6 +152,10 @@ enum {
     FP_ROUND_DYNAMIC = 0x3,
 };
 
+enum {
+    float_flag_IOV = 2,
+};
+
 /* FPCR bits */
 #define FPCR_SUM               (1ULL << 63)
 #define FPCR_INED              (1ULL << 62)
diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c
index d2d776c..2b39ea4 100644
--- a/target-alpha/fpu_helper.c
+++ b/target-alpha/fpu_helper.c
@@ -45,10 +45,11 @@ uint32_t helper_fp_exc_get(CPUAlphaState *env)
 }
 
 static inline void inline_fp_exc_raise(CPUAlphaState *env, uintptr_t retaddr,
-                                       uint32_t exc, uint32_t regno)
+                                       uint32_t exc, uint32_t regno, uint32_t 
hw_exc)
 {
     if (exc) {
-        uint32_t hw_exc = 0;
+       if (hw_exc)
+            exc &= ~env->fpcr_exc_mask;
 
         if (exc & float_flag_invalid) {
             hw_exc |= EXC_M_INV;
@@ -65,6 +66,9 @@ static inline void inline_fp_exc_raise(CPUAlphaState *env, 
uintptr_t retaddr,
         if (exc & float_flag_inexact) {
             hw_exc |= EXC_M_INE;
         }
+        if (exc & float_flag_IOV) {
+            hw_exc |= EXC_M_IOV;
+        }
 
         arith_excp(env, retaddr, hw_exc, 1ull << regno);
     }
@@ -73,18 +77,21 @@ static inline void inline_fp_exc_raise(CPUAlphaState *env, 
uintptr_t retaddr,
 /* Raise exceptions for ieee fp insns without software completion.
    In that case there are no exceptions that don't trap; the mask
    doesn't apply.  */
-void helper_fp_exc_raise(CPUAlphaState *env, uint32_t exc, uint32_t regno)
+void helper_fp_exc_raise(CPUAlphaState *env, uint32_t ignore, uint32_t regno)
 {
-    inline_fp_exc_raise(env, GETPC(), exc, regno);
+    uint32_t exc = (uint8_t)env->fp_status.float_exception_flags;
+    if (exc) {
+       env->fpcr_exc_status |= exc;
+       inline_fp_exc_raise(env, GETPC(), exc & ~ignore, regno, 0);
+    }
 }
 
-/* Raise exceptions for ieee fp insns with software completion.  */
-void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t exc, uint32_t regno)
+void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t ignore, uint32_t regno)
 {
+    uint32_t exc = (uint8_t)env->fp_status.float_exception_flags;
     if (exc) {
-        env->fpcr_exc_status |= exc;
-        exc &= ~env->fpcr_exc_mask;
-        inline_fp_exc_raise(env, GETPC(), exc, regno);
+       env->fpcr_exc_status |= exc;
+        inline_fp_exc_raise(env, GETPC(), exc & ~ignore, regno, EXC_M_SWC);
     }
 }
 
@@ -682,14 +689,12 @@ uint64_t helper_cvtqs(CPUAlphaState *env, uint64_t a)
     return float32_to_s(fr);
 }
 
-/* Implement float64 to uint64 conversion without saturation -- we must
+/* Implement float64 to int64 conversion without saturation -- we must
    supply the truncated result.  This behaviour is used by the compiler
-   to get unsigned conversion for free with the same instruction.
-
-   The VI flag is set when overflow or inexact exceptions should be raised.  */
+   to get unsigned conversion for free with the same instruction. */
 
 static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a,
-                                    int roundmode, int VI)
+                                    int roundmode)
 {
     uint64_t frac, ret = 0;
     uint32_t exp, sign, exc = 0;
@@ -704,7 +709,7 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, 
uint64_t a,
             goto do_underflow;
         }
     } else if (exp == 0x7ff) {
-        exc = (frac ? float_flag_invalid : VI ? float_flag_overflow : 0);
+        exc = frac ? float_flag_invalid : float_flag_IOV | float_flag_inexact;
     } else {
         /* Restore implicit bit.  */
         frac |= 0x10000000000000ull;
@@ -715,10 +720,12 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, 
uint64_t a,
                the fraction left.  There is no rounding to do.  */
             if (shift < 63) {
                 ret = frac << shift;
-                if (VI && (ret >> shift) != frac) {
-                    exc = float_flag_overflow;
+                if ((ret >> shift) != frac || (int64_t)(ret-sign) < 0) {
+                    exc = float_flag_IOV | float_flag_inexact;
                 }
-            }
+            } else {
+               exc = float_flag_IOV | float_flag_inexact;
+           }
         } else {
             uint64_t round;
 
@@ -739,7 +746,7 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, 
uint64_t a,
             }
 
             if (round) {
-                exc = (VI ? float_flag_inexact : 0);
+                exc = float_flag_inexact;
                 switch (roundmode) {
                 case float_round_nearest_even:
                     if (round == (1ull << 63)) {
@@ -773,17 +780,12 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, 
uint64_t a,
 
 uint64_t helper_cvttq(CPUAlphaState *env, uint64_t a)
 {
-    return inline_cvttq(env, a, FP_STATUS.float_rounding_mode, 1);
+    return inline_cvttq(env, a, FP_STATUS.float_rounding_mode);
 }
 
 uint64_t helper_cvttq_c(CPUAlphaState *env, uint64_t a)
 {
-    return inline_cvttq(env, a, float_round_to_zero, 0);
-}
-
-uint64_t helper_cvttq_svic(CPUAlphaState *env, uint64_t a)
-{
-    return inline_cvttq(env, a, float_round_to_zero, 1);
+    return inline_cvttq(env, a, float_round_to_zero);
 }
 
 uint64_t helper_cvtqt(CPUAlphaState *env, uint64_t a)
diff --git a/target-alpha/helper.c b/target-alpha/helper.c
index 7c053a3..82b1040 100644
--- a/target-alpha/helper.c
+++ b/target-alpha/helper.c
@@ -36,6 +36,9 @@ uint64_t cpu_alpha_load_fpcr (CPUAlphaState *env)
         if (t & float_flag_invalid) {
             r |= FPCR_INV;
         }
+        if (t & float_flag_IOV) {
+            r |= FPCR_IOV;
+       }
         if (t & float_flag_divbyzero) {
             r |= FPCR_DZE;
         }
@@ -103,6 +106,9 @@ void cpu_alpha_store_fpcr (CPUAlphaState *env, uint64_t val)
     if (val & FPCR_INV) {
         t |= float_flag_invalid;
     }
+    if (val & FPCR_IOV) {
+        t |= float_flag_IOV;
+    }
     if (val & FPCR_DZE) {
         t |= float_flag_divbyzero;
     }
@@ -527,6 +533,7 @@ void QEMU_NORETURN dynamic_excp(CPUAlphaState *env, 
uintptr_t retaddr,
     env->error_code = error;
     if (retaddr) {
         cpu_restore_state(cs, retaddr);
+       env->pc += 4;
     }
     cpu_loop_exit(cs);
 }
diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index a451cfe..173db01 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -83,7 +83,6 @@ DEF_HELPER_FLAGS_2(cvtqg, TCG_CALL_NO_RWG, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(cvttq, TCG_CALL_NO_RWG, i64, env, i64)
 DEF_HELPER_FLAGS_2(cvttq_c, TCG_CALL_NO_RWG, i64, env, i64)
-DEF_HELPER_FLAGS_2(cvttq_svic, TCG_CALL_NO_RWG, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(setroundmode, TCG_CALL_NO_RWG, void, env, i32)
 DEF_HELPER_FLAGS_2(setflushzero, TCG_CALL_NO_RWG, void, env, i32)
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index cc81e77..e7b0f0d 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -672,26 +672,22 @@ static void gen_fp_exc_clear(void)
 #endif
 }
 
-static void gen_fp_exc_raise_ignore(int rc, int fn11, int ignore)
+static void gen_fp_exc_raise(int rc, int fn11)
 {
     /* ??? We ought to be able to do something with imprecise exceptions.
        E.g. notice we're still in the trap shadow of something within the
        TB and do not generate the code to signal the exception; end the TB
        when an exception is forced to arrive, either by consumption of a
        register value or TRAPB or EXCB.  */
-    TCGv_i32 exc = tcg_temp_new_i32();
     TCGv_i32 reg;
+    TCGv_i32 ign;
+    uint32_t ignore = float_flag_inexact;
+    if (!(fn11 & QUAL_U))
+       ignore |= float_flag_underflow | float_flag_IOV;
+    if (fn11 & QUAL_I)
+       ignore &= ~float_flag_inexact;
 
-#if defined(CONFIG_SOFTFLOAT_INLINE)
-    tcg_gen_ld8u_i32(exc, cpu_env,
-                     offsetof(CPUAlphaState, fp_status.float_exception_flags));
-#else
-    gen_helper_fp_exc_get(exc, cpu_env);
-#endif
-
-    if (ignore) {
-        tcg_gen_andi_i32(exc, exc, ~ignore);
-    }
+    ign = tcg_const_i32(ignore);
 
     /* ??? Pass in the regno of the destination so that the helper can
        set EXC_MASK, which contains a bitmask of destination registers
@@ -701,18 +697,13 @@ static void gen_fp_exc_raise_ignore(int rc, int fn11, int 
ignore)
     reg = tcg_const_i32(rc + 32);
 
     if (fn11 & QUAL_S) {
-        gen_helper_fp_exc_raise_s(cpu_env, exc, reg);
+        gen_helper_fp_exc_raise_s(cpu_env, ign, reg);
     } else {
-        gen_helper_fp_exc_raise(cpu_env, exc, reg);
+        gen_helper_fp_exc_raise(cpu_env, ign, reg);
     }
 
+    tcg_temp_free_i32(ign);
     tcg_temp_free_i32(reg);
-    tcg_temp_free_i32(exc);
-}
-
-static inline void gen_fp_exc_raise(int rc, int fn11)
-{
-    gen_fp_exc_raise_ignore(rc, fn11, fn11 & QUAL_I ? 0 : float_flag_inexact);
 }
 
 static void gen_fcvtlq(TCGv vc, TCGv vb)
@@ -773,7 +764,6 @@ IEEE_ARITH2(cvtts)
 static void gen_fcvttq(DisasContext *ctx, int rb, int rc, int fn11)
 {
     TCGv vb, vc;
-    int ignore = 0;
 
     /* No need to set flushzero, since we have an integer output.  */
     gen_fp_exc_clear();
@@ -782,26 +772,13 @@ static void gen_fcvttq(DisasContext *ctx, int rb, int rc, 
int fn11)
 
     /* Almost all integer conversions use cropped rounding, and most
        also do not have integer overflow enabled.  Special case that.  */
-    switch (fn11) {
-    case QUAL_RM_C:
+    if ((fn11 & QUAL_RM_MASK) == QUAL_RM_C) {
         gen_helper_cvttq_c(vc, cpu_env, vb);
-        break;
-    case QUAL_V | QUAL_RM_C:
-    case QUAL_S | QUAL_V | QUAL_RM_C:
-        ignore = float_flag_inexact;
-        /* FALLTHRU */
-    case QUAL_S | QUAL_V | QUAL_I | QUAL_RM_C:
-        gen_helper_cvttq_svic(vc, cpu_env, vb);
-        break;
-    default:
+    } else {
         gen_qual_roundmode(ctx, fn11);
         gen_helper_cvttq(vc, cpu_env, vb);
-        ignore |= (fn11 & QUAL_V ? 0 : float_flag_overflow);
-        ignore |= (fn11 & QUAL_I ? 0 : float_flag_inexact);
-        break;
     }
-
-    gen_fp_exc_raise_ignore(rc, fn11, ignore);
+    gen_fp_exc_raise(rc, fn11);
 }
 
 static void gen_ieee_intcvt(DisasContext *ctx,