From f03b4b8f5d9b48a904966c2907dfab018f1b6335 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 15 Jul 2022 01:19:06 +0200 Subject: [PATCH 1/5] fix(math): roundUpToPowerOf2 was depending on UB: It assumed that a signed integer could overflow which is UB and thus the compiler optimizes anything that depended on that out. The result was that if the power of two is actually greater than representable, the function would go into an infinite loop --- src/util/fifo.h | 3 +-- src/util/math.h | 12 ++++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/fifo.h b/src/util/fifo.h index e0a37b05748..d41afbfa59a 100644 --- a/src/util/fifo.h +++ b/src/util/fifo.h @@ -10,9 +10,8 @@ class FIFO { public: explicit FIFO(int size) : m_data(roundUpToPowerOf2(size)) { - size = roundUpToPowerOf2(size); // If we can't represent the next higher power of 2 then bail. - if (size < 0) { + if (m_data.size() == 0) { return; } PaUtil_InitializeRingBuffer(&m_ringBuffer, diff --git a/src/util/math.h b/src/util/math.h index 5ab81690a93..1eb561a86c9 100644 --- a/src/util/math.h +++ b/src/util/math.h @@ -38,16 +38,12 @@ inline bool even(T value) { #pragma intrinsic(fabs) #endif -inline int roundUpToPowerOf2(int v) { - int power = 1; - while (power < v && power > 0) { +inline unsigned int roundUpToPowerOf2(int v) { + DEBUG_ASSERT(v >= 0); + unsigned int power = 1; + while (power <= v && power > 0) { power *= 2; } - // There is not a power of 2 higher than v representable by our - // architecture's integer size. - if (power < 0) { - return -1; - } return power; } From 6d7c77605c9b374fb47e9188f95d9f60a11b04ff Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 15 Jul 2022 01:10:04 +0200 Subject: [PATCH 2/5] refactor(util): modernize math.h --- src/util/math.h | 47 ++++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/util/math.h b/src/util/math.h index 1eb561a86c9..87f4baf6de7 100644 --- a/src/util/math.h +++ b/src/util/math.h @@ -7,29 +7,36 @@ #include "util/assert.h" #include "util/fpclassify.h" +#if __has_include() +#include +#endif + // If we don't do this then we get the C90 fabs from the global namespace which // is only defined for double. using std::fabs; #define math_max std::max #define math_min std::min -#define math_max3(a, b, c) math_max(math_max((a), (b)), (c)) -#define math_min3(a, b, c) math_min(math_min((a), (b)), (c)) +#define math_max3(a, b, c) std::max({a, b, c}); +#define math_min3(a, b, c) std::min({a, b, c}); // Restrict value to the range [min, max]. Undefined behavior if min > max. -template -inline T math_clamp(T value, T min, T max) { +template +constexpr T math_clamp(T value, T min, T max) { // DEBUG_ASSERT compiles out in release builds so it does not affect // vectorization or pipelining of clamping in tight loops. DEBUG_ASSERT(min <= max); - return math_max(min, math_min(max, value)); + return std::clamp(value, min, max); } // NOTE(rryan): It is an error to call even() on a floating point number. Do not // hack this to support floating point values! The programmer should be required // to manually convert so they are aware of the conversion. -template -inline bool even(T value) { +template +constexpr bool even(T value) { + // since we also want to this to work on size_t and ptrdiff_t, is_integer would be too strict. + static_assert(std::is_arithmetic_v && !std::is_floating_point_v, + "even only supports integral types"); return value % 2 == 0; } @@ -38,15 +45,21 @@ inline bool even(T value) { #pragma intrinsic(fabs) #endif -inline unsigned int roundUpToPowerOf2(int v) { +constexpr unsigned int roundUpToPowerOf2(int v) { DEBUG_ASSERT(v >= 0); +#ifdef __cpp_lib_bitops + const auto uv = static_cast(v); + return std::bit_ceil(uv); +#else unsigned int power = 1; - while (power <= v && power > 0) { + while (power < v && power > 0) { power *= 2; } return power; +#endif } +// TODO (XXX): make this constexpr once has constexpr support inline double roundToFraction(double value, int denominator) { int wholePart = static_cast(value); double fractionPart = value - wholePart; @@ -54,20 +67,16 @@ inline double roundToFraction(double value, int denominator) { return wholePart + numerator / denominator; } -template +// TODO (XXX): make this constexpr once has constexpr support +template inline const T ratio2db(const T a) { - static_assert(std::is_same::value || - std::is_same::value || - std::is_same::value, - "ratio2db works only for floating point types"); + static_assert(std::is_floating_point_v, "ratio2db works only for floating point types"); return log10(a) * 20; } -template +// TODO (XXX): make this constexpr once has constexpr support +template inline const T db2ratio(const T a) { - static_assert(std::is_same::value || - std::is_same::value || - std::is_same::value, - "db2ratio works only for floating point types"); + static_assert(std::is_floating_point_v, "db2ratio works only for floating point type"); return static_cast(pow(10, a / 20)); } From 9f25046609cd6e30154d3fb953a16bf143805b9a Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Wed, 20 Jul 2022 20:27:40 +0200 Subject: [PATCH 3/5] refactor(util): further improve roundUpToPowerOf2 --- src/util/math.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/math.h b/src/util/math.h index 87f4baf6de7..f0453e0da4a 100644 --- a/src/util/math.h +++ b/src/util/math.h @@ -45,14 +45,14 @@ constexpr bool even(T value) { #pragma intrinsic(fabs) #endif -constexpr unsigned int roundUpToPowerOf2(int v) { - DEBUG_ASSERT(v >= 0); -#ifdef __cpp_lib_bitops +// return value of 0 indicates failure (no greater power possible) +constexpr unsigned int roundUpToPowerOf2(unsigned int v) { const auto uv = static_cast(v); +#ifdef __cpp_lib_int_pow2 return std::bit_ceil(uv); #else unsigned int power = 1; - while (power < v && power > 0) { + while (power < uv && power > 0) { power *= 2; } return power; From c2d5458787be425b256f3ef0899a3c505e777b80 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Thu, 21 Jul 2022 17:25:19 +0200 Subject: [PATCH 4/5] fixup! refactor(util): further improve roundUpToPowerOf2 --- src/util/math.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/math.h b/src/util/math.h index f0453e0da4a..f6e0fa8afa0 100644 --- a/src/util/math.h +++ b/src/util/math.h @@ -47,12 +47,11 @@ constexpr bool even(T value) { // return value of 0 indicates failure (no greater power possible) constexpr unsigned int roundUpToPowerOf2(unsigned int v) { - const auto uv = static_cast(v); #ifdef __cpp_lib_int_pow2 - return std::bit_ceil(uv); + return std::bit_ceil(v); #else unsigned int power = 1; - while (power < uv && power > 0) { + while (power < v && power > 0) { power *= 2; } return power; From 968ee4130d4458842e51c54e919b337620ef8309 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Thu, 21 Jul 2022 22:59:03 +0200 Subject: [PATCH 5/5] fixup! refactor(util): further improve roundUpToPowerOf2 --- src/util/math.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/math.h b/src/util/math.h index f6e0fa8afa0..0a016a2d043 100644 --- a/src/util/math.h +++ b/src/util/math.h @@ -47,7 +47,7 @@ constexpr bool even(T value) { // return value of 0 indicates failure (no greater power possible) constexpr unsigned int roundUpToPowerOf2(unsigned int v) { -#ifdef __cpp_lib_int_pow2 +#if (defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L) return std::bit_ceil(v); #else unsigned int power = 1;