Re: [Qemu-devel] target-alpha: An approach to fp insn qualifiers

[Richard Henderson <rth@twiddle.net>]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

On 12/14/2009 04:31 PM, Richard Henderson wrote:
> On 12/14/2009 12:11 PM, Laurent Desnogues wrote:
>> I'll take a closer look at your patch tomorrow.
>
> For the record, I believe this finishes what I had in mind for the
> exception handling there in op_handler.c.

Hmph.  One more patch for correctness.  With this 183.equake runs 
correctly.  I couldn't remember all the hoops to get runspec.pl to work, 
to do the whole testsuite, but I did run this one by hand.

./quake-amd64: Done. Terminating the simulation.

real	0m34.943s
user	0m34.913s
sys	0m0.024s

./quake-axp: Done. Terminating the simulation.

real	33m24.105s
user	33m23.674s
sys	0m0.116s

with identical output.


r~

[-- Attachment #2: commit-fpu-4 --]
[-- Type: text/plain, Size: 7279 bytes --]

commit daf11ad5cd50c56d44e36e4ea334c660f8fe4c16
Author: Richard Henderson <rth@twiddle.net>
Date:   Mon Dec 14 19:46:57 2009 -0800

    target-alpha: Don't ever saturate cvttq.
    
    The previous patch tried allowing saturation if /S;
    that doesn't match what the kernels generate.

diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index d031f56..2d1c3d5 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -1220,120 +1220,106 @@ uint64_t helper_cvtqs (uint64_t a, uint32_t quals)
    is used by the compiler to get unsigned conversion for free with
    the same instruction.  */
 
-static uint64_t cvttq_noqual_internal(uint64_t a, uint32_t rounding_mode)
+static uint64_t cvttq_internal(uint64_t a)
 {
     uint64_t frac, ret = 0;
-    uint32_t exp, sign;
+    uint32_t exp, sign, exc = 0;
     int shift;
 
     sign = (a >> 63);
     exp = (uint32_t)(a >> 52) & 0x7ff;
     frac = a & 0xfffffffffffffull;
 
-    /* We already handled denormals in remap_ieee_input; infinities and
-       nans are defined to return zero as per truncation.  */
-    if (exp == 0 || exp == 0x7ff)
-        return 0;
-
-    /* Restore implicit bit.  */
-    frac |= 0x10000000000000ull;
-
-    /* Note that neither overflow exceptions nor inexact exceptions
-       are desired.  This lets us streamline the checks quite a bit.  */
-    shift = exp - 1023 - 52;
-    if (shift >= 0) {
-        /* In this case the number is so large that we must shift
-           the fraction left.  There is no rounding to do.  */
-        if (shift < 63) {
-            ret = frac << shift;
-        }
+    if (exp == 0) {
+        if (unlikely(frac != 0))
+            goto do_underflow;
+    } else if (exp == 0x7ff) {
+        if (frac == 0)
+            exc = float_flag_overflow;
+        else
+            exc = float_flag_invalid;
     } else {
-        uint64_t round;
-
-        /* In this case the number is smaller than the fraction as
-           represented by the 52 bit number.  Here we must think 
-           about rounding the result.  Handle this by shifting the
-           fractional part of the number into the high bits of ROUND.
-           This will let us efficiently handle round-to-nearest.  */
-        shift = -shift;
-        if (shift < 63) {
-            ret = frac >> shift;
-            round = frac << (64 - shift);
+        /* Restore implicit bit.  */
+        frac |= 0x10000000000000ull;
+
+        /* Note that neither overflow exceptions nor inexact exceptions
+           are desired.  This lets us streamline the checks quite a bit.  */
+        shift = exp - 1023 - 52;
+        if (shift >= 0) {
+            /* In this case the number is so large that we must shift
+               the fraction left.  There is no rounding to do.  */
+            if (shift < 63) {
+                ret = frac << shift;
+                if ((ret >> shift) != frac)
+                    exc = float_flag_overflow;
+            }
         } else {
-            /* The exponent is so small we shift out everything.
-               Leave a sticky bit for proper rounding below.  */
-            round = 1;
-        }
+            uint64_t round;
+
+            /* In this case the number is smaller than the fraction as
+               represented by the 52 bit number.  Here we must think 
+               about rounding the result.  Handle this by shifting the
+               fractional part of the number into the high bits of ROUND.
+               This will let us efficiently handle round-to-nearest.  */
+            shift = -shift;
+            if (shift < 63) {
+                ret = frac >> shift;
+                round = frac << (64 - shift);
+            } else {
+                /* The exponent is so small we shift out everything.
+                   Leave a sticky bit for proper rounding below.  */
+            do_underflow:
+                round = 1;
+            }
 
-        if (round) {
-            switch (rounding_mode) {
-            case float_round_nearest_even:
-                if (round == (1ull << 63)) {
-                    /* Remaining fraction is exactly 0.5; round to even.  */
-                    ret += (ret & 1);
-                } else if (round > (1ull << 63)) {
-                    ret += 1;
+            if (round) {
+                exc = float_flag_inexact;
+                switch (FP_STATUS.float_rounding_mode) {
+                case float_round_nearest_even:
+                    if (round == (1ull << 63)) {
+                        /* Fraction is exactly 0.5; round to even.  */
+                        ret += (ret & 1);
+                    } else if (round > (1ull << 63)) {
+                        ret += 1;
+                    }
+                    break;
+                case float_round_to_zero:
+                    break;
+                case float_round_up:
+                    if (!sign)
+                        ret += 1;
+                    break;
+                case float_round_down:
+                    if (sign)
+                        ret += 1;
+                    break;
                 }
-                break;
-            case float_round_to_zero:
-                break;
-            case float_round_up:
-                if (!sign)
-                    ret += 1;
-                break;
-            case float_round_down:
-                if (sign)
-                    ret += 1;
-                break;
             }
         }
+        if (sign)
+            ret = -ret;
     }
+    if (unlikely(exc))
+        float_raise(exc, &FP_STATUS);
 
-    if (sign)
-        ret = -ret;
     return ret;
 }
 
 uint64_t helper_cvttq (uint64_t a, uint32_t quals)
 {
     uint64_t ret;
+    uint32_t token;
 
-    a = remap_ieee_input(quals, a);
-
-    if (quals & QUAL_V) {
-        float64 fa = t_to_float64(a);
-        uint32_t token;
-
-        token = begin_fp_exception();
-        if ((quals & QUAL_RM_MASK) == QUAL_RM_C) {
-            ret = float64_to_int64_round_to_zero(fa, &FP_STATUS);
-        } else {
-            token |= begin_fp_roundmode(quals);
-            ret = float64_to_int64(fa, &FP_STATUS);
-            end_fp_roundmode(token);
-        }
-        end_fp_exception(quals, token);
-    } else {
-        uint32_t round_mode;
-
-        switch (quals & QUAL_RM_MASK) {
-        case QUAL_RM_N:
-            round_mode = float_round_nearest_even;
-            break;
-        case QUAL_RM_C:
-        default:
-            round_mode = float_round_to_zero;
-            break;
-        case QUAL_RM_M:
-            round_mode = float_round_down;
-            break;
-        case QUAL_RM_D:
-            round_mode = FP_STATUS.float_rounding_mode;
-            break;
-        }
+    /* ??? There's an arugument to be made that when /S is enabled, we
+       should provide the standard IEEE saturated result, instead of
+       the truncated result that we *must* provide when /V is disabled.
+       However, that's not how either the Tru64 or Linux completion
+       handlers actually work, and GCC knows it.  */
 
-        ret = cvttq_noqual_internal(a, round_mode);
-    }
+    token = begin_fp(quals);
+    a = remap_ieee_input(quals, a);
+    ret = cvttq_internal(a);
+    end_fp(quals, token);
 
     return ret;
 }