From 9ef64245fa164b20db592fa866f846feb9559b14 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sun, 9 Mar 2014 19:34:58 +0100 Subject: [PATCH 1/2] MathUtil: fix IsQNAN() The constants were one nibble too short and the lower 51 bits don't actually have to be zero. --- Source/Core/Common/MathUtil.h | 39 ++++++++++++------- .../PowerPC/Interpreter/Interpreter_FPUtils.h | 9 ++--- Source/UnitTests/Common/MathUtilTest.cpp | 17 ++++++-- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/Source/Core/Common/MathUtil.h b/Source/Core/Common/MathUtil.h index 0febed3014..9ddacd6e1c 100644 --- a/Source/Core/Common/MathUtil.h +++ b/Source/Core/Common/MathUtil.h @@ -20,16 +20,18 @@ inline void Clamp(T* val, const T& min, const T& max) *val = max; } +// The most significant bit of the fraction is an is-quiet bit on all architectures we care about. static const u64 DOUBLE_SIGN = 0x8000000000000000ULL, - DOUBLE_EXP = 0x7FF0000000000000ULL, - DOUBLE_FRAC = 0x000FFFFFFFFFFFFFULL, - DOUBLE_ZERO = 0x0000000000000000ULL; + DOUBLE_EXP = 0x7FF0000000000000ULL, + DOUBLE_FRAC = 0x000FFFFFFFFFFFFFULL, + DOUBLE_ZERO = 0x0000000000000000ULL, + DOUBLE_QBIT = 0x0008000000000000ULL; static const u32 FLOAT_SIGN = 0x80000000, - FLOAT_EXP = 0x7F800000, - FLOAT_FRAC = 0x007FFFFF, - FLOAT_ZERO = 0x00000000; + FLOAT_EXP = 0x7F800000, + FLOAT_FRAC = 0x007FFFFF, + FLOAT_ZERO = 0x00000000; union IntDouble { double d; @@ -40,34 +42,41 @@ union IntFloat { u32 i; }; +inline bool IsINF(double d) +{ + IntDouble x; x.d = d; + return (x.i & ~DOUBLE_SIGN) == DOUBLE_EXP; +} + inline bool IsNAN(double d) { IntDouble x; x.d = d; - return ( ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && - ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) ); + return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && + ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO); } inline bool IsQNAN(double d) { IntDouble x; x.d = d; - return ( ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && - ((x.i & 0x0007fffffffffffULL) == 0x000000000000000ULL) && - ((x.i & 0x000800000000000ULL) == 0x000800000000000ULL) ); + return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && + ((x.i & DOUBLE_QBIT) == DOUBLE_QBIT); } inline bool IsSNAN(double d) { IntDouble x; x.d = d; - return( ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && - ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) && - ((x.i & 0x0008000000000000ULL) == DOUBLE_ZERO) ); + return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && + ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) && + ((x.i & DOUBLE_QBIT) == DOUBLE_ZERO); } inline float FlushToZero(float f) { IntFloat x; x.f = f; if ((x.i & FLOAT_EXP) == 0) + { x.i &= FLOAT_SIGN; // turn into signed zero + } return x.f; } @@ -75,7 +84,9 @@ inline double FlushToZero(double d) { IntDouble x; x.d = d; if ((x.i & DOUBLE_EXP) == 0) + { x.i &= DOUBLE_SIGN; // turn into signed zero + } return x.d; } diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h index fa5e6ff2f4..72136d71d3 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h @@ -33,22 +33,19 @@ const u32 FPSCR_VXSQRT = (u32)1 << (31 - 22); const u32 FPSCR_VXCVI = (u32)1 << (31 - 23); const u32 FPSCR_VX_ANY = FPSCR_VXSNAN | FPSCR_VXISI | FPSCR_VXIDI | FPSCR_VXZDZ | - FPSCR_VXIMZ | FPSCR_VXVC | FPSCR_VXSOFT | FPSCR_VXSQRT | FPSCR_VXCVI; + FPSCR_VXIMZ | FPSCR_VXVC | FPSCR_VXSOFT | FPSCR_VXSQRT | FPSCR_VXCVI; const u32 FPSCR_ANY_X = FPSCR_OX | FPSCR_UX | FPSCR_ZX | FPSCR_XX | FPSCR_VX_ANY; const u64 PPC_NAN_U64 = 0x7ff8000000000000ull; const double PPC_NAN = *(double* const)&PPC_NAN_U64; -inline bool IsINF(double x) -{ - return ((*(u64*)&x) & ~DOUBLE_SIGN) == DOUBLE_EXP; -} - inline void SetFPException(u32 mask) { if ((FPSCR.Hex & mask) != mask) + { FPSCR.FX = 1; + } FPSCR.Hex |= mask; } diff --git a/Source/UnitTests/Common/MathUtilTest.cpp b/Source/UnitTests/Common/MathUtilTest.cpp index a6924f9cf7..56ade34522 100644 --- a/Source/UnitTests/Common/MathUtilTest.cpp +++ b/Source/UnitTests/Common/MathUtilTest.cpp @@ -2,8 +2,8 @@ // Licensed under GPLv2 // Refer to the license.txt file included. -#include #include +#include #include "Common/MathUtil.h" @@ -27,19 +27,28 @@ TEST(MathUtil, Clamp) EXPECT_EQ(0.0, ClampAndReturn(-1.0, 0.0, 2.0)); } +TEST(MathUtil, IsINF) +{ + EXPECT_TRUE(MathUtil::IsINF( std::numeric_limits::infinity())); + EXPECT_TRUE(MathUtil::IsINF(-std::numeric_limits::infinity())); +} + TEST(MathUtil, IsNAN) { - EXPECT_TRUE(MathUtil::IsNAN(nan(""))); + EXPECT_TRUE(MathUtil::IsNAN(std::numeric_limits::quiet_NaN())); + EXPECT_TRUE(MathUtil::IsNAN(std::numeric_limits::signaling_NaN())); } TEST(MathUtil, IsQNAN) { - // TODO + EXPECT_TRUE(MathUtil::IsQNAN(std::numeric_limits::quiet_NaN())); + EXPECT_FALSE(MathUtil::IsQNAN(std::numeric_limits::signaling_NaN())); } TEST(MathUtil, IsSNAN) { - // TODO + EXPECT_FALSE(MathUtil::IsSNAN(std::numeric_limits::quiet_NaN())); + EXPECT_TRUE(MathUtil::IsSNAN(std::numeric_limits::signaling_NaN())); } TEST(MathUtil, Log2) From 16885d0f74521c904d76dc1e0cdac2d2b690cdc2 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sun, 9 Mar 2014 19:31:50 +0100 Subject: [PATCH 2/2] Interpreter: less duplicate code in float compares --- Source/Core/Core/PowerPC/Gekko.h | 3 +- .../Core/PowerPC/Interpreter/Interpreter.h | 3 + .../PowerPC/Interpreter/Interpreter_FPUtils.h | 8 ++ .../Interpreter/Interpreter_FloatingPoint.cpp | 62 +++++++++---- .../Interpreter/Interpreter_Paired.cpp | 86 +------------------ 5 files changed, 60 insertions(+), 102 deletions(-) diff --git a/Source/Core/Core/PowerPC/Gekko.h b/Source/Core/Core/PowerPC/Gekko.h index 4141d074c7..a100c97c97 100644 --- a/Source/Core/Core/PowerPC/Gekko.h +++ b/Source/Core/Core/PowerPC/Gekko.h @@ -410,7 +410,8 @@ union UReg_FPSCR u32 VXSOFT : 1; // reserved u32 : 1; - // Floating point result flags (not sticky) + // Floating point result flags (includes FPCC) (not sticky) + // from more to less significand: class, <, >, =, ? u32 FPRF : 5; // Fraction inexact (not sticky) u32 FI : 1; diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.h b/Source/Core/Core/PowerPC/Interpreter/Interpreter.h index 0e58f98776..b1321df18d 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.h +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.h @@ -326,6 +326,9 @@ private: Interpreter(const Interpreter &); Interpreter & operator=(const Interpreter &); + static void Helper_FloatCompareOrdered(UGeckoInstruction _inst, double a, double b); + static void Helper_FloatCompareUnordered(UGeckoInstruction _inst, double a, double b); + // TODO: These should really be in the save state, although it's unlikely to matter much. // They are for lwarx and its friend stwcxd. static bool g_bReserve; diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h index 72136d71d3..5131e526b3 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h @@ -40,6 +40,14 @@ const u32 FPSCR_ANY_X = FPSCR_OX | FPSCR_UX | FPSCR_ZX | FPSCR_XX | FPSCR_V const u64 PPC_NAN_U64 = 0x7ff8000000000000ull; const double PPC_NAN = *(double* const)&PPC_NAN_U64; +// the 4 less-significand bits in FPSCR[FPRF] +enum FPCC { + FL = 8, // < + FG = 4, // > + FE = 2, // = + FU = 1, // ? +}; + inline void SetFPException(u32 mask) { if ((FPSCR.Hex & mask) != mask) diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FloatingPoint.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FloatingPoint.cpp index 3b51496663..4a375ff06b 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_FloatingPoint.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_FloatingPoint.cpp @@ -33,29 +33,36 @@ void Interpreter::Helper_UpdateCR1(double _fValue) SetCRField(1, (FPSCR.FX << 4) | (FPSCR.FEX << 3) | (FPSCR.VX << 2) | FPSCR.OX); } -void Interpreter::fcmpo(UGeckoInstruction _inst) +void Interpreter::Helper_FloatCompareOrdered(UGeckoInstruction _inst, double fa, double fb) { - double fa = rPS0(_inst.FA); - double fb = rPS0(_inst.FB); - int compareResult; - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; - else + if (fa < fb) + { + compareResult = FPCC::FL; + } + else if (fa > fb) + { + compareResult = FPCC::FG; + } + else if (fa == fb) + { + compareResult = FPCC::FE; + } + else // NaN { FPSCR.FX = 1; - compareResult = 1; + compareResult = FPCC::FU; if (IsSNAN(fa) || IsSNAN(fb)) { SetFPException(FPSCR_VXSNAN); if (FPSCR.VE == 0) + { SetFPException(FPSCR_VXVC); + } } - else + else // QNaN { - //if (IsQNAN(fa) || IsQNAN(fb)) // this is always true SetFPException(FPSCR_VXVC); } } @@ -64,21 +71,28 @@ void Interpreter::fcmpo(UGeckoInstruction _inst) SetCRField(_inst.CRFD, compareResult); } -void Interpreter::fcmpu(UGeckoInstruction _inst) +void Interpreter::Helper_FloatCompareUnordered(UGeckoInstruction _inst, double fa, double fb) { - double fa = rPS0(_inst.FA); - double fb = rPS0(_inst.FB); - int compareResult; - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; + if (fa < fb) + { + compareResult = FPCC::FL; + } + else if (fa > fb) + { + compareResult = FPCC::FG; + } + else if (fa == fb) + { + compareResult = FPCC::FE; + } else { - compareResult = 1; + compareResult = FPCC::FU; if (IsSNAN(fa) || IsSNAN(fb)) { + FPSCR.FX = 1; SetFPException(FPSCR_VXSNAN); } } @@ -86,6 +100,16 @@ void Interpreter::fcmpu(UGeckoInstruction _inst) SetCRField(_inst.CRFD, compareResult); } +void Interpreter::fcmpo(UGeckoInstruction _inst) +{ + Helper_FloatCompareOrdered(_inst, rPS0(_inst.FA), rPS0(_inst.FB)); +} + +void Interpreter::fcmpu(UGeckoInstruction _inst) +{ + Helper_FloatCompareUnordered(_inst, rPS0(_inst.FA), rPS0(_inst.FB)); +} + // Apply current rounding mode void Interpreter::fctiwx(UGeckoInstruction _inst) { diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Paired.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Paired.cpp index ce99a5cfe9..c50798b308 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Paired.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Paired.cpp @@ -354,100 +354,22 @@ void Interpreter::ps_madds1(UGeckoInstruction _inst) void Interpreter::ps_cmpu0(UGeckoInstruction _inst) { - double fa = rPS0(_inst.FA); - double fb = rPS0(_inst.FB); - int compareResult; - - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; - else - { - compareResult = 1; - if (IsSNAN(fa) || IsSNAN(fb)) - { - SetFPException(FPSCR_VXSNAN); - } - } - FPSCR.FPRF = compareResult; - SetCRField(_inst.CRFD, compareResult); + Helper_FloatCompareUnordered(_inst, rPS0(_inst.FA), rPS0(_inst.FB)); } void Interpreter::ps_cmpo0(UGeckoInstruction _inst) { - double fa = rPS0(_inst.FA); - double fb = rPS0(_inst.FB); - int compareResult; - - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; - else - { - compareResult = 1; - if (IsSNAN(fa) || IsSNAN(fb)) - { - SetFPException(FPSCR_VXSNAN); - if (!FPSCR.VE) - SetFPException(FPSCR_VXVC); - } - else - { - //if (IsQNAN(fa) || IsQNAN(fb)) // this is always true - SetFPException(FPSCR_VXVC); - } - } - FPSCR.FPRF = compareResult; - SetCRField(_inst.CRFD, compareResult); + Helper_FloatCompareOrdered(_inst, rPS0(_inst.FA), rPS0(_inst.FB)); } void Interpreter::ps_cmpu1(UGeckoInstruction _inst) { - double fa = rPS1(_inst.FA); - double fb = rPS1(_inst.FB); - int compareResult; - - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; - else - { - compareResult = 1; - if (IsSNAN(fa) || IsSNAN(fb)) - { - SetFPException(FPSCR_VXSNAN); - } - } - FPSCR.FPRF = compareResult; - SetCRField(_inst.CRFD, compareResult); + Helper_FloatCompareUnordered(_inst, rPS1(_inst.FA), rPS1(_inst.FB)); } void Interpreter::ps_cmpo1(UGeckoInstruction _inst) { - double fa = rPS1(_inst.FA); - double fb = rPS1(_inst.FB); - int compareResult; - - if (fa < fb) compareResult = 8; - else if (fa > fb) compareResult = 4; - else if (fa == fb) compareResult = 2; - else - { - compareResult = 1; - if (IsSNAN(fa) || IsSNAN(fb)) - { - SetFPException(FPSCR_VXSNAN); - if (!FPSCR.VE) - SetFPException(FPSCR_VXVC); - } - else - { - //if (IsQNAN(fa) || IsQNAN(fb)) // this is always true - SetFPException(FPSCR_VXVC); - } - } - FPSCR.FPRF = compareResult; - SetCRField(_inst.CRFD, compareResult); + Helper_FloatCompareOrdered(_inst, rPS1(_inst.FA), rPS1(_inst.FB)); } // __________________________________________________________________________________________________