[libc-commits] [libc] [libc] Fix LIBC_LIKELY/LIBC_UNLIKELY and add LIBC_NOINLINE. (PR #230505)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: lntue

<details>
<summary>Changes</summary>

`__builtin_expect(value, expected)` requires the second argument to be a constant expression.  Previously, `LIBC_LIKELY` and `LIBC_UNLIKELY` wrapped `__builtin_expect(value, expected)` inside `details::expects_bool_condition(T value, T expected)`, where `expected` was a function parameter rather than a constant expression. So the hints were silently dropped by the compilers.

Also add LIBC_NOINLINE macro to `attributes.h` to outlining slow paths.

---
Full diff: https://github.com/llvm/llvm-project/pull/230505.diff


2 Files Affected:

- (modified) libc/src/__support/macros/attributes.h (+8) 
- (modified) libc/src/__support/macros/optimization.h (+11-4) 


``````````diff
diff --git a/libc/src/__support/macros/attributes.h b/libc/src/__support/macros/attributes.h
index 1e8ebc13fea9a4..e251f917d9850b 100644
--- a/libc/src/__support/macros/attributes.h
+++ b/libc/src/__support/macros/attributes.h
@@ -37,6 +37,14 @@
 #define LIBC_ALWAYS_INLINE LIBC_INLINE
 #endif // __has_attribute(always_inline)
 
+#if __has_attribute(noinline)
+#define LIBC_NOINLINE __attribute__((noinline))
+#elif defined(LIBC_COMPILER_IS_MSVC) || defined(_MSC_VER)
+#define LIBC_NOINLINE __declspec(noinline)
+#else
+#define LIBC_NOINLINE
+#endif // __has_attribute(noinline)
+
 #ifndef LIBC_HAS_BUILTIN_IS_CONSTANT_EVALUATED
 #if (defined(LIBC_COMPILER_IS_GCC) && (LIBC_COMPILER_GCC_VER >= 900)) ||       \
     (defined(LIBC_COMPILER_IS_CLANG) && LIBC_COMPILER_CLANG_VER >= 900)
diff --git a/libc/src/__support/macros/optimization.h b/libc/src/__support/macros/optimization.h
index 51091b6d464043..fa62887a250c50 100644
--- a/libc/src/__support/macros/optimization.h
+++ b/libc/src/__support/macros/optimization.h
@@ -16,17 +16,24 @@
 
 // We use a template to implement likely/unlikely to make sure that we don't
 // accidentally pass an integer.
+// Note that `__builtin_expect` requires the second argument to be a constant
+// expression.
 namespace LIBC_NAMESPACE_DECL {
 namespace details {
 template <typename T>
-LIBC_INLINE constexpr bool expects_bool_condition(T value, T expected) {
-  return __builtin_expect(value, expected);
+LIBC_ALWAYS_INLINE constexpr bool expects_bool_condition(T value) = delete;
+
+template <>
+LIBC_ALWAYS_INLINE constexpr bool expects_bool_condition<bool>(bool value) {
+  return value;
 }
 } // namespace details
 } // namespace LIBC_NAMESPACE_DECL
-#define LIBC_LIKELY(x) LIBC_NAMESPACE::details::expects_bool_condition(x, true)
+#define LIBC_LIKELY(x)                                                         \
+  (__builtin_expect(LIBC_NAMESPACE::details::expects_bool_condition((x)), true))
 #define LIBC_UNLIKELY(x)                                                       \
-  LIBC_NAMESPACE::details::expects_bool_condition(x, false)
+  (__builtin_expect(LIBC_NAMESPACE::details::expects_bool_condition((x)),      \
+                    false))
 
 #if defined(LIBC_COMPILER_IS_CLANG)
 #define LIBC_LOOP_NOUNROLL _Pragma("nounroll")

``````````

</details>


https://github.com/llvm/llvm-project/pull/230505