Skip to content

Commit

Permalink
isfinite() substitute that works despite -ffast-math
Browse files Browse the repository at this point in the history
With -ffast-math the compiler treats operations on NaNs or infinities
like undefined behavior, and optimizes out isnan or isfinite. But we can
trick it using bit arithmetic instead.

Use this substitute to replace existing calls to isfinite and isnan.

Unit tests included.
  • Loading branch information
slipher committed May 27, 2024
1 parent 2ae2181 commit 0c40d95
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 4 deletions.
1 change: 1 addition & 0 deletions src.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ set(ENGINETESTLIST
${COMMON_DIR}/ColorTest.cpp
${COMMON_DIR}/StringTest.cpp
${COMMON_DIR}/cm/unittest.cpp
${COMMON_DIR}/MathTest.cpp
${COMMON_DIR}/UtilTest.cpp
${ENGINE_DIR}/framework/CommandSystemTest.cpp
)
Expand Down
6 changes: 6 additions & 0 deletions src/common/Endian.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
# error The file boost/detail/endian.hpp needs to be set up for your CPU type.
#endif

// forward declaration
namespace Util {
template<typename ToT, typename FromT>
ToT bit_cast(FromT from);
}

// Compilers are smart enough to optimize these to a single bswap instruction
inline int16_t Swap16(int16_t x)
{
Expand Down
19 changes: 18 additions & 1 deletion src/common/Math.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

namespace Math {

// Returns min if value is NaN
// This is designed to return min if value is NaN. That's not guaranteed to work when
// compiling with fast-math flags though.
template<typename T> inline WARN_UNUSED_RESULT
T Clamp(T value, T min, T max)
{
Expand All @@ -47,6 +48,22 @@ namespace Math {
return value;
}

// IsFinite: Replacements for std::isfinite that should work even with fast-math flags

// An IEEE754 float is finite when the exponent is not all ones.
// 'volatile' serves as an optimization barrier against the compiler assuming that
// the float can never have a NaN bit pattern.
inline bool IsFinite(float x)
{
volatile uint32_t bits = Util::bit_cast<uint32_t>(x);
return ~bits & 0x7f800000;
}

inline bool IsFinite(double x)
{
volatile uint64_t bits = Util::bit_cast<uint64_t>(x);
return ~bits & 0x7ff0000000000000;
}
}

#include "math/Vector.h"
Expand Down
66 changes: 66 additions & 0 deletions src/common/MathTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
===========================================================================
Daemon BSD Source Code
Copyright (c) 2024, Daemon Developers
All rights reserved.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.
* Neither the name of the Daemon developers nor the
names of its contributors may be used to endorse or promote products
derived from this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL DAEMON DEVELOPERS BE LIABLE FOR ANY
DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
===========================================================================
*/

#include <gtest/gtest.h>

#include "common/Common.h"

namespace Math {
namespace {

// Uncomment this and the tests should fail with GCC or Clang in release mode :-)
//#define IsFinite std::isfinite

TEST(IsFiniteTest, Float)
{
ASSERT_TRUE(IsFinite(-0.0f));
ASSERT_TRUE(IsFinite(std::numeric_limits<float>::max()));
ASSERT_TRUE(IsFinite(std::numeric_limits<float>::min()));
ASSERT_TRUE(IsFinite(123.45f));

ASSERT_FALSE(IsFinite(std::stof("nan")));
ASSERT_FALSE(IsFinite(std::stof("inf")));
ASSERT_FALSE(IsFinite(std::stof("-inf")));
}

TEST(IsFiniteTest, Double)
{
ASSERT_TRUE(IsFinite(-0.0));
ASSERT_TRUE(IsFinite(std::numeric_limits<double>::max()));
ASSERT_TRUE(IsFinite(std::numeric_limits<double>::min()));
ASSERT_TRUE(IsFinite(123.45));

ASSERT_FALSE(IsFinite(std::stod("nan")));
ASSERT_FALSE(IsFinite(std::stod("inf")));
ASSERT_FALSE(IsFinite(std::stod("-inf")));
}

} // namespace Math
} // namespace
4 changes: 2 additions & 2 deletions src/engine/audio/Audio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ namespace Audio {
}

bool IsValidVector(Vec3 v) {
return not std::isnan(v[0]) and not std::isnan(v[1]) and not std::isnan(v[2]);
return Math::IsFinite(v[0]) && Math::IsFinite(v[1]) && Math::IsFinite(v[2]);
}

bool Init() {
Expand Down Expand Up @@ -436,7 +436,7 @@ namespace Audio {
return;
}

if (slotNum < 0 or slotNum >= N_REVERB_SLOTS or std::isnan(ratio)) {
if (slotNum < 0 or slotNum >= N_REVERB_SLOTS or !Math::IsFinite(ratio)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/engine/audio/Emitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ namespace Audio {
void UpdateReverbSlot(int slotNum, std::string name, float ratio) {
ASSERT_GE(slotNum, 0);
ASSERT_LT(slotNum, N_REVERB_SLOTS);
ASSERT(!std::isnan(ratio));
ASSERT(Math::IsFinite(ratio));

auto& slot = reverbSlots[slotNum];

Expand Down

0 comments on commit 0c40d95

Please sign in to comment.