[libcxx-commits] [PATCH] D153968: [libc++] Stop using __builtin_assume in _LIBCPP_ASSERT

[Louis Dionne via Phabricator via libcxx-commits]

ldionne created this revision.
Herald added a project: All.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

__builtin_assume can sometimes worsen code generation. For now, the
guideline seems to be to avoid adding assumptions without a clear
optimization intent. Since _LIBCPP_ASSERT is very general, we can't
have a clear optimization intent at this level, which makes
__builtin_assume the wrong tool for the job -- at least until
__builtin_assume is changed.

See https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609
for a discussion of this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153968

Files:
  libcxx/include/__assert


Index: libcxx/include/__assert
===================================================================
--- libcxx/include/__assert
+++ libcxx/include/__assert
@@ -38,7 +38,10 @@
          ? (void)0                                                                                                     \
          : _LIBCPP_VERBOSE_ABORT(                                                                                      \
                "%s:%d: assertion %s failed: %s\n", __builtin_FILE(), __builtin_LINE(), #expression, message))
-#elif !defined(_LIBCPP_ASSERTIONS_DISABLE_ASSUME) && __has_builtin(__builtin_assume)
+// TODO: __builtin_assume can currently inhibit optimizations. Until this has been fixed and we can add
+//       assumptions without a clear optimization intent, disable that to avoid worsening the code generation.
+//       See https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 for a discussion.
+#elif 0 && __has_builtin(__builtin_assume)
 #  define _LIBCPP_ASSERT(expression, message)                                                                          \
     (_LIBCPP_DIAGNOSTIC_PUSH _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wassume")                                              \
          __builtin_assume(static_cast<bool>(expression)) _LIBCPP_DIAGNOSTIC_POP)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153968.535364.patch
Type: text/x-patch
Size: 1301 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230628/5b3dc791/attachment-0001.bin>