From 46a4243d9a724f100aaeb2011dbda2a5e222f6ae Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 9 May 2018 09:36:17 -0400 Subject: [PATCH 1/3] FloatUtils: Remove using namespace std in ApproximateReciprocal() This was made quite a long time ago when we supported 32-bit ARM targets --- Source/Core/Common/FloatUtils.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Source/Core/Common/FloatUtils.cpp b/Source/Core/Common/FloatUtils.cpp index 4bbb4266d6..0cd78c30da 100644 --- a/Source/Core/Common/FloatUtils.cpp +++ b/Source/Core/Common/FloatUtils.cpp @@ -167,12 +167,6 @@ const std::array fres_expected = {{ // Used by fres and ps_res. double ApproximateReciprocal(double val) { - // We are using namespace std scoped here because the Android NDK is complete trash as usual - // For 32bit targets(mips, ARMv7, x86) it doesn't provide an implementation of std::copysign - // but instead provides just global namespace copysign implementations. - // The workaround for this is to just use namespace std within this function's scope - // That way on real toolchains it will use the std:: variant like normal. - using namespace std; union { double valf; @@ -186,23 +180,23 @@ double ApproximateReciprocal(double val) // Special case 0 if (mantissa == 0 && exponent == 0) - return copysign(std::numeric_limits::infinity(), valf); + return std::copysign(std::numeric_limits::infinity(), valf); // Special case NaN-ish numbers if (exponent == (0x7FFLL << 52)) { if (mantissa == 0) - return copysign(0.0, valf); + return std::copysign(0.0, valf); return 0.0 + valf; } // Special case small inputs if (exponent < (895LL << 52)) - return copysign(std::numeric_limits::max(), valf); + return std::copysign(std::numeric_limits::max(), valf); // Special case large inputs if (exponent >= (1149LL << 52)) - return copysign(0.0, valf); + return std::copysign(0.0, valf); exponent = (0x7FDLL << 52) - exponent; From fe218ea3f60ccb49b88c8a5bdcc70f75455dc761 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 9 May 2018 09:40:40 -0400 Subject: [PATCH 2/3] FloatUtils: Remove union type punning from ApproximateReciprocal functions This form of type punning invokes undefined behavior in C++ --- Source/Core/Common/FloatUtils.cpp | 71 ++++++++++++++++--------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/Source/Core/Common/FloatUtils.cpp b/Source/Core/Common/FloatUtils.cpp index 0cd78c30da..a05d62eeb0 100644 --- a/Source/Core/Common/FloatUtils.cpp +++ b/Source/Core/Common/FloatUtils.cpp @@ -5,6 +5,7 @@ #include "Common/FloatUtils.h" #include +#include namespace Common { @@ -99,20 +100,20 @@ const std::array frsqrte_expected = {{ double ApproximateReciprocalSquareRoot(double val) { - union - { - double valf; - s64 vali; - }; - valf = val; - s64 mantissa = vali & ((1LL << 52) - 1); - s64 sign = vali & (1ULL << 63); - s64 exponent = vali & (0x7FFLL << 52); + s64 integral; + std::memcpy(&integral, &val, sizeof(integral)); + + s64 mantissa = integral & ((1LL << 52) - 1); + const s64 sign = integral & (1ULL << 63); + s64 exponent = integral & (0x7FFLL << 52); // Special case 0 if (mantissa == 0 && exponent == 0) + { return sign ? -std::numeric_limits::infinity() : std::numeric_limits::infinity(); + } + // Special case NaN-ish numbers if (exponent == (0x7FFLL << 52)) { @@ -124,7 +125,7 @@ double ApproximateReciprocalSquareRoot(double val) return 0.0; } - return 0.0 + valf; + return 0.0 + val; } // Negative numbers return NaN @@ -143,15 +144,18 @@ double ApproximateReciprocalSquareRoot(double val) exponent += 1LL << 52; } - bool odd_exponent = !(exponent & (1LL << 52)); + const bool odd_exponent = !(exponent & (1LL << 52)); exponent = ((0x3FFLL << 52) - ((exponent - (0x3FELL << 52)) / 2)) & (0x7FFLL << 52); + integral = sign | exponent; - int i = (int)(mantissa >> 37); - vali = sign | exponent; - int index = i / 2048 + (odd_exponent ? 16 : 0); + const int i = static_cast(mantissa >> 37); + const int index = i / 2048 + (odd_exponent ? 16 : 0); const auto& entry = frsqrte_expected[index]; - vali |= (s64)(entry.m_base - entry.m_dec * (i % 2048)) << 26; - return valf; + integral |= static_cast(entry.m_base - entry.m_dec * (i % 2048)) << 26; + + double result; + std::memcpy(&result, &integral, sizeof(result)); + return result; } const std::array fres_expected = {{ @@ -167,44 +171,43 @@ const std::array fres_expected = {{ // Used by fres and ps_res. double ApproximateReciprocal(double val) { - union - { - double valf; - s64 vali; - }; + s64 integral; + std::memcpy(&integral, &val, sizeof(integral)); - valf = val; - s64 mantissa = vali & ((1LL << 52) - 1); - s64 sign = vali & (1ULL << 63); - s64 exponent = vali & (0x7FFLL << 52); + const s64 mantissa = integral & ((1LL << 52) - 1); + const s64 sign = integral & (1ULL << 63); + s64 exponent = integral & (0x7FFLL << 52); // Special case 0 if (mantissa == 0 && exponent == 0) - return std::copysign(std::numeric_limits::infinity(), valf); + return std::copysign(std::numeric_limits::infinity(), val); // Special case NaN-ish numbers if (exponent == (0x7FFLL << 52)) { if (mantissa == 0) - return std::copysign(0.0, valf); - return 0.0 + valf; + return std::copysign(0.0, val); + return 0.0 + val; } // Special case small inputs if (exponent < (895LL << 52)) - return std::copysign(std::numeric_limits::max(), valf); + return std::copysign(std::numeric_limits::max(), val); // Special case large inputs if (exponent >= (1149LL << 52)) - return std::copysign(0.0, valf); + return std::copysign(0.0, val); exponent = (0x7FDLL << 52) - exponent; - int i = (int)(mantissa >> 37); + const int i = static_cast(mantissa >> 37); const auto& entry = fres_expected[i / 1024]; - vali = sign | exponent; - vali |= (s64)(entry.m_base - (entry.m_dec * (i % 1024) + 1) / 2) << 29; - return valf; + integral = sign | exponent; + integral |= static_cast(entry.m_base - (entry.m_dec * (i % 1024) + 1) / 2) << 29; + + double result; + std::memcpy(&result, &integral, sizeof(result)); + return result; } } // namespace Common From f29e7fea2a776c286fc2ca85d66e015f122fdcd8 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 9 May 2018 09:51:38 -0400 Subject: [PATCH 3/3] FloatUtils: Remove union type punning from ClassifyX functions Type-punning via unions is undefined behavior in C++ Also take the liberty of cleaning these up a little bit by removing unnecessary else usages. --- Source/Core/Common/FloatUtils.cpp | 102 ++++++++++++++---------------- 1 file changed, 46 insertions(+), 56 deletions(-) diff --git a/Source/Core/Common/FloatUtils.cpp b/Source/Core/Common/FloatUtils.cpp index a05d62eeb0..5c51f805c0 100644 --- a/Source/Core/Common/FloatUtils.cpp +++ b/Source/Core/Common/FloatUtils.cpp @@ -11,80 +11,70 @@ namespace Common { u32 ClassifyDouble(double dvalue) { - // TODO: Optimize the below to be as fast as possible. - IntDouble value(dvalue); - u64 sign = value.i & DOUBLE_SIGN; - u64 exp = value.i & DOUBLE_EXP; + u64 ivalue; + std::memcpy(&ivalue, &dvalue, sizeof(ivalue)); + + const u64 sign = ivalue & DOUBLE_SIGN; + const u64 exp = ivalue & DOUBLE_EXP; + if (exp > DOUBLE_ZERO && exp < DOUBLE_EXP) { // Nice normalized number. return sign ? PPC_FPCLASS_NN : PPC_FPCLASS_PN; } - else + + const u64 mantissa = ivalue & DOUBLE_FRAC; + if (mantissa) { - u64 mantissa = value.i & DOUBLE_FRAC; - if (mantissa) - { - if (exp) - { - return PPC_FPCLASS_QNAN; - } - else - { - // Denormalized number. - return sign ? PPC_FPCLASS_ND : PPC_FPCLASS_PD; - } - } - else if (exp) - { - // Infinite - return sign ? PPC_FPCLASS_NINF : PPC_FPCLASS_PINF; - } - else - { - // Zero - return sign ? PPC_FPCLASS_NZ : PPC_FPCLASS_PZ; - } + if (exp) + return PPC_FPCLASS_QNAN; + + // Denormalized number. + return sign ? PPC_FPCLASS_ND : PPC_FPCLASS_PD; } + + if (exp) + { + // Infinite + return sign ? PPC_FPCLASS_NINF : PPC_FPCLASS_PINF; + } + + // Zero + return sign ? PPC_FPCLASS_NZ : PPC_FPCLASS_PZ; } u32 ClassifyFloat(float fvalue) { - // TODO: Optimize the below to be as fast as possible. - IntFloat value(fvalue); - u32 sign = value.i & FLOAT_SIGN; - u32 exp = value.i & FLOAT_EXP; + u32 ivalue; + std::memcpy(&ivalue, &fvalue, sizeof(ivalue)); + + const u32 sign = ivalue & FLOAT_SIGN; + const u32 exp = ivalue & FLOAT_EXP; + if (exp > FLOAT_ZERO && exp < FLOAT_EXP) { // Nice normalized number. return sign ? PPC_FPCLASS_NN : PPC_FPCLASS_PN; } - else + + const u32 mantissa = ivalue & FLOAT_FRAC; + if (mantissa) { - u32 mantissa = value.i & FLOAT_FRAC; - if (mantissa) - { - if (exp) - { - return PPC_FPCLASS_QNAN; // Quiet NAN - } - else - { - // Denormalized number. - return sign ? PPC_FPCLASS_ND : PPC_FPCLASS_PD; - } - } - else if (exp) - { - // Infinite - return sign ? PPC_FPCLASS_NINF : PPC_FPCLASS_PINF; - } - else - { - // Zero - return sign ? PPC_FPCLASS_NZ : PPC_FPCLASS_PZ; - } + if (exp) + return PPC_FPCLASS_QNAN; // Quiet NAN + + // Denormalized number. + return sign ? PPC_FPCLASS_ND : PPC_FPCLASS_PD; } + + if (exp) + { + // Infinite + return sign ? PPC_FPCLASS_NINF : PPC_FPCLASS_PINF; + } + + // Zero + return sign ? PPC_FPCLASS_NZ : PPC_FPCLASS_PZ; } const std::array frsqrte_expected = {{