On 1 December 2014 at 21:36, Mike Holmes <[email protected]> wrote:
>
>
> On 27 November 2014 at 18:54, Ola Liljedahl <[email protected]>
> wrote:
>>
>> Signed-off-by: Ola Liljedahl <[email protected]>
>> ---
>> (This document/code contribution attached is provided under the terms of
>> agreement LES-LTM-21309)
>>
>> odp_atomic_internal.h is updated with operations (e.g. exchange,
>> compare-and-
>> exchange) on 16-byte atomics (_odp_atomic_u128_t atomic type with
>> corresponding
>> _uint128_t scalar type).
>> GCC on x86-64 requires the compiler flag -mcx16 to enable support for
>> 16-byte
>> atomics.
>
>
> Some of this description at least should be above --- so that it can be seen
> in the commit history
How do I do this now, the patch has already been merged?
I will post another patch fixing the issues you have identified below.
Can I add some of the description there?
>
>
>>
>> Some minor changes to the comments.
>>
>> .../linux-generic/include/odp_atomic_internal.h | 72
>> ++++++++++++++++++++--
>> 1 file changed, 66 insertions(+), 6 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp_atomic_internal.h
>> b/platform/linux-generic/include/odp_atomic_internal.h
>> index 38b0de0..25da2cd 100644
>> --- a/platform/linux-generic/include/odp_atomic_internal.h
>> +++ b/platform/linux-generic/include/odp_atomic_internal.h
>> @@ -320,9 +320,10 @@ static inline uint64_t
>> _odp_atomic_u64_xchg_mm(odp_atomic_u64_t *ptr,
>> * @param ptr Pointer to a 64-bit atomic variable
>
>
> is this an input or output @param[in] ptr - same for all arguments
OK. Sometimes 'ptr' is both an input and an output parameter (the
variable it points to is updated), use "[in,out]"?
>
>>
>> * @param exp_p Pointer to expected value (updated on failure)
>> * @param val New value to write
>> - * @param succ Memory model associated with a successful compare-and-swap
>> + * @param succ Memory model associated with a successful
>> compare-and-swap
>> + * operation
>> + * @param fail Memory model associated with a failed compare-and-swap
>> * operation
>> - * @param fail Memory model associated with a failed compare-and-swap
>> operation
>> *
>> * @return 1 (true) if exchange successful, 0 (false) if not successful
>> (and
>> * '*exp_p' updated with current value)
>> @@ -511,9 +512,9 @@ static inline void
>> *_odp_atomic_ptr_xchg(_odp_atomic_ptr_t *ptr,
>> * @param ptr Pointer to a pointer atomic variable
>> * @param exp_p Pointer to expected value (updated on failure)
>> * @param val New value to write
>> - * @param succ Memory model associated with a successful
>> compare-and-swap
>> + * @param succ Memory model associated with a successful
>> compare-and-swap
>> * operation
>> - * @param fail Memory model associated with a failed
>> compare-and-swap
>> + * @param fail Memory model associated with a failed compare-and-swap
>> * operation
>> *
>> * @return 1 (true) if exchange successful, 0 (false) if not successful
>> (and
>> @@ -541,7 +542,7 @@ static inline int
>> _odp_atomic_ptr_cmp_xchg_strong(_odp_atomic_ptr_t *ptr,
>
>
> should this be odp_bool_t
OK. I don't think odp_bool_t was merged when I submitted the patch.
>
>>
>>
>> *****************************************************************************/
>>
>> /**
>> - * Initialize a flag atomic variable
>> + * Initialize an atomic flag variable
>> *
>> * @param ptr Pointer to a flag atomic variable
>> * @param val The initial value (true or false) of the variable
>> @@ -568,7 +569,8 @@ static inline int
>> _odp_atomic_flag_load(_odp_atomic_flag_t *ptr)
>> /**
>> * Test-and-set of atomic flag variable
>> * @Note Operation has acquire memory model. It pairs with a later
>> - * release operation in some thread.
>> + * release operation in some thread. It does not provide release or
>> + * acquire/release semantics.
>> *
>> * @param ptr Pointer to a flag atomic variable
>> * @return The old value (true or false) of the variable
>> @@ -591,6 +593,64 @@ static inline void
>> _odp_atomic_flag_clear(_odp_atomic_flag_t *ptr)
>> __atomic_clear(ptr, __ATOMIC_RELEASE);
>> }
>>
>> +/* Check if target and compiler supports 128-bit scalars and
>> corresponding
>> + * exchange and CAS operations */
>> +/* GCC on x86-64 needs -mcx16 compiler option */
>> +#if defined __SIZEOF_INT128__ && defined
>> __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
>> +
>> +/** Preprocessor symbol that indicates support for 128-bit atomics */
>> +#define ODP_ATOMIC_U128
>> +
>> +/** An unsigned 128-bit (16-byte) scalar type */
>> +typedef __int128 _uint128_t;
>> +
>> +/** Atomic 128-bit type */
>> +typedef struct {
>> + _uint128_t v; /**< Actual storage for the atomic variable */
>> +} _odp_atomic_u128_t ODP_ALIGNED(16);
>> +
>> +/**
>> + * 16-byte atomic exchange operation
>> + *
>> + * @param ptr Pointer to a 16-byte atomic variable
>> + * @param val_p Pointer to new value to write
>> + * @param old_p Pointer to location for old value
>> + * @param mmodel Memory model associated with the exchange
>> operation
>> + */
>> +static inline void _odp_atomic_u128_xchg_mm(_odp_atomic_u128_t *ptr,
>> + _uint128_t *val_p,
>
>
> nit we appear to been putting the p before the variable name generally.
OK.
> you mix ptr and p, we should be consistent in one file.
OK, I will invent something for 'ptr', perhaps 'p_atom'?
>
>>
>> + _uint128_t *old_p,
>> + _odp_memmodel_t mm)
>> +{
>> + __atomic_exchange(&ptr->v, val_p, old_p, mm);
>> +}
>> +
>> +/**
>> + * Atomic compare and exchange (swap) of 16-byte atomic variable
>> + * "Strong" semantics, will not fail spuriously.
>> + *
>> + * @param ptr Pointer to a 16-byte atomic variable
>> + * @param exp_p Pointer to expected value (updated on failure)
>> + * @param val_p Pointer to new value to write
>> + * @param succ Memory model associated with a successful
>> compare-and-swap
>> + * operation
>> + * @param fail Memory model associated with a failed compare-and-swap
>> + * operation
>> + *
>> + * @return 1 (true) if exchange successful, 0 (false) if not successful
>> (and
>> + * '*exp_p' updated with current value)
>
>
> Use retval, one for each case.
OK
>
>>
>> + */
>> +static inline int _odp_atomic_u128_cmp_xchg_mm(_odp_atomic_u128_t *ptr,
>> + _uint128_t *exp_p,
>> + _uint128_t *val_p,
>> + _odp_memmodel_t succ,
>
>
> success would be better, no need to save space, applications will pass in
> whatever length variable they were using anyway.
OK
>
>>
>> + _odp_memmodel_t fail)
>> +{
>> + return __atomic_compare_exchange(&ptr->v, exp_p, val_p,
>> + false/*strong*/, succ, fail);
>> +}
>> +#endif
>> +
>> /**
>> * @}
>> */
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> [email protected]
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
