| Message ID | 1417132472-23039-2-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 27 November 2014 at 18:54, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > --- > (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 > 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 > * @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 > > *****************************************************************************/ > > /** > - * 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. you mix ptr and p, we should be consistent in one file. > + _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. > + */ > +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. > + _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 > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 1 December 2014 at 21:36, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 27 November 2014 at 18:54, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> --- >> (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 >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP
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 * @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, *****************************************************************************/ /** - * 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, + _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) + */ +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, + _odp_memmodel_t fail) +{ + return __atomic_compare_exchange(&ptr->v, exp_p, val_p, + false/*strong*/, succ, fail); +} +#endif + /** * @} */
Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (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 minor changes to the comments. .../linux-generic/include/odp_atomic_internal.h | 72 ++++++++++++++++++++-- 1 file changed, 66 insertions(+), 6 deletions(-)