On Tue, Apr 4, 2017 at 3:35 PM, Bill Fischofer
<bill.fischo...@linaro.org> wrote:
> On Tue, Apr 4, 2017 at 3:08 PM, Brian Brooks <brian.bro...@arm.com> 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?
>
> Per the Linaro membership agreement, contributions from assignees may
> be from company addresses, but assignees should use their linaro.org
> e-mail addresses.
Sorry, that should read contributions from member engineers may be
from company addresses. Assignees use linaro.org.
>
> The Linaro copyright is part of the contributor's agreement. You can
> retain the ARM copyright as well. See, for example,
> odp_traffic_mngr.c, which contains both an EZchip and a Linaro
> copyright.
>
>>
>>> > +
>>> > +#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?
>>
>>> > +}
>>> > +
>>> > +#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
>>> >
>>>
