From 0b98428ba666c761b4b86d59d36d592f93e37028 Mon Sep 17 00:00:00 2001
From: Bryan Bernhart <bryan.bernhart@intel.com>
Date: Thu, 25 Aug 2022 11:32:26 -0700
Subject: [PATCH] Fix MSVC assert code generation on for release builds.

ASSERT(expr) would get optimized out on release builds, even if gpgmm_always_assert was true. This fix changes the ASSERT generated code to always evaluate the condition first and hint failure is unreachable rather then always assume true.
---
 src/gpgmm/utils/Assert.h   | 20 +++++++++++++++-----
 src/gpgmm/utils/Compiler.h |  8 ++++----
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/gpgmm/utils/Assert.h b/src/gpgmm/utils/Assert.h
index 5c421c8e8..933689be8 100644
--- a/src/gpgmm/utils/Assert.h
+++ b/src/gpgmm/utils/Assert.h
@@ -48,7 +48,16 @@
         } while (GPGMM_ASSERT_LOOP_CONDITION)
 #else
 #    if defined(GPGMM_COMPILER_MSVC)
-#        define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) __assume(condition)
+// Avoid calling __assume(condition) directly because we can't assume the |condition| will always
+// evaluate to be true at runtime and when false, the condition (or code) could be optimized out by
+// MSVC and never executed. To protect the ASSERT's code, the equivelent generated code using
+// __assume(0) is used.
+#        define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) \
+            do {                                                          \
+                if (!(condition)) {                                       \
+                    GPGMM_UNREACHABLE();                                  \
+                }                                                         \
+            } while (GPGMM_ASSERT_LOOP_CONDITION)
 #    elif defined(GPGMM_COMPILER_CLANG) && GPGMM_HAS_BUILTIN(__builtin_unreachable)
 // Avoid using __builtin_assume since it results into clang assuming _every_ function call has a
 // side effect. Alternatively, suppress these warnings with -Wno-assume or wrap _builtin_assume in
@@ -57,7 +66,7 @@
 #        define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) \
             do {                                                          \
                 if (!(condition)) {                                       \
-                    GPGMM_BUILTIN_UNREACHABLE();                          \
+                    GPGMM_UNREACHABLE();                                  \
                 }                                                         \
             } while (GPGMM_ASSERT_LOOP_CONDITION)
 #    else
@@ -70,10 +79,11 @@
 
 #define GPGMM_ASSERT(condition) \
     GPGMM_ASSERT_CALLSITE_HELPER(__FILE__, __func__, __LINE__, condition)
-#define GPGMM_UNREACHABLE()                                                  \
+
+#define GPGMM_ASSERT_UNREACHABLE()                                           \
     do {                                                                     \
         GPGMM_ASSERT(GPGMM_ASSERT_LOOP_CONDITION && "Unreachable code hit"); \
-        GPGMM_BUILTIN_UNREACHABLE();                                         \
+        GPGMM_UNREACHABLE();                                                 \
     } while (GPGMM_ASSERT_LOOP_CONDITION)
 
 // Disable short-hand defined macros due to possible name clash.
@@ -83,7 +93,7 @@
 #endif
 
 #if !defined(UNREACHABLE)
-#    define UNREACHABLE GPGMM_UNREACHABLE
+#    define UNREACHABLE GPGMM_ASSERT_UNREACHABLE
 #endif
 
 namespace gpgmm {
diff --git a/src/gpgmm/utils/Compiler.h b/src/gpgmm/utils/Compiler.h
index 65468ac49..57a581bdc 100644
--- a/src/gpgmm/utils/Compiler.h
+++ b/src/gpgmm/utils/Compiler.h
@@ -19,7 +19,7 @@
 // Defines macros for compiler-specific functionality
 //  - GPGMM_COMPILER_[CLANG|GCC|MSVC]: Compiler detection
 //  - GPGMM_BREAKPOINT(): Raises an exception and breaks in the debugger
-//  - GPGMM_BUILTIN_UNREACHABLE(): Hints the compiler that a code path is unreachable
+//  - GPGMM_UNREACHABLE(): Hints the compiler that a code path is unreachable
 //  - GPGMM_NO_DISCARD: An attribute that is C++17 [[nodiscard]] where available
 //  - GPGMM_(UN)?LIKELY(EXPR): Where available, hints the compiler that the expression will be true
 //      (resp. false) to help it generate code that leads to better branch prediction.
@@ -44,7 +44,7 @@
 #        define GPGMM_BREAKPOINT()
 #    endif
 
-#    define GPGMM_BUILTIN_UNREACHABLE() __builtin_unreachable()
+#    define GPGMM_UNREACHABLE() __builtin_unreachable()
 #    define GPGMM_LIKELY(x) __builtin_expect(!!(x), 1)
 #    define GPGMM_UNLIKELY(x) __builtin_expect(!!(x), 0)
 
@@ -64,10 +64,10 @@
 extern void __cdecl __debugbreak(void);
 #    define GPGMM_BREAKPOINT() __debugbreak()
 
-#    define GPGMM_BUILTIN_UNREACHABLE() __assume(false)
+#    define GPGMM_UNREACHABLE() __assume(false)
 
 // Visual Studio 2017 15.3 adds support for [[nodiscard]]
-#    if _MSC_VER >= 1911 && DAWN_CPP_VERSION >= 17
+#    if _MSC_VER >= 1911 && GPGMM_CPP_VERSION >= 17
 #        define GPGMM_NO_DISCARD [[nodiscard]]
 #    endif