Pushing/popping the FMA state on clang to avoid accidentally turning on FMA for client code (#678)

* Also added unit test

Build/Android/build.gradle

@@ -5,7 +5,7 @@ buildscript {
         mavenCentral()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:8.0.1'
+        classpath 'com.android.tools.build:gradle:8.1.0'
 
         // NOTE: Do not place your application dependencies here; they belong
         // in the individual module build.gradle files
@@ -14,4 +14,4 @@ buildscript {
 
 task clean(type: Delete) {
     delete rootProject.buildDir
-}
+}

Jolt/Core/Core.h

@@ -306,12 +306,12 @@
 // OS-specific includes
 #if defined(JPH_PLATFORM_WINDOWS)
 	#define JPH_BREAKPOINT		__debugbreak()
-#elif defined(JPH_PLATFORM_BLUE) 
+#elif defined(JPH_PLATFORM_BLUE)
 	// Configuration for a popular game console.
-	// This file is not distributed because it would violate an NDA. 
-	// Creating one should only be a couple of minutes of work if you have the documentation for the platform 
+	// This file is not distributed because it would violate an NDA.
+	// Creating one should only be a couple of minutes of work if you have the documentation for the platform
 	// (you only need to define JPH_BREAKPOINT, JPH_PLATFORM_BLUE_GET_TICKS and JPH_PLATFORM_BLUE_GET_TICK_FREQUENCY and include the right header).
-	#include <Jolt/Core/PlatformBlue.h> 
+	#include <Jolt/Core/PlatformBlue.h>
 #elif defined(JPH_PLATFORM_LINUX) || defined(JPH_PLATFORM_ANDROID) || defined(JPH_PLATFORM_MACOS) || defined(JPH_PLATFORM_IOS)
 	#if defined(JPH_CPU_X86)
 		#define JPH_BREAKPOINT		__asm volatile ("int $0x3")
@@ -447,7 +447,7 @@ static_assert(sizeof(void *) == (JPH_CPU_ADDRESS_BITS == 64? 8 : 4), "Invalid si
 
 // Stack allocation
 #define JPH_STACK_ALLOC(n)		alloca(n)
-	
+
 // Shorthand for #ifdef _DEBUG / #endif
 #ifdef _DEBUG
 	#define JPH_IF_DEBUG(...)	__VA_ARGS__
@@ -493,12 +493,18 @@ static_assert(sizeof(void *) == (JPH_CPU_ADDRESS_BITS == 64? 8 : 4), "Invalid si
 	#define JPH_PRECISE_MATH_ON
 	#define JPH_PRECISE_MATH_OFF
 #elif defined(JPH_COMPILER_CLANG)
-	// We compile without -ffast-math because it cannot be turned off for a single compilation unit
-	// On clang 14 and later we can turn off float contraction through a pragma, so if FMA is on we can disable it through this macro
-	#if __clang_major__ >= 14 && defined(JPH_USE_FMADD)
-		#define JPH_PRECISE_MATH_ON					\
+	// We compile without -ffast-math because pragma float_control(precise, on) doesn't seem to actually negate all of the -ffast-math effects and causes the unit tests to fail (even if the pragma is added to all files)
+	// On clang 14 and later we can turn off float contraction through a pragma (before it was buggy), so if FMA is on we can disable it through this macro
+	#if (defined(JPH_CPU_ARM) && __clang_major__ >= 16) || (defined(JPH_CPU_X86) && __clang_major__ >= 14)
+		#define JPH_PRECISE_MATH_ON						\
+			_Pragma("float_control(precise, on, push)")	\
+			_Pragma("clang fp contract(off)")
+		#define JPH_PRECISE_MATH_OFF					\
+			_Pragma("float_control(pop)")
+	#elif __clang_major__ >= 14 && (defined(JPH_USE_FMADD) || defined(FP_FAST_FMA))
+		#define JPH_PRECISE_MATH_ON						\
 			_Pragma("clang fp contract(off)")
-		#define JPH_PRECISE_MATH_OFF				\
+		#define JPH_PRECISE_MATH_OFF					\
 			_Pragma("clang fp contract(on)")
 	#else
 		#define JPH_PRECISE_MATH_ON
@@ -506,11 +512,11 @@ static_assert(sizeof(void *) == (JPH_CPU_ADDRESS_BITS == 64? 8 : 4), "Invalid si
 	#endif
 #elif defined(JPH_COMPILER_MSVC)
 	// Unfortunately there is no way to push the state of fp_contract, so we have to assume it was turned on before JPH_PRECISE_MATH_ON
-	#define JPH_PRECISE_MATH_ON						\
-		__pragma(float_control(precise, on, push))	\
+	#define JPH_PRECISE_MATH_ON							\
+		__pragma(float_control(precise, on, push))		\
 		__pragma(fp_contract(off))
-	#define JPH_PRECISE_MATH_OFF					\
-		__pragma(fp_contract(on))					\
+	#define JPH_PRECISE_MATH_OFF						\
+		__pragma(fp_contract(on))						\
 		__pragma(float_control(pop))
 #else
 	#error Undefined

UnitTests/Core/PreciseMathTest.cpp

@@ -0,0 +1,29 @@
+// Jolt Physics Library (https://github.com/jrouwe/JoltPhysics)
+// SPDX-FileCopyrightText: 2023 Jorrit Rouwe
+// SPDX-License-Identifier: MIT
+
+#include "UnitTestFramework.h"
+#include <atomic>
+
+// Implemented as a global atomic so the compiler can't optimize it to a constant
+extern atomic<float> One, OneTenth, Ten;
+atomic<float> One = 1.0f, OneTenth = 0.1f /* Actually: 0.100000001f */, Ten = 10.0f;
+
+JPH_PRECISE_MATH_ON
+
+TEST_SUITE("PreciseMathTest")
+{
+	TEST_CASE("CheckNoFMA")
+	{
+		// Should not use the FMA instruction and end up being zero
+		// If FMA is active, a * b will not be rounded to 1.0f so the result will be a small positive number
+		float a = OneTenth, b = Ten, c = One;
+		float result = a * b - c;
+		CHECK(result == 0.0f);
+
+		// Compiler should not optimize these variables away
+		One = OneTenth = Ten = 2.0f;
+	}
+}
+
+JPH_PRECISE_MATH_OFF

UnitTests/UnitTests.cmake

@@ -7,6 +7,7 @@ set(UNIT_TESTS_SRC_FILES
 	${UNIT_TESTS_ROOT}/Core/InsertionSortTest.cpp
 	${UNIT_TESTS_ROOT}/Core/JobSystemTest.cpp
 	${UNIT_TESTS_ROOT}/Core/LinearCurveTest.cpp
+	${UNIT_TESTS_ROOT}/Core/PreciseMathTest.cpp
 	${UNIT_TESTS_ROOT}/Core/StringToolsTest.cpp
 	${UNIT_TESTS_ROOT}/Core/QuickSortTest.cpp
 	${UNIT_TESTS_ROOT}/doctest.h