From afe01108d1aed62ce4074a734ea76600c8f9c5a4 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Mon, 26 Feb 2018 02:25:46 -0500 Subject: [PATCH] ck_cc: add a disable builtin flag for the FreeBSD kernel. This primarily affects the FreeBSD kernel, where the popcount builtin can be problematic (relies on compiler-provided libraries). See the history of __POPCNT__ for details [1]. - A new flag, CK_MD_CC_BUILTIN_DISABLE, can be set to indicate that CK should not rely on compiler builtins when possible. - ck_cc_clz has been removed, it was unused. - ck_internal_bsf has been removed, it was duplicate of ck_cc_ffs but broken, replaced in favor of ck_cc_ffs. Previous consumers were using the bsf instruction, eitherway. - ck_{rhs,hs,ht} have been updated to use ck_cc_ffs*. If FreeBSD requires the builtins for performance reasons, we will lift the appropriate detection into ck_md (at least, bt* bs* family of functions don't have the same problems on most targets unlike popcount). 1: https://lists.freebsd.org/pipermail/svn-src-head/2015-March/069663.html --- include/ck_cc.h | 55 +++++++++++++----------------- include/freebsd/ck_md.h.in | 9 +++-- include/gcc/ck_cc.h | 19 +++++------ regressions/ck_cc/validate/ck_cc.c | 29 +++++++++++++--- src/ck_hs.c | 2 +- src/ck_ht.c | 2 +- src/ck_internal.h | 37 -------------------- src/ck_rhs.c | 2 +- 8 files changed, 67 insertions(+), 88 deletions(-) diff --git a/include/ck_cc.h b/include/ck_cc.h index e17dc7b..9a152a3 100644 --- a/include/ck_cc.h +++ b/include/ck_cc.h @@ -104,41 +104,35 @@ #define CK_CC_TYPEOF(X, DEFAULT) (DEFAULT) #endif -#ifndef CK_F_CC_FFS -#define CK_F_CC_FFS -CK_CC_INLINE static int -ck_cc_ffs(unsigned int x) -{ - unsigned int i; - - if (x == 0) - return 0; - - for (i = 1; (x & 1) == 0; i++, x >>= 1); - - return i; +#define CK_F_CC_FFS_G(L, T) \ +CK_CC_INLINE static int \ +ck_cc_##L(T v) \ +{ \ + unsigned int i; \ + \ + if (v == 0) \ + return 0; \ + \ + for (i = 1; (v & 1) == 0; i++, v >>= 1); \ + return i; \ } -#endif - -#ifndef CK_F_CC_CLZ -#define CK_F_CC_CLZ -#include -CK_CC_INLINE static int -ck_cc_clz(unsigned int x) -{ - unsigned int count, i; +#ifndef CK_F_CC_FFS +#define CK_F_CC_FFS +CK_F_CC_FFS_G(ffs, unsigned int) +#endif /* CK_F_CC_FFS */ - for (count = 0, i = sizeof(unsigned int) * CHAR_BIT; i > 0; count++) { - unsigned int bit = 1U << --i; +#ifndef CK_F_CC_FFSL +#define CK_F_CC_FFSL +CK_F_CC_FFS_G(ffsl, unsigned long) +#endif /* CK_F_CC_FFSL */ - if (x & bit) - break; - } +#ifndef CK_F_CC_FFSLL +#define CK_F_CC_FFSLL +CK_F_CC_FFS_G(ffsll, unsigned long long) +#endif /* CK_F_CC_FFSLL */ - return count; -} -#endif +#undef CK_F_CC_FFS_G #ifndef CK_F_CC_CTZ #define CK_F_CC_CTZ @@ -151,7 +145,6 @@ ck_cc_ctz(unsigned int x) return 0; for (i = 0; (x & 1) == 0; i++, x >>= 1); - return i; } #endif diff --git a/include/freebsd/ck_md.h.in b/include/freebsd/ck_md.h.in index 8965e05..7fb6c64 100644 --- a/include/freebsd/ck_md.h.in +++ b/include/freebsd/ck_md.h.in @@ -50,7 +50,7 @@ #define CK_MD_CACHELINE (64) #else #define CK_MD_CACHELINE (CACHE_LINE_SIZE) -#endif +#endif /* !__amd64__ && !__i386__ */ #endif /* CK_MD_CACHELINE */ #ifndef CK_MD_PAGESIZE @@ -95,6 +95,11 @@ #define CK_MD_UMP #endif /* SMP */ +/* + * Disable the use of compiler builtin functions. + */ +#define CK_MD_CC_BUILTIN_DISABLE 1 + /* * CK expects those, which are normally defined by the build system. */ @@ -105,7 +110,7 @@ * __mbk() to avoid potential issues around false sharing. */ #define CK_MD_TSO -#define CK_MD_SSE_DISABLE 1 +#define CK_MD_SSE_DISABLE 1 #elif defined(__amd64__) #define CK_MD_TSO #elif defined(__sparc64__) && !defined(__sparcv9__) diff --git a/include/gcc/ck_cc.h b/include/gcc/ck_cc.h index a14a4b5..6ebc59c 100644 --- a/include/gcc/ck_cc.h +++ b/include/gcc/ck_cc.h @@ -103,28 +103,26 @@ #define CK_CC_TYPEOF(X, DEFAULT) __typeof__(X) /* - * Portability wrappers for bitwise ops. + * Portability wrappers for bitwise operations. */ - +#ifndef CK_MD_CC_BUILTIN_DISABLE #define CK_F_CC_FFS -#define CK_F_CC_CLZ -#define CK_F_CC_CTZ -#define CK_F_CC_POPCOUNT - CK_CC_INLINE static int ck_cc_ffs(unsigned int x) { - return __builtin_ffs(x); + return __builtin_ffsl(x); } +#define CK_F_CC_FFSL CK_CC_INLINE static int -ck_cc_clz(unsigned int x) +ck_cc_ffsl(unsigned long x) { - return __builtin_clz(x); + return __builtin_ffsll(x); } +#define CK_F_CC_CTZ CK_CC_INLINE static int ck_cc_ctz(unsigned int x) { @@ -132,11 +130,12 @@ ck_cc_ctz(unsigned int x) return __builtin_ctz(x); } +#define CK_F_CC_POPCOUNT CK_CC_INLINE static int ck_cc_popcount(unsigned int x) { return __builtin_popcount(x); } - +#endif /* CK_MD_CC_BUILTIN_DISABLE */ #endif /* CK_GCC_CC_H */ diff --git a/regressions/ck_cc/validate/ck_cc.c b/regressions/ck_cc/validate/ck_cc.c index b9035ea..a22030f 100644 --- a/regressions/ck_cc/validate/ck_cc.c +++ b/regressions/ck_cc/validate/ck_cc.c @@ -1,4 +1,5 @@ #include +#include #include #include "../../common.h" @@ -8,11 +9,29 @@ main(void) { unsigned int x; - ck_pr_store_uint(&x, 4); + ck_pr_store_uint(&x, 0x10110); + + if (ck_cc_ffs(0) != 0) + ck_error("ffs(0) = %d\n", ck_cc_ffs(0)); + if (ck_cc_ffs(4) != 3) + ck_error("ffs(4) = %d\n", ck_cc_ffs(4)); + if (ck_cc_ffs(UINT_MAX) != 1) + ck_error("ffs(UINT_MAX) = %d\n", ck_cc_ffs(UINT_MAX)); + if (ck_cc_ffs(x) != 5) + ck_error("ffs(%u) = %d\n", x, ck_cc_ffs(x)); + + if (ck_cc_ffs(x) != ck_cc_ffsl(x) || + ck_cc_ffsl(x) != ck_cc_ffsll(x) || + ck_cc_ffs(x) != ck_cc_ffsll(x)) { + ck_error(" ffs = %d, ffsl = %d, ffsll = %d\n", + ck_cc_ffs(x), ck_cc_ffsl(x), ck_cc_ffsll(x)); + } + + if (ck_cc_ctz(x) != 4) + ck_error("ctz = %d\n", ck_cc_ctz(x)); + + if (ck_cc_popcount(x) != 3) + ck_error("popcount = %d\n", ck_cc_popcount(x)); - printf(" ffs = %d\n", ck_cc_ffs(x)); - printf(" clz = %d\n", ck_cc_clz(x)); - printf(" ctz = %d\n", ck_cc_ctz(x)); - printf("popcount = %d\n", ck_cc_popcount(x)); return 0; } diff --git a/src/ck_hs.c b/src/ck_hs.c index a16da18..a7e15ea 100644 --- a/src/ck_hs.c +++ b/src/ck_hs.c @@ -223,7 +223,7 @@ ck_hs_map_create(struct ck_hs *hs, unsigned long entries) map->probe_limit = (unsigned int)limit; map->probe_maximum = 0; map->capacity = n_entries; - map->step = ck_internal_bsf(n_entries); + map->step = ck_cc_ffsl(n_entries); map->mask = n_entries - 1; map->n_entries = 0; diff --git a/src/ck_ht.c b/src/ck_ht.c index 2c864c5..48b04c9 100644 --- a/src/ck_ht.c +++ b/src/ck_ht.c @@ -171,7 +171,7 @@ ck_ht_map_create(struct ck_ht *table, CK_HT_TYPE entries) map->deletions = 0; map->probe_maximum = 0; map->capacity = n_entries; - map->step = ck_internal_bsf_64(map->capacity); + map->step = ck_cc_ffsll(map->capacity); map->mask = map->capacity - 1; map->n_entries = 0; map->entries = (struct ck_ht_entry *)(((uintptr_t)&map[1] + prefix + diff --git a/src/ck_internal.h b/src/ck_internal.h index 7aad3d7..1bca36a 100644 --- a/src/ck_internal.h +++ b/src/ck_internal.h @@ -80,40 +80,3 @@ ck_internal_max_32(uint32_t x, uint32_t y) return x ^ ((x ^ y) & -(x < y)); } - -CK_CC_INLINE static unsigned long -ck_internal_bsf(unsigned long v) -{ -#if defined(__GNUC__) - return __builtin_ffs(v); -#else - unsigned int i; - const unsigned int s = sizeof(unsigned long) * 8 - 1; - - for (i = 0; i < s; i++) { - if (v & (1UL << (s - i))) - return sizeof(unsigned long) * 8 - i; - } - - return 1; -#endif /* !__GNUC__ */ -} - -CK_CC_INLINE static uint64_t -ck_internal_bsf_64(uint64_t v) -{ -#if defined(__GNUC__) - return __builtin_ffs(v); -#else - unsigned int i; - const unsigned int s = sizeof(unsigned long) * 8 - 1; - - for (i = 0; i < s; i++) { - if (v & (1ULL << (63U - i))) - return i; - } -#endif /* !__GNUC__ */ - - return 1; -} - diff --git a/src/ck_rhs.c b/src/ck_rhs.c index f6dd2ee..1d6b0f0 100644 --- a/src/ck_rhs.c +++ b/src/ck_rhs.c @@ -366,7 +366,7 @@ ck_rhs_map_create(struct ck_rhs *hs, unsigned long entries) map->probe_limit = (unsigned int)limit; map->probe_maximum = 0; map->capacity = n_entries; - map->step = ck_internal_bsf(n_entries); + map->step = ck_cc_ffsl(n_entries); map->mask = n_entries - 1; map->n_entries = 0;