From ee312dc4d242141638ce666b056237f2e538f3e2 Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Fri, 19 Aug 2022 14:14:32 -0700 Subject: [PATCH] Fix ASSERT generation from not evaluating the condition during release builds. The ASSERT code-generator incorrectly checks if __builtin_assume exists. Once fixed, clang incorrectly assumes all functions have side effects and emits a warning. To avoid these warnings, __builtin_assume was replaced with __builtin_unreachable, since the generated code was equivelent. --- src/gpgmm/utils/Assert.h | 12 ++++++++++-- src/gpgmm/utils/Compiler.h | 8 ++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/gpgmm/utils/Assert.h b/src/gpgmm/utils/Assert.h index 466720cc0..5c421c8e8 100644 --- a/src/gpgmm/utils/Assert.h +++ b/src/gpgmm/utils/Assert.h @@ -49,9 +49,17 @@ #else # if defined(GPGMM_COMPILER_MSVC) # define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) __assume(condition) -# elif defined(GPGMM_COMPILER_CLANG) && defined(__builtin_assume) +# 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 +// pragmas. Since the generated code below is equivelent, replacing with __builtin_unreachable is +// used. # define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) \ - __builtin_assume(condition) + do { \ + if (!(condition)) { \ + GPGMM_BUILTIN_UNREACHABLE(); \ + } \ + } while (GPGMM_ASSERT_LOOP_CONDITION) # else # define GPGMM_ASSERT_CALLSITE_HELPER(file, func, line, condition) \ do { \ diff --git a/src/gpgmm/utils/Compiler.h b/src/gpgmm/utils/Compiler.h index 5dc2ff00e..65468ac49 100644 --- a/src/gpgmm/utils/Compiler.h +++ b/src/gpgmm/utils/Compiler.h @@ -80,6 +80,11 @@ extern void __cdecl __debugbreak(void); # error "Unsupported compiler" #endif +// Non-compiler specific language extensions +#if defined(__has_builtin) +# define GPGMM_HAS_BUILTIN __has_builtin +#endif + // It seems that (void) EXPR works on all compilers to silence the unused variable warning. #define GPGMM_UNUSED(EXPR) (void)EXPR // Likewise using static asserting on sizeof(&FUNC) seems to make it tagged as used @@ -98,6 +103,9 @@ extern void __cdecl __debugbreak(void); #if !defined(GPGMM_FORCE_INLINE) # define GPGMM_FORCE_INLINE inline #endif +#if !defined(GPGMM_HAS_BUILTIN) +# define GPGMM_HAS_BUILTIN(X) 0 +#endif #if defined(__clang__) # define GPGMM_FALLTHROUGH [[clang::fallthrough]]