From 5df6391246bbd4ba3c695dbe1801739613efe4f7 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Thu, 25 Apr 2024 16:33:47 -0500 Subject: [PATCH] Add support for builtin_expect compiler hint (#4425) * Add support for __builtin_expect extension And H5_LIKELY / H5_UNLIKELY macros to wrap it Signed-off-by: Quincey Koziol * Committing clang-format changes --------- Signed-off-by: Quincey Koziol Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- config/cmake/ConfigureChecks.cmake | 1 + config/cmake/H5pubconf.h.in | 3 +++ config/cmake/HDFTests.c | 15 +++++++++++++++ configure.ac | 8 ++++++++ src/H5private.h | 31 +++++++++++++++--------------- 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/config/cmake/ConfigureChecks.cmake b/config/cmake/ConfigureChecks.cmake index 15d96f607fb..56e91cddc93 100644 --- a/config/cmake/ConfigureChecks.cmake +++ b/config/cmake/ConfigureChecks.cmake @@ -434,6 +434,7 @@ endif () if (MINGW OR NOT WINDOWS) foreach (other_test HAVE_ATTRIBUTE + HAVE_BUILTIN_EXPECT SYSTEM_SCOPE_THREADS HAVE_SOCKLEN_T ) diff --git a/config/cmake/H5pubconf.h.in b/config/cmake/H5pubconf.h.in index 586032bf269..af80d0b545b 100644 --- a/config/cmake/H5pubconf.h.in +++ b/config/cmake/H5pubconf.h.in @@ -328,6 +328,9 @@ /* Define to 1 if you have the header file. */ #cmakedefine H5_HAVE_SZLIB_H @H5_HAVE_SZLIB_H@ +/* Define to 1 if the compiler supports the __builtin_expect() extension */ +#cmakedefine H5_HAVE_BUILTIN_EXPECT @H5_HAVE_BUILTIN_EXPECT@ + #if defined(_WIN32) && !defined(H5_BUILT_AS_DYNAMIC_LIB) /* Not supported on WIN32 platforms with static linking */ /* #undef H5_HAVE_THREADSAFE */ diff --git a/config/cmake/HDFTests.c b/config/cmake/HDFTests.c index 0f0600abd81..8d0e78f46b7 100644 --- a/config/cmake/HDFTests.c +++ b/config/cmake/HDFTests.c @@ -15,6 +15,21 @@ #define SIMPLE_TEST(x) int main(void){ x; return 0; } +#ifdef HAVE_BUILTIN_EXPECT + +int +main () +{ + void *ptr = (void*) 0; + + if (__builtin_expect (ptr != (void*) 0, 1)) + return 0; + + return 0; +} + +#endif /* HAVE_BUILTIN_EXPECT */ + #ifdef HAVE_ATTRIBUTE int diff --git a/configure.ac b/configure.ac index b22c8ea21a4..394c05959ca 100644 --- a/configure.ac +++ b/configure.ac @@ -2417,6 +2417,14 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[int __attribute__((unused)) x]])], AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no])]) +AC_MSG_CHECKING([if compiler supports the __builtin_expect() extension]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[void *ptr = (void*) 0; + if (__builtin_expect (ptr != (void*) 0, 1)) return 0;]])], + [AC_DEFINE([HAVE_BUILTIN_EXPECT], [1], + [Define if supports __builtin_expect() extension]) + AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no])]) + ## ---------------------------------------------------------------------- ## Remove old ways of determining debug/production build. ## These were used in 1.8.x and earlier. We should probably keep these checks diff --git a/src/H5private.h b/src/H5private.h index 8e4330f5b3c..148ac884072 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -160,13 +160,15 @@ #include "uthash.h" /* - * NT doesn't define SIGBUS, but since NT only runs on processors - * that do not have alignment constraints a SIGBUS would never be - * raised, so we just replace it with SIGILL (which also should - * never be raised by the hdf5 library). + * Does the compiler support the __builtin_expect() syntax? + * It's not a problem if not. */ -#ifndef SIGBUS -#define SIGBUS SIGILL +#if H5_HAVE_BUILTIN_EXPECT +#define H5_LIKELY(expression) __builtin_expect(!!(expression), 1) +#define H5_UNLIKELY(expression) __builtin_expect(!!(expression), 0) +#else +#define H5_LIKELY(expression) (expression) +#define H5_UNLIKELY(expression) (expression) #endif /* @@ -1217,7 +1219,7 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); { \ static bool func_check = false; \ \ - if (!func_check) { \ + if (H5_UNLIKELY(!func_check)) { \ /* Check function naming status */ \ assert(asrt && \ "Function naming conventions are incorrect - check H5_IS_API|PUB|PRIV|PKG macros in " \ @@ -1253,8 +1255,8 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); #define FUNC_ENTER_API_INIT(err) \ /* Initialize the library */ \ - if (!H5_INIT_GLOBAL && !H5_TERM_GLOBAL) { \ - if (H5_init_library() < 0) \ + if (H5_UNLIKELY(!H5_INIT_GLOBAL && !H5_TERM_GLOBAL)) { \ + if (H5_UNLIKELY(H5_init_library() < 0)) \ HGOTO_ERROR(H5E_FUNC, H5E_CANTINIT, err, "library initialization failed"); \ } @@ -1263,7 +1265,7 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); H5_PUSH_FUNC \ \ /* Push the API context */ \ - if (H5CX_push() < 0) \ + if (H5_UNLIKELY(H5CX_push() < 0)) \ HGOTO_ERROR(H5E_FUNC, H5E_CANTSET, err, "can't set API context"); \ else \ api_ctx_pushed = true; @@ -1412,7 +1414,6 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); #define FUNC_ENTER_NOAPI_NOFS \ { \ FUNC_ENTER_COMMON(!H5_IS_API(__func__)); \ - \ { /* @@ -1517,12 +1518,12 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); #define FUNC_LEAVE_API(ret_value) \ ; \ } /*end scope from end of FUNC_ENTER*/ \ - if (api_ctx_pushed) { \ + if (H5_LIKELY(api_ctx_pushed)) { \ (void)H5CX_pop(true); \ api_ctx_pushed = false; \ } \ H5_POP_FUNC \ - if (err_occurred) \ + if (H5_UNLIKELY(err_occurred)) \ (void)H5E_dump_api_stack(); \ FUNC_LEAVE_API_THREADSAFE \ return (ret_value); \ @@ -1534,7 +1535,7 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); ; \ } /*end scope from end of FUNC_ENTER*/ \ H5_POP_FUNC \ - if (err_occurred) \ + if (H5_UNLIKELY(err_occurred)) \ (void)H5E_dump_api_stack(); \ FUNC_LEAVE_API_THREADSAFE \ return (ret_value); \ @@ -1557,7 +1558,7 @@ H5_DLL herr_t H5CX_pop(bool update_dxpl_props); #define FUNC_LEAVE_API_NOPUSH(ret_value) \ ; \ } /*end scope from end of FUNC_ENTER*/ \ - if (err_occurred) \ + if (H5_UNLIKELY(err_occurred)) \ (void)H5E_dump_api_stack(); \ FUNC_LEAVE_API_THREADSAFE \ return (ret_value); \