Patchwork [2/3] target-ppc: fix default qNaN

login
register
mail settings
Submitter Aurelien Jarno
Date Jan. 2, 2011, 2:39 p.m.
Message ID <1293979183-27108-3-git-send-email-aurelien@aurel32.net>
Download mbox | patch
Permalink /patch/77183/
State New
Headers show

Comments

Aurelien Jarno - Jan. 2, 2011, 2:39 p.m.
On PPC the default qNaN doesn't have the sign bit set.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-ppc/op_helper.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Alexander Graf - Jan. 5, 2011, 5:09 p.m.
On 02.01.2011, at 15:39, Aurelien Jarno wrote:

> On PPC the default qNaN doesn't have the sign bit set.

The spec says "don't care" for the sign bit. Did you extract the value empirically? I'm not saying it's wrong - the default 32 Bit value seems to be 0x7FC0_0000 (2.06 ISA 6.6.2.2).

Hrm ... reading section 5.4.2:

A special QNaN is sometimes supplied as the default QNaN for a disabled invalid-operation exception; it has a plus sign, the leftmost 6 bits of the combination field set to 0b111110 and remaining bits in the combination field and the trailing significand field set to zero.

So I guess you're right.


Acked-by: Alexander Graf <agraf@suse.de>


Alex
Aurelien Jarno - Jan. 6, 2011, 3:03 p.m.
On Wed, Jan 05, 2011 at 06:09:34PM +0100, Alexander Graf wrote:
> 
> On 02.01.2011, at 15:39, Aurelien Jarno wrote:
> 
> > On PPC the default qNaN doesn't have the sign bit set.
> 
> The spec says "don't care" for the sign bit. Did you extract the value empirically? I'm not saying it's wrong - the default 32 Bit value seems to be 0x7FC0_0000 (2.06 ISA 6.6.2.2).

"don't care" is for detecting a qNaN which can be represented by
thousands of different values. It's different from the default qNaN.

2.06 ISA 6.6.2.2 is for vector operations, which are always 32-bit. We 
are also using this value, this time through softfloat-specialize.h 
(look at float32_default_nan).

For 64-bit values, the default value is defined in 4.3. For the long
term, we should remove the default qNaN value from
target-ppc/op_helper.c and only use the one in softfloat-specialize.h.
However it needs reworking of part of the softfloat library first.

> Hrm ... reading section 5.4.2:
> 
> A special QNaN is sometimes supplied as the default QNaN for a disabled invalid-operation exception; it has a plus sign, the leftmost 6 bits of the combination field set to 0b111110 and remaining bits in the combination field and the trailing significand field set to zero.
> 

That's for decimal floating point, and not binary floating point,
however it seems they use the same convention.

Thanks for the review.

Patch

diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 858877e..279f345 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -643,7 +643,7 @@  static inline uint64_t fload_invalid_op_excp(int op)
         env->fpscr &= ~((1 << FPSCR_FR) | (1 << FPSCR_FI));
         if (ve == 0) {
             /* Set the result to quiet NaN */
-            ret = 0xFFF8000000000000ULL;
+            ret = 0x7FF8000000000000ULL;
             env->fpscr &= ~(0xF << FPSCR_FPCC);
             env->fpscr |= 0x11 << FPSCR_FPCC;
         }
@@ -654,7 +654,7 @@  static inline uint64_t fload_invalid_op_excp(int op)
         env->fpscr &= ~((1 << FPSCR_FR) | (1 << FPSCR_FI));
         if (ve == 0) {
             /* Set the result to quiet NaN */
-            ret = 0xFFF8000000000000ULL;
+            ret = 0x7FF8000000000000ULL;
             env->fpscr &= ~(0xF << FPSCR_FPCC);
             env->fpscr |= 0x11 << FPSCR_FPCC;
         }