On 04/04/17 23:08, Brian Brooks wrote: > On 04/04 22:17:48, Maxim Uvarov wrote: >> On 04/04/17 21:48, Brian Brooks wrote: >>> Signed-off-by: Ola Liljedahl <ola.liljed...@arm.com> >>> Reviewed-by: Brian Brooks <brian.bro...@arm.com> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> >>> --- >>> platform/linux-generic/include/odp_bitset.h | 155 >>> ++++++++++++++++++++++++++++ >>> 1 file changed, 155 insertions(+) >>> create mode 100644 platform/linux-generic/include/odp_bitset.h >>> >>> diff --git a/platform/linux-generic/include/odp_bitset.h >>> b/platform/linux-generic/include/odp_bitset.h >>> new file mode 100644 >>> index 00000000..db004267 >>> --- /dev/null >>> +++ b/platform/linux-generic/include/odp_bitset.h >>> @@ -0,0 +1,155 @@ >>> +/* Copyright (c) 2017, ARM Limited >>> + * All rights reserved. >>> + * >>> + * SPDX-License-Identifier: BSD-3-Clause >>> + */ >> >> >> Should be Linaro copyright and patches needs to be resend from Linaro >> address. > > Can you raise this concern through the appropriate channels? > >>> + >>> +#ifndef _ODP_BITSET_H_ >>> +#define _ODP_BITSET_H_ >>> + >>> +/****************************************************************************** >>> + * bitset abstract data type >>> + >>> *****************************************************************************/ >>> +/* This could be a struct of scalars to support larger bit sets */ >>> + >>> +#if ATOM_BITSET_SIZE <= 32 >>> + >>> +typedef uint32_t bitset_t; >>> + >>> +static inline bitset_t bitset_mask(uint32_t bit) >>> +{ >>> + return 1UL << bit; >>> +} >>> + >>> +/* Return first-bit-set with StdC ffs() semantics */ >>> +static inline uint32_t bitset_ffs(bitset_t b) >>> +{ >>> + return __builtin_ffsl(b); >>> +} >>> + >>> +/* Load-exclusive with memory ordering */ >>> +static inline bitset_t bitset_ldex(bitset_t *bs, int mo) >>> +{ >>> + return LDXR32(bs, mo); >> >> is it arm specific? > > Can you please explain? >
is macros LDXR32 arm specific? Or that can be also compiled on other arches (x86, mips, ppc)? Maxim. >>> +} >>> + >>> +#elif ATOM_BITSET_SIZE <= 64 >>> + >>> +typedef uint64_t bitset_t; >>> + >>> +static inline bitset_t bitset_mask(uint32_t bit) >>> +{ >>> + return 1ULL << bit; >>> +} >>> + >>> +/* Return first-bit-set with StdC ffs() semantics */ >>> +static inline uint32_t bitset_ffs(bitset_t b) >>> +{ >>> + return __builtin_ffsll(b); >>> +} >>> + >>> +/* Load-exclusive with memory ordering */ >>> +static inline bitset_t bitset_ldex(bitset_t *bs, int mo) >>> +{ >>> + return LDXR64(bs, mo); >>> +} >>> + >>> +#elif ATOM_BITSET_SIZE <= 128 >>> + >>> +#if __SIZEOF_INT128__ == 16 >>> +typedef unsigned __int128 bitset_t; >>> + >> >> so bitset_t can be 32 or 64 or 128. That might be difficult. And sort of >> this needs to be done at ./configure stage, I think. > > Can you please explain? > >>> +static inline bitset_t bitset_mask(uint32_t bit) >>> +{ >>> + if (bit < 64) >>> + return 1ULL << bit; >>> + else >>> + return (unsigned __int128)(1ULL << (bit - 64)) << 64; >>> +} >> >> you set 32 bit mast to 128 bit value, other 96 bits unreachable. Why is >> that needed? > > 'bit' represents a bit position in the scalar type. Unfortunately GCC > doesn't support 128-bit numeric literals, so you have to generate the > mask using 'ull' and then cast that to the scalar type and then shift > that even further. Please take another look. > >>> + >>> +/* Return first-bit-set with StdC ffs() semantics */ >>> +static inline uint32_t bitset_ffs(bitset_t b) >>> +{ >>> + if ((uint64_t)b != 0) >>> + return __builtin_ffsll((uint64_t)b); >>> + else if ((b >> 64) != 0) >>> + return __builtin_ffsll((uint64_t)(b >> 64)) + 64; >>> + else >>> + return 0; >>> +} >>> + >>> +/* Load-exclusive with memory ordering */ >>> +static inline bitset_t bitset_ldex(bitset_t *bs, int mo) >>> +{ >>> + return LDXR128(bs, mo); >>> +} >>> + >>> +#else >>> +#error __int128 not supported by compiler >> >> >> this should be captured at configure stage, not while compiling project >> code. > > When you use the macros provided by the compiler, e.g. system-specific > macros, you can eliminate the dependency on the build system.. for achieving > the same thing. This is an advantage. > >> Maxim. >> >>> +#endif >>> + >>> +#else >>> +#error Unsupported size of bit sets (ATOM_BITSET_SIZE) >>> +#endif >>> + >>> +/* Atomic load with memory ordering */ >>> +static inline bitset_t atom_bitset_load(bitset_t *bs, int mo) >>> +{ >>> + return __atomic_load_n(bs, mo); >>> +} >>> + >>> +/* Atomic bit set with memory ordering */ >>> +static inline void atom_bitset_set(bitset_t *bs, uint32_t bit, int mo) >>> +{ >>> + (void)__atomic_fetch_or(bs, bitset_mask(bit), mo); >>> +} >>> + >>> +/* Atomic bit clear with memory ordering */ >>> +static inline void atom_bitset_clr(bitset_t *bs, uint32_t bit, int mo) >>> +{ >>> + (void)__atomic_fetch_and(bs, ~bitset_mask(bit), mo); >>> +} >>> + >>> +/* Atomic exchange with memory ordering */ >>> +static inline bitset_t atom_bitset_xchg(bitset_t *bs, bitset_t neu, int mo) >>> +{ >>> + return __atomic_exchange_n(bs, neu, mo); >>> +} >>> + >>> +/* Return a & ~b */ >>> +static inline bitset_t bitset_andn(bitset_t a, bitset_t b) >>> +{ >>> + return a & ~b; >>> +} >>> + >>> +static inline bool bitset_is_eql(bitset_t a, bitset_t b) >>> +{ >>> + return a == b; >>> +} >>> + >>> +static inline bitset_t bitset_clr(bitset_t bs, uint32_t bit) >>> +{ >>> + return bs & ~bitset_mask(bit); >>> +} >>> + >>> +static inline bitset_t bitset_set(bitset_t bs, uint32_t bit) >>> +{ >>> + return bs | bitset_mask(bit); >>> +} >>> + >>> +static inline bitset_t bitset_null(void) >>> +{ >>> + return 0U; >>> +} >>> + >>> +static inline bool bitset_is_null(bitset_t a) >>> +{ >>> + return a == 0U; >>> +} >>> + >>> +static inline bool bitset_is_set(bitset_t a, uint32_t bit) >>> +{ >>> + return (a & bitset_mask(bit)) != 0; >>> +} >>> + >>> +#endif >>> >>