Merge pull request #152 from Tilka/float_stuff

Fix IsQNAN() and reduce code duplication

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_EXP  = 0x7FF0000000000000ULL,
-	DOUBLE_FRAC = 0x000FFFFFFFFFFFFFULL,
+                 DOUBLE_FRAC = 0x000FFFFFFFFFFFFFULL,
-	DOUBLE_ZERO = 0x0000000000000000ULL;
+                 DOUBLE_ZERO = 0x0000000000000000ULL,
+                 DOUBLE_QBIT = 0x0008000000000000ULL;
 
 static const u32 FLOAT_SIGN = 0x80000000,
-	FLOAT_EXP  = 0x7F800000,
+                 FLOAT_EXP  = 0x7F800000,
-	FLOAT_FRAC = 0x007FFFFF,
+                 FLOAT_FRAC = 0x007FFFFF,
-	FLOAT_ZERO = 0x00000000;
+                 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) &&
+	return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) &&
-			 ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) );
+	       ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO);
 }
 
 inline bool IsQNAN(double d)
 {
 	IntDouble x; x.d = d;
-	return ( ((x.i & DOUBLE_EXP) == DOUBLE_EXP) &&
+	return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) &&
-		     ((x.i & 0x0007fffffffffffULL) == 0x000000000000000ULL) &&
+	       ((x.i & DOUBLE_QBIT) == DOUBLE_QBIT);
-		     ((x.i & 0x000800000000000ULL) == 0x000800000000000ULL) );
 }
 
 inline bool IsSNAN(double d)
 {
 	IntDouble x; x.d = d;
-	return( ((x.i & DOUBLE_EXP) == DOUBLE_EXP) &&
+	return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) &&
-			((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) &&
+	       ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) &&
-			((x.i & 0x0008000000000000ULL) == 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;
 }
 

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;

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;

Source/Core/Core/PowerPC/Interpreter/Interpreter_FPUtils.h

@@ -33,22 +33,27 @@ 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)
+// the 4 less-significand bits in FPSCR[FPRF]
-{
+enum FPCC {
-	return ((*(u64*)&x) & ~DOUBLE_SIGN) == DOUBLE_EXP;
+	FL = 8, // <
-}
+	FG = 4, // >
+	FE = 2, // =
+	FU = 1, // ?
+};
 
 inline void SetFPException(u32 mask)
 {
 	if ((FPSCR.Hex & mask) != mask)
+	{
 		FPSCR.FX = 1;
+	}
 	FPSCR.Hex |= mask;
 }
 

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;
+	if (fa < fb)
-	else if (fa > fb)  compareResult = 4;
+	{
-	else if (fa == fb) compareResult = 2;
+		compareResult = FPCC::FL;
-	else
+	}
+	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;
+	if (fa < fb)
-	else if (fa > fb)  compareResult = 4;
+	{
-	else if (fa == fb) compareResult = 2;
+		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)
 {

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);
+	Helper_FloatCompareUnordered(_inst, rPS0(_inst.FA), rPS0(_inst.FB));
-	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);
 }
 
 void Interpreter::ps_cmpo0(UGeckoInstruction _inst)
 {
-	double fa = rPS0(_inst.FA);
+	Helper_FloatCompareOrdered(_inst, rPS0(_inst.FA), rPS0(_inst.FB));
-	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);
 }
 
 void Interpreter::ps_cmpu1(UGeckoInstruction _inst)
 {
-	double fa = rPS1(_inst.FA);
+	Helper_FloatCompareUnordered(_inst, rPS1(_inst.FA), rPS1(_inst.FB));
-	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);
 }
 
 void Interpreter::ps_cmpo1(UGeckoInstruction _inst)
 {
-	double fa = rPS1(_inst.FA);
+	Helper_FloatCompareOrdered(_inst, rPS1(_inst.FA), rPS1(_inst.FB));
-	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);
 }
 
 // __________________________________________________________________________________________________

Source/UnitTests/Common/MathUtilTest.cpp

@@ -2,8 +2,8 @@
 // Licensed under GPLv2
 // Refer to the license.txt file included.
 
-#include <cmath>
 #include <gtest/gtest.h>
+#include <limits>
 
 #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<double>::infinity()));
+	EXPECT_TRUE(MathUtil::IsINF(-std::numeric_limits<double>::infinity()));
+}
+
 TEST(MathUtil, IsNAN)
 {
-	EXPECT_TRUE(MathUtil::IsNAN(nan("")));
+	EXPECT_TRUE(MathUtil::IsNAN(std::numeric_limits<double>::quiet_NaN()));
+	EXPECT_TRUE(MathUtil::IsNAN(std::numeric_limits<double>::signaling_NaN()));
 }
 
 TEST(MathUtil, IsQNAN)
 {
-	// TODO
+	EXPECT_TRUE(MathUtil::IsQNAN(std::numeric_limits<double>::quiet_NaN()));
+	EXPECT_FALSE(MathUtil::IsQNAN(std::numeric_limits<double>::signaling_NaN()));
 }
 
 TEST(MathUtil, IsSNAN)
 {
-	// TODO
+	EXPECT_FALSE(MathUtil::IsSNAN(std::numeric_limits<double>::quiet_NaN()));
+	EXPECT_TRUE(MathUtil::IsSNAN(std::numeric_limits<double>::signaling_NaN()));
 }
 
 TEST(MathUtil, Log2)