commit ef976b28459f55b5d916d97d0b43b1ec6341d258
author Corentin Wallez <cwallez@chromium.org> Wed Mar 05 09:58:18 2025 -0800
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 05 09:58:18 2025 -0800
tree 0e273573fa433f928466fbd497d731c4f821947b
parent a2ad6b01f4a1032deaadda1737e1fbfceaf6aa0f

[dawn] Fix detection of __builtin_assume for DAWN_ASSERT

The proper way to detect the presence of the builtin is using
__has_builtin. This triggers a warning that the assume is a noop
if the ASSERT contains side effects. It is suppressed because we
want to call functions in ASSERT, and still turn most of them into
__builtin_assume in Release.

Fixed: 394823026
Change-Id: I60f6526a0c9e8019b2b83afe2d48ea43713ac13f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/227794
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/cmake/DawnCompilerExtraFlags.cmake

diff --git a/src/cmake/DawnCompilerExtraFlags.cmake b/src/cmake/DawnCompilerExtraFlags.cmake
index 8133040..9fa5ed7 100644
--- a/src/cmake/DawnCompilerExtraFlags.cmake
+++ b/src/cmake/DawnCompilerExtraFlags.cmake
@@ -40,6 +40,7 @@
         "-fno-exceptions"
         "-fno-rtti"
 
+        "-Wno-assume"
         "-Wno-deprecated-builtins"
         "-Wno-unknown-warning-option"
         "-Wno-switch-default"

src/dawn/common/Assert.h

diff --git a/src/dawn/common/Assert.h b/src/dawn/common/Assert.h
index 1182883..d324e32 100644
--- a/src/dawn/common/Assert.h
+++ b/src/dawn/common/Assert.h
@@ -83,8 +83,8 @@
 
 #if DAWN_COMPILER_IS(MSVC)
 #define DAWN_ASSERT_CALLSITE_HELPER(file, func, line, condition) __assume(condition)
-#elif DAWN_COMPILER_IS(CLANG) && defined(__builtin_assume)
-#define DAWN_ASSERT_CALLSITE_HELPER(file, func, line, condition) __builtin_assume(condition)
+#elif DAWN_COMPILER_IS(CLANG) && __has_builtin(__builtin_assume)
+#define DAWN_ASSERT_CALLSITE_HELPER(file, func, line, condition) __builtin_assume(!!(condition))
 #else  // DAWN_COMPILER_IS(*)
 #define DAWN_ASSERT_CALLSITE_HELPER(file, func, line, condition) \
     do {                                                         \

src/dawn/common/BUILD.gn

diff --git a/src/dawn/common/BUILD.gn b/src/dawn/common/BUILD.gn
index 64ab821..38833e3 100644
--- a/src/dawn/common/BUILD.gn
+++ b/src/dawn/common/BUILD.gn
@@ -160,6 +160,9 @@
       "-Wunused-macros",
     ]
 
+    # We use __builtin_assume liberally in DAWN_ASSERT, allow ignoring side-effects there.
+    cflags += [ "-Wno-assume" ]
+
     if (is_win) {
       cflags += [
         # clang-cl doesn't know -pedantic, pass it explicitly to the clang driver

src/dawn/platform/DawnPlatform.cpp

diff --git a/src/dawn/platform/DawnPlatform.cpp b/src/dawn/platform/DawnPlatform.cpp
index 481ff87..cab1a11 100644
--- a/src/dawn/platform/DawnPlatform.cpp
+++ b/src/dawn/platform/DawnPlatform.cpp
@@ -62,8 +62,7 @@
                                  const uint64_t* argValues,
                                  unsigned char flags) {
     // AddTraceEvent cannot be called if events are disabled.
-    DAWN_ASSERT(false);
-    return 0;
+    DAWN_UNREACHABLE();
 }
 
 void Platform::HistogramCustomCounts(const char* name,