Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions cmake/DaemonFlags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,23 @@ else()
set(WARNMODE "no-")
endif()

# Compiler options
option(USE_FLOAT_EXCEPTIONS "Use floating point exceptions with common.floatException.* cvars" OFF)
option(USE_FAST_MATH "Use fast math" ON)

# Compiler options
if (USE_FLOAT_EXCEPTIONS)
add_definitions(-DDAEMON_USE_FLOAT_EXCEPTIONS)
endif()

if (MSVC)
set_c_cxx_flag("/MP")

if (USE_FAST_MATH)
if (USE_FLOAT_EXCEPTIONS)
set_c_cxx_flag("/fp:strict")
# Don't switch on C4305 "truncation from 'double' to 'float'" every
# time an unsuffixed decimal constant is used
set_c_cxx_flag("/wd4305")
elseif (USE_FAST_MATH)
set_c_cxx_flag("/fp:fast")
endif()

Expand Down Expand Up @@ -261,6 +271,15 @@ else()
set_c_cxx_flag("-ffast-math")
endif()

if (USE_FLOAT_EXCEPTIONS)
# Floating point exceptions requires trapping math
# to avoid false positives on architectures with SSE.
set_c_cxx_flag("-ftrapping-math")
# GCC prints noisy warnings saying -ftrapping-math implies this.
set_c_cxx_flag("-fno-associative-math")
# Other optimizations from -ffast-math can be kept.
endif()

# Use hidden symbol visibility if possible.
try_c_cxx_flag(FVISIBILITY_HIDDEN "-fvisibility=hidden")

Expand Down
14 changes: 10 additions & 4 deletions src/common/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,8 @@ class InjectFaultCmd : public Cmd::StaticCmd
.detach();
} else {
if (!DoFault(args)) {
Print("Usage: %s (drop | error | segfault | intdiv | floatdiv | unreachable"
" | exception | throw | terminate | abort | freeze <seconds>) [thread]",
Print("Usage: %s (drop | error | segfault | intdiv | floatinvalid | floatdiv | floatoverflow"
" | unreachable | exception | throw | terminate | abort | freeze <seconds>) [thread]",
args.Argv(0));
}
}
Expand Down Expand Up @@ -573,9 +573,15 @@ class InjectFaultCmd : public Cmd::StaticCmd
} else if (how == "intdiv") {
volatile int n = 0;
n = n / n;
} else if (how == "floatinvalid") {
volatile float f = -1.0f;
f = sqrtf(f);
} else if (how == "floatdiv") {
volatile float x = 0;
x = x / x;
volatile float f = 0;
f = 1 / f;
} else if (how == "floatoverflow") {
volatile float f = std::numeric_limits<float>::max();
f = 2 * f;
} else if (how == "unreachable") { // May print the usage string :)
UNREACHABLE();
} else if (how == "exception") { // std::exception
Expand Down
147 changes: 147 additions & 0 deletions src/engine/framework/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,35 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <sys/file.h>
#endif

#if defined(DAEMON_USE_FLOAT_EXCEPTIONS)
#if defined(__USE_GNU) || defined(__FreeBSD__) || defined(_WIN32)
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#define DAEMON_USE_FLOAT_EXCEPTIONS_AVAILABLE
#endif
#elif defined (__APPLE__)
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_arm64)
#define DAEMON_USE_FLOAT_EXCEPTIONS_AVAILABLE
#endif
#endif

#if defined(DAEMON_USE_FLOAT_EXCEPTIONS_AVAILABLE)
#include <cfenv>
#include <cfloat>

static Cvar::Cvar<bool> common_floatExceptions_invalid("common.floatExceptions.invalid",
"enable floating point exception for operation with NaN",
Cvar::INIT, false);
static Cvar::Cvar<bool> common_floatExceptions_divByZero("common.floatExceptions.divByZero",
"enable floating point exception for division-by-zero operation",
Cvar::INIT, false);
static Cvar::Cvar<bool> common_floatExceptions_overflow("common.floatExceptions.overflow",
"enable floating point exception for operation producing an overflow",
Cvar::INIT, false);
#else
#warning Missing floating point exception implementation.
#endif
#endif

namespace Sys {
static Cvar::Cvar<bool> cvar_common_shutdownOnDrop("common.shutdownOnDrop", "shut down engine on game drop", Cvar::TEMPORARY, false);

Expand Down Expand Up @@ -355,6 +384,122 @@ static void CloseSingletonSocket()
#endif
}

static void SetFloatingPointExceptions()
{
#if defined(DAEMON_USE_FLOAT_EXCEPTIONS_AVAILABLE)
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#if defined(__USE_GNU) || defined(__FreeBSD__) || defined(__APPLE__)
int exceptions = 0;
#elif defined(_WIN32)
unsigned int exceptions = 0;
#endif

#if defined(DAEMON_USE_ARCH_INTRINSICS_i686_sse)
int mxcsr_exceptions = 0;
#endif
#elif defined(DAEMON_ARCH_arm64)
#if defined(__APPLE__)
unsigned long long fpcr_exceptions = 0;
#endif
#endif

// Operations with NaN.
if (common_floatExceptions_invalid.Get())
{
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#if defined(__USE_GNU) || defined(__FreeBSD__) || defined(__APPLE__)
exceptions |= FE_INVALID;
#elif defined(_WIN32)
exceptions |= _EM_INVALID;
#endif

#if defined(DAEMON_USE_ARCH_INTRINSICS_i686_sse)
mxcsr_exceptions |= _MM_MASK_INVALID;
#endif
#elif defined(DAEMON_ARCH_arm64)
#if defined(__APPLE__)
fpcr_exceptions |= __fpcr_trap_invalid;
#endif
#endif
}

// Division by zero.
if (common_floatExceptions_divByZero.Get())
{
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#if defined(__USE_GNU) || defined(__FreeBSD__) || defined(__APPLE__)
exceptions |= FE_DIVBYZERO;
#elif defined(_WIN32)
exceptions |= _EM_ZERODIVIDE;
#endif

#if defined(DAEMON_USE_ARCH_INTRINSICS_i686_sse)
mxcsr_exceptions |= _MM_MASK_DIV_ZERO;
#endif
#elif defined(DAEMON_ARCH_arm64)
#if defined(__APPLE__)
fpcr_exceptions |= __fpcr_trap_divbyzero;
#endif
#endif
}

// Operations producing an overflow.
if (common_floatExceptions_overflow.Get())
{
#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#if defined(__USE_GNU) || defined(__FreeBSD__) || defined(__APPLE__)
exceptions |= FE_OVERFLOW;
#elif defined(_WIN32)
exceptions |= _EM_OVERFLOW;
#endif

#if defined(DAEMON_USE_ARCH_INTRINSICS_i686_sse)
mxcsr_exceptions |= _MM_MASK_OVERFLOW;
#endif
#elif defined(DAEMON_ARCH_arm64)
#if defined(__APPLE__)
fpcr_exceptions |= __fpcr_trap_overflow;
#endif
#endif
}

#if defined(DAEMON_ARCH_amd64) || defined(DAEMON_ARCH_i686)
#if defined(__USE_GNU) || defined(__FreeBSD__)
// https://www.gnu.org/savannah-checkouts/gnu/libc/manual/html_node/Control-Functions.html
feenableexcept(exceptions);
#elif defined(_WIN32)
// https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2012/c9676k6h(v=vs.110)
unsigned int current = 0;
_controlfp_s(&current, ~exceptions, _MCW_EM);
#endif
#endif

#if defined(__APPLE__)
// https://stackoverflow.com/a/15302624
// https://stackoverflow.com/a/71792418
fenv_t env;
fegetenv( &env );

#if defined(DAEMON_ARCH_amd64)
env.__control &= ~exceptions;
env.__mxcsr &= ~mxcsr_exceptions;
#elif defined(DAEMON_ARCH_arm64)
env.__fpcr |= fpcr_exceptions;
#endif

fesetenv( &env );

/* The signal is on SIGFPE on amd64 and on SIGILL on arm64.
We already have some sigaction() in src/common/System.cpp. */
#endif

// Superfluous on some systems, but always safe to do.
#if defined(DAEMON_USE_ARCH_INTRINSICS_i686_sse)
_MM_SET_EXCEPTION_MASK(_MM_GET_EXCEPTION_MASK() & ~mxcsr_exceptions);
#endif
#endif
}

// Common code for fatal and non-fatal exit
// TODO: Handle shutdown requests coming from multiple threads (could happen from the *nix signal thread)
static void Shutdown(bool error, Str::StringRef message)
Expand Down Expand Up @@ -747,6 +892,8 @@ static void Init(int argc, char** argv)
// Set cvars set from the command line having the Cvar::INIT flag
SetCvarsWithInitFlag(cmdlineArgs);

SetFloatingPointExceptions();

// Initialize the filesystem. For pakpaths, the libpath is added first and has the
// lowest priority, while the homepath is added last and has the highest.
cmdlineArgs.pakPaths.insert(cmdlineArgs.pakPaths.begin(), FS::Path::Build(cmdlineArgs.libPath, "pkg"));
Expand Down
10 changes: 8 additions & 2 deletions src/engine/qcommon/q_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,9 +699,15 @@ inline vec_t VectorNormalize( vec3_t v )
// that length != 0, nor does it return length
inline void VectorNormalizeFast( vec3_t v )
{
vec_t ilength = Q_rsqrt_fast( DotProduct( v, v ) );
vec_t length = DotProduct( v, v );

VectorScale( v, ilength, v );
#if DAEMON_USE_FLOAT_EXCEPTIONS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The program should not change its behavior like that when the debugging thing is used. That nullifies the whole point of it.

Copy link
Member Author

@illwieckz illwieckz Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this behavior nullifies the whole point of it.

Copy link
Member Author

@illwieckz illwieckz Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of it is:

  • Making possible to debug everything that is not Q_rsqrt_fast() as called by VectorNormalizeFast().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of it is:

* Making possible to debug everything that is not `Q_rsqrt_fast()` as called by `VectorNormalizeFast()`.

Why? There should be some explanation for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is that it can be fine to purposely ignore one specific hack, instead of making impossible to debug anything else because of one hack.

Anyway, this code raises other questions:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, answers were:

I am not a Quake 3 developer but I will give my best guess.

  • Were Quake 3 developers correct to consider it was acceptable to get garbage when the length is zero?

Yes for VectorNormalizeFast, no for VectorNormalize.

  • If they were right to consider it acceptable to get that garbage, is it still correct to get NaN instead of that garbage?

Yes

So if we consider it OK to get NaN there when length is 0, we better make the NaN detector purposely ignore this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to see some code comment about why this happens.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to see for myself and apparently some models trigger it in R_TBNtoQtangents.

if ( length )
#endif
{
vec_t ilength = Q_rsqrt_fast( length );
VectorScale( v, ilength, v );
}
}

inline vec_t VectorNormalize2( const vec3_t v, vec3_t out )
Expand Down