From b3292298c976279765926c9927f2e4cfe38b3ede Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 10 May 2018 09:54:35 -0400 Subject: [PATCH 1/3] BitUtils: Add C++14/C++17 compatible equivalent of std::bit_cast from C++2a Given bit conversions between types are quite common in emulation (particularly when it comes to floating-point among other things) it makes sense to provide a utility function that keeps all the boilerplate contained; especially considering it makes it harder to accidentally misuse std::memcpy (such as accidentally transposing arguments, etc). Another benefit of this function is that it doesn't require separating declarations from assignments, allowing variables to be declared const. This makes the scenario of of uninitialized variables being used less likely to occur. --- Source/Core/Common/BitUtils.h | 34 ++++++++++++++++++++++++ Source/UnitTests/Common/BitUtilsTest.cpp | 13 +++++++++ 2 files changed, 47 insertions(+) diff --git a/Source/Core/Common/BitUtils.h b/Source/Core/Common/BitUtils.h index 2d45902e41..c975b5c475 100644 --- a/Source/Core/Common/BitUtils.h +++ b/Source/Core/Common/BitUtils.h @@ -6,6 +6,7 @@ #include #include +#include #include namespace Common @@ -165,4 +166,37 @@ constexpr bool IsValidLowMask(const T mask) noexcept // and doesn't require special casing either edge case. return (mask & (mask + 1)) == 0; } + +/// +/// Reinterpret objects of one type as another by bit-casting between object representations. +/// +/// @remark This is the example implementation of std::bit_cast which is to be included +/// in C++2a. See http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0476r2.html +/// for more details. The only difference is this variant is not constexpr, +/// as the mechanism for bit_cast requires a compiler built-in to have that quality. +/// +/// @param source The source object to convert to another representation. +/// +/// @tparam To The type to reinterpret source as. +/// @tparam From The initial type representation of source. +/// +/// @return The representation of type From as type To. +/// +/// @pre Both To and From types must be the same size +/// @pre Both To and From types must satisfy the TriviallyCopyable concept. +/// +template +inline To BitCast(const From& source) noexcept +{ + static_assert(sizeof(From) == sizeof(To), + "BitCast source and destination types must be equal in size."); + static_assert(std::is_trivially_copyable(), + "BitCast source type must be trivially copyable."); + static_assert(std::is_trivially_copyable(), + "BitCast destination type must be trivially copyable."); + + std::aligned_storage_t storage; + std::memcpy(&storage, &source, sizeof(storage)); + return reinterpret_cast(storage); +} } // namespace Common diff --git a/Source/UnitTests/Common/BitUtilsTest.cpp b/Source/UnitTests/Common/BitUtilsTest.cpp index 4785f9dcef..e847e908bd 100644 --- a/Source/UnitTests/Common/BitUtilsTest.cpp +++ b/Source/UnitTests/Common/BitUtilsTest.cpp @@ -127,3 +127,16 @@ TEST(BitUtils, IsValidLowMask) EXPECT_FALSE(Common::IsValidLowMask((u64) ~(0b10000))); EXPECT_FALSE(Common::IsValidLowMask((u64)(~((u64)(~0b0) >> 1) | 0b1111))); } + +TEST(BitUtils, BitCast) +{ + EXPECT_EQ(0x00000000U, Common::BitCast(0.0f)); + EXPECT_EQ(0x80000000U, Common::BitCast(-0.0f)); + EXPECT_EQ(0x3F800000U, Common::BitCast(1.0f)); + EXPECT_EQ(0xBF800000U, Common::BitCast(-1.0f)); + + EXPECT_EQ(0x0000000000000000ULL, Common::BitCast(0.0)); + EXPECT_EQ(0x8000000000000000ULL, Common::BitCast(-0.0)); + EXPECT_EQ(0x3FF0000000000000ULL, Common::BitCast(1.0)); + EXPECT_EQ(0xBFF0000000000000ULL, Common::BitCast(-1.0)); +} From bde4e970f10b261dc60b1770d18cb99788fed760 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 10 May 2018 09:58:26 -0400 Subject: [PATCH 2/3] FloatUtils: Clean up memcpy usages Now that we have BitCast, we can use that instead. --- Source/Core/Common/FloatUtils.cpp | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/Source/Core/Common/FloatUtils.cpp b/Source/Core/Common/FloatUtils.cpp index 5c51f805c0..d007c161d6 100644 --- a/Source/Core/Common/FloatUtils.cpp +++ b/Source/Core/Common/FloatUtils.cpp @@ -5,15 +5,14 @@ #include "Common/FloatUtils.h" #include -#include + +#include "Common/BitUtils.h" namespace Common { u32 ClassifyDouble(double dvalue) { - u64 ivalue; - std::memcpy(&ivalue, &dvalue, sizeof(ivalue)); - + const u64 ivalue = BitCast(dvalue); const u64 sign = ivalue & DOUBLE_SIGN; const u64 exp = ivalue & DOUBLE_EXP; @@ -45,9 +44,7 @@ u32 ClassifyDouble(double dvalue) u32 ClassifyFloat(float fvalue) { - u32 ivalue; - std::memcpy(&ivalue, &fvalue, sizeof(ivalue)); - + const u32 ivalue = BitCast(fvalue); const u32 sign = ivalue & FLOAT_SIGN; const u32 exp = ivalue & FLOAT_EXP; @@ -90,9 +87,7 @@ const std::array frsqrte_expected = {{ double ApproximateReciprocalSquareRoot(double val) { - s64 integral; - std::memcpy(&integral, &val, sizeof(integral)); - + s64 integral = BitCast(val); s64 mantissa = integral & ((1LL << 52) - 1); const s64 sign = integral & (1ULL << 63); s64 exponent = integral & (0x7FFLL << 52); @@ -143,9 +138,7 @@ double ApproximateReciprocalSquareRoot(double val) const auto& entry = frsqrte_expected[index]; integral |= static_cast(entry.m_base - entry.m_dec * (i % 2048)) << 26; - double result; - std::memcpy(&result, &integral, sizeof(result)); - return result; + return BitCast(integral); } const std::array fres_expected = {{ @@ -161,9 +154,7 @@ const std::array fres_expected = {{ // Used by fres and ps_res. double ApproximateReciprocal(double val) { - s64 integral; - std::memcpy(&integral, &val, sizeof(integral)); - + s64 integral = BitCast(val); const s64 mantissa = integral & ((1LL << 52) - 1); const s64 sign = integral & (1ULL << 63); s64 exponent = integral & (0x7FFLL << 52); @@ -195,9 +186,7 @@ double ApproximateReciprocal(double val) 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; + return BitCast(integral); } } // namespace Common From 0a3631cc7657d6fe0cb577681092e9b6f926dbb9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 10 May 2018 10:35:00 -0400 Subject: [PATCH 3/3] FloatUtils: Remove IntDouble and IntFloat Type punning via unions in C++ invokes undefined behavior. Instead, leverage BitCast, our variant of C++2a's std::bit_cast --- Source/Core/Common/FloatUtils.h | 46 +++++++------------ Source/UnitTests/Common/FloatUtilsTest.cpp | 19 ++++---- .../VideoCommon/VertexLoaderTest.cpp | 13 +++--- 3 files changed, 32 insertions(+), 46 deletions(-) diff --git a/Source/Core/Common/FloatUtils.h b/Source/Core/Common/FloatUtils.h index 959a20bb17..a841165b70 100644 --- a/Source/Core/Common/FloatUtils.h +++ b/Source/Core/Common/FloatUtils.h @@ -7,6 +7,7 @@ #include #include +#include "Common/BitUtils.h" #include "Common/CommonTypes.h" namespace Common @@ -55,54 +56,39 @@ enum : u32 FLOAT_ZERO = 0x00000000 }; -union IntDouble -{ - double d; - u64 i; - - explicit IntDouble(u64 _i) : i(_i) {} - explicit IntDouble(double _d) : d(_d) {} -}; -union IntFloat -{ - float f; - u32 i; - - explicit IntFloat(u32 _i) : i(_i) {} - explicit IntFloat(float _f) : f(_f) {} -}; - inline bool IsQNAN(double d) { - IntDouble x(d); - return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && ((x.i & DOUBLE_QBIT) == DOUBLE_QBIT); + const u64 i = BitCast(d); + return ((i & DOUBLE_EXP) == DOUBLE_EXP) && ((i & DOUBLE_QBIT) == DOUBLE_QBIT); } inline bool IsSNAN(double d) { - IntDouble x(d); - return ((x.i & DOUBLE_EXP) == DOUBLE_EXP) && ((x.i & DOUBLE_FRAC) != DOUBLE_ZERO) && - ((x.i & DOUBLE_QBIT) == DOUBLE_ZERO); + const u64 i = BitCast(d); + return ((i & DOUBLE_EXP) == DOUBLE_EXP) && ((i & DOUBLE_FRAC) != DOUBLE_ZERO) && + ((i & DOUBLE_QBIT) == DOUBLE_ZERO); } inline float FlushToZero(float f) { - IntFloat x(f); - if ((x.i & FLOAT_EXP) == 0) + u32 i = BitCast(f); + if ((i & FLOAT_EXP) == 0) { - x.i &= FLOAT_SIGN; // turn into signed zero + // Turn into signed zero + i &= FLOAT_SIGN; } - return x.f; + return BitCast(i); } inline double FlushToZero(double d) { - IntDouble x(d); - if ((x.i & DOUBLE_EXP) == 0) + u64 i = BitCast(d); + if ((i & DOUBLE_EXP) == 0) { - x.i &= DOUBLE_SIGN; // turn into signed zero + // Turn into signed zero + i &= DOUBLE_SIGN; } - return x.d; + return BitCast(i); } enum PPCFpClass diff --git a/Source/UnitTests/Common/FloatUtilsTest.cpp b/Source/UnitTests/Common/FloatUtilsTest.cpp index c1f7117162..bcc3e94cf2 100644 --- a/Source/UnitTests/Common/FloatUtilsTest.cpp +++ b/Source/UnitTests/Common/FloatUtilsTest.cpp @@ -7,6 +7,7 @@ #include +#include "Common/BitUtils.h" #include "Common/FloatUtils.h" TEST(FloatUtils, IsQNAN) @@ -51,18 +52,16 @@ TEST(FloatUtils, FlushToZero) std::uniform_int_distribution dist(0x00800000u, 0x7fffffffu); for (u32 i = 0; i <= 0x007fffffu; ++i) { - Common::IntFloat x(i); - EXPECT_EQ(+0.f, Common::FlushToZero(x.f)); + u32 i_tmp = i; + EXPECT_EQ(+0.f, Common::FlushToZero(Common::BitCast(i_tmp))); - x.i = i | 0x80000000u; - EXPECT_EQ(-0.f, Common::FlushToZero(x.f)); + i_tmp |= 0x80000000u; + EXPECT_EQ(-0.f, Common::FlushToZero(Common::BitCast(i_tmp))); - x.i = dist(engine); - Common::IntFloat y(Common::FlushToZero(x.f)); - EXPECT_EQ(x.i, y.i); + i_tmp = dist(engine); + EXPECT_EQ(i_tmp, Common::BitCast(Common::FlushToZero(Common::BitCast(i_tmp)))); - x.i |= 0x80000000u; - y.f = Common::FlushToZero(x.f); - EXPECT_EQ(x.i, y.i); + i_tmp |= 0x80000000u; + EXPECT_EQ(i_tmp, Common::BitCast(Common::FlushToZero(Common::BitCast(i_tmp)))); } } diff --git a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp index 6ba3c4c464..70a8a96235 100644 --- a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp +++ b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp @@ -10,8 +10,8 @@ #include // NOLINT +#include "Common/BitUtils.h" #include "Common/Common.h" -#include "Common/FloatUtils.h" #include "VideoCommon/CPMemory.h" #include "VideoCommon/DataReader.h" #include "VideoCommon/OpcodeDecoding.h" @@ -72,14 +72,15 @@ protected: m_src.Write(val); } - void ExpectOut(float val) + void ExpectOut(float expected) { // Read unswapped. - Common::IntFloat expected(val), actual(m_dst.Read()); - if (!actual.f || actual.f != actual.f) - EXPECT_EQ(expected.i, actual.i); + const float actual = m_dst.Read(); + + if (!actual || actual != actual) + EXPECT_EQ(Common::BitCast(expected), Common::BitCast(actual)); else - EXPECT_EQ(expected.f, actual.f); + EXPECT_EQ(expected, actual); } void RunVertices(int count, int expected_count = -1)