Sending from my ARM email account, I hope Outlook does not mess up the
format.
On 04/04/2017, 22:21, "Dmitry Eremin-Solenikov"
<dmitry.ereminsoleni...@linaro.org> wrote:
>On 04.04.2017 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>
>
>>
>>+/***********************************************************************
>>*******
>> + * bitset abstract data type
>> +
>>*************************************************************************
>>****/
>> +/* This could be a struct of scalars to support larger bit sets */
>> +
>> +#if ATOM_BITSET_SIZE <= 32
>
>Maybe I missed, where did you set this macro?
In odp_config_internal.h
It is a build time configuration.
>
>Also, why do you need several versions of bitset? Can you stick to one
>size that fits all?
Some 32-bit archs (ARMv7a, x86) will only support 64-bit atomics (AFAIK).
Only x86-64 and ARMv8a supports 128-bit atomics (and compiler support for
128-bit atomics for ARMv8a is a bit lackingŠ).
Other architectures might only support 32-bit atomic operations.
I think the user should have control over this but if you think that we
should just select the max value that is supported by the architecture in
question and thus skip one build configuration, I am open to this. We will
still need separate versions for 32/64/128 bits because there are slight
differences in the syntax and implementation. Such are the vagaries of the
C standard (and GCC extensions).
>
>> +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);
>> +}
>> +
>> +#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;
>> +
>> +static inline bitset_t bitset_mask(uint32_t bit)
>> +{
>> +if (bit < 64)
>> +return 1ULL << bit;
>> +else
>> +return (unsigned __int128)(1ULL << (bit - 64)) << 64;
>> +}
>> +
>> +/* 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
>> +#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);
>> +}
>> +
>
>Any real reason for the following defines? Why do you need them?
The functions were added as they were needed, e.g. in
odp_schedule_scalable.c.
I dont think there is anyone which is not used anymore but can
double-check that.
>
>> +/* Return a & ~b */
>> +static inline bitset_t bitset_andn(bitset_t a, bitset_t b)
Used
>> +{
>> +return a & ~b;
>> +}
>> +
>> +static inline bool bitset_is_eql(bitset_t a, bitset_t b)
Used
>> +{
>> +return a == b;
>> +}
>> +
>> +static inline bitset_t bitset_clr(bitset_t bs, uint32_t bit)
Used
>> +{
>> +return bs & ~bitset_mask(bit);
>> +}
>> +
>> +static inline bitset_t bitset_set(bitset_t bs, uint32_t bit)
Used
>> +{
>> +return bs | bitset_mask(bit);
>> +}
>> +
>> +static inline bitset_t bitset_null(void)
Used
>> +{
>> +return 0U;
>> +}
>> +
>> +static inline bool bitset_is_null(bitset_t a)
Used
>> +{
>> +return a == 0U;
>> +}
>> +
>> +static inline bool bitset_is_set(bitset_t a, uint32_t bit)
Used
>> +{
>> +return (a & bitset_mask(bit)) != 0;
>> +}
>> +
>> +#endif
>>
>
>
>--
>With best wishes
>Dmitry
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
