[libcxx-commits] [libcxx] e7c63c0 - [libc++] Stop using __builtin_assume in _LIBCPP_ASSERT

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2023-06-29T16:59:29-04:00
New Revision: e7c63c0e90a952f6e6fdd40f9d27b2e8338dd4ac

URL: https://github.com/llvm/llvm-project/commit/e7c63c0e90a952f6e6fdd40f9d27b2e8338dd4ac
DIFF: https://github.com/llvm/llvm-project/commit/e7c63c0e90a952f6e6fdd40f9d27b2e8338dd4ac.diff

LOG: [libc++] Stop using __builtin_assume in _LIBCPP_ASSERT

__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.

Differential Revision: https://reviews.llvm.org/D153968

Added: 
    

Modified: 
    libcxx/include/__assert
    libcxx/src/filesystem/posix_compat.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__assert b/libcxx/include/__assert
index cb712e3d89823e..2ba28b5aad585f 100644
--- a/libcxx/include/__assert
+++ b/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)

diff  --git a/libcxx/src/filesystem/posix_compat.h b/libcxx/src/filesystem/posix_compat.h
index 58bfa024308931..84356169e5058b 100644
--- a/libcxx/src/filesystem/posix_compat.h
+++ b/libcxx/src/filesystem/posix_compat.h
@@ -383,7 +383,7 @@ inline int statvfs(const wchar_t *p, StatVFS *buf) {
 
 inline wchar_t *getcwd(wchar_t *buff, size_t size) { return _wgetcwd(buff, size); }
 
-inline wchar_t *realpath(const wchar_t *path, wchar_t *resolved_name) {
+inline wchar_t *realpath(const wchar_t *path, [[maybe_unused]] wchar_t *resolved_name) {
   // Only expected to be used with us allocating the buffer.
   _LIBCPP_ASSERT_UNCATEGORIZED(resolved_name == nullptr,
                  "Windows realpath() assumes a null resolved_name");