[PATCH 1/2] Improve __atomic_clear/test_and_set documentation

[PATCH 1/2] Improve __atomic_clear/test_and_set documentation

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Document that __atomic_clear and __atomic_test_and_set should
only be used with bool.

gcc/:
2013-06-13  Andi Kleen  <ak@linux.intel.com>

	* doc/extend.texi: Document that __atomic_clear and
	  __atomic_test_and_set should only be used with bool.
---
 gcc/doc/extend.texi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 1e1f8b3..aa3abef 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7438,6 +7438,8 @@ This built-in function performs an atomic test-and-set operation on
 the byte at @code{*@var{ptr}}.  The byte is set to some implementation
 defined nonzero ``set'' value and the return value is @code{true} if and only
 if the previous contents were ``set''.
+It should be only used for operands of type bool or atomic_flag. For 
+other types only part of the value may be set.
 
 All memory models are valid.
 
@@ -7447,6 +7449,10 @@ All memory models are valid.
 
 This built-in function performs an atomic clear operation on
 @code{*@var{ptr}}.  After the operation, @code{*@var{ptr}} contains 0.
+It should be only used for operands of type bool or atomic_flag and 
+in conjunction with __atomic_test_and_set.
+For other types it may only clear partially. If the type is not bool
+prefer using @code{__atomic_store}.
 
 The valid memory model variants are
 @code{__ATOMIC_RELAXED}, @code{__ATOMIC_SEQ_CST}, and
-- 
1.8.3

[PATCH 2/2] Fix HLE example in manual

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The HLE example in the manual only commits when using bool
for the flag, because __atomic_clear only writes bool, and
HLE requires the acquire and release to match.

So when the example is copied with e.g. an int variable it
does not commit and causes slower than expected performance.

Some people are running into problems because of this.

Switch it over to use __atomic_store.

Also fix a minor typo nearby.

gcc/:
2013-06-13  Andi Kleen  <ak@linux.intel.com>

	* doc/extend.texi: Dont use __atomic_clear in HLE
	example.  Fix typo.
---
 gcc/doc/extend.texi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index aa3abef..b6f786d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7524,18 +7524,20 @@ End lock elision on a lock variable.
 Memory model must be @code{__ATOMIC_RELEASE} or stronger.
 @end table
 
-When a lock acquire fails it's required for good performance to abort
+When a lock acquire fails it is required for good performance to abort
 the transaction quickly. This can be done with a @code{_mm_pause}
 
 @smallexample
 #include <immintrin.h> // For _mm_pause
 
+int lockvar;
+
 /* Acquire lock with lock elision */
 while (__atomic_exchange_n(&lockvar, 1, __ATOMIC_ACQUIRE|__ATOMIC_HLE_ACQUIRE))
     _mm_pause(); /* Abort failed transaction */
 ...
 /* Free lock with lock elision */
-__atomic_clear(&lockvar, __ATOMIC_RELEASE|__ATOMIC_HLE_RELEASE);
+__atomic_store(&lockvar, 0, __ATOMIC_RELEASE|__ATOMIC_HLE_RELEASE);
 @end smallexample
 
 @node Object Size Checking
-- 
1.8.3

Re: [PATCH 1/2] Improve __atomic_clear/test_and_set documentation

[Richard Henderson]

On 06/20/2013 06:20 AM, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Document that __atomic_clear and __atomic_test_and_set should
> only be used with bool.
> 
> gcc/:
> 2013-06-13  Andi Kleen  <ak@linux.intel.com>
> 
> 	* doc/extend.texi: Document that __atomic_clear and
> 	  __atomic_test_and_set should only be used with bool.
> ---
>  gcc/doc/extend.texi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 1e1f8b3..aa3abef 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7438,6 +7438,8 @@ This built-in function performs an atomic test-and-set operation on
>  the byte at @code{*@var{ptr}}.  The byte is set to some implementation
>  defined nonzero ``set'' value and the return value is @code{true} if and only
>  if the previous contents were ``set''.
> +It should be only used for operands of type bool or atomic_flag. For 
> +other types only part of the value may be set.

@code{bool}.  We have no definition for atomic_flag.  Perhaps just @code{char}
for now?

> +It should be only used for operands of type bool or atomic_flag and 

Same.

> +in conjunction with __atomic_test_and_set.

@code{__atomic_test_and_set}.



r~

Re: [PATCH 2/2] Fix HLE example in manual

[Richard Henderson]

On 06/20/2013 06:20 AM, Andi Kleen wrote:
> gcc/:
> 2013-06-13  Andi Kleen  <ak@linux.intel.com>
> 
> 	* doc/extend.texi: Dont use __atomic_clear in HLE
> 	example.  Fix typo.

Ok.


r~

Re: [PATCH 1/2] Improve __atomic_clear/test_and_set documentation

[Andi Kleen]

> > +It should be only used for operands of type bool or atomic_flag. For 
> > +other types only part of the value may be set.
> 
> @code{bool}.  We have no definition for atomic_flag.  Perhaps just @code{char}
> for now?
> 
> > +It should be only used for operands of type bool or atomic_flag and 
> 
> Same.
> 
> > +in conjunction with __atomic_test_and_set.
> 
> @code{__atomic_test_and_set}.

Ok with these changes?

-Andi


-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] Improve __atomic_clear/test_and_set documentation

[Richard Henderson]

On 06/20/2013 11:14 AM, Andi Kleen wrote:
>>> +It should be only used for operands of type bool or atomic_flag. For 
>>> +other types only part of the value may be set.
>>
>> @code{bool}.  We have no definition for atomic_flag.  Perhaps just @code{char}
>> for now?
>>
>>> +It should be only used for operands of type bool or atomic_flag and 
>>
>> Same.
>>
>>> +in conjunction with __atomic_test_and_set.
>>
>> @code{__atomic_test_and_set}.
> 
> Ok with these changes?

Yeah.


r~

Re: [PATCH 2/2] Fix HLE example in manual

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:
>  ...
>  /* Free lock with lock elision */
> -__atomic_clear(&lockvar, __ATOMIC_RELEASE|__ATOMIC_HLE_RELEASE);
> +__atomic_store(&lockvar, 0, __ATOMIC_RELEASE|__ATOMIC_HLE_RELEASE);

Sorry I realized it should be actually __atomic_store_n, not __atomic_store.
I will fix that as an obvious change, unless someone objects.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH 2/2] Fix HLE example in manual

[Andi Kleen]

On Thu, Jun 20, 2013 at 11:05:15AM -0700, Richard Henderson wrote:
> On 06/20/2013 06:20 AM, Andi Kleen wrote:
> > gcc/:
> > 2013-06-13  Andi Kleen  <ak@linux.intel.com>
> > 
> > 	* doc/extend.texi: Dont use __atomic_clear in HLE
> > 	example.  Fix typo.
> 
> Ok.

I would like to to backport this to 4.8. Ok too?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

*PING* Re: [PATCH 2/2] Fix HLE example in manual

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:

> On Thu, Jun 20, 2013 at 11:05:15AM -0700, Richard Henderson wrote:
>> On 06/20/2013 06:20 AM, Andi Kleen wrote:
>> > gcc/:
>> > 2013-06-13  Andi Kleen  <ak@linux.intel.com>
>> > 
>> > 	* doc/extend.texi: Dont use __atomic_clear in HLE
>> > 	example.  Fix typo.
>> 
>> Ok.
>
> I would like to to backport this to 4.8. Ok too?

Ping!

Is this ok to backport?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: *PING* Re: [PATCH 2/2] Fix HLE example in manual

[Gerald Pfeifer]

On Sun, 8 Sep 2013, Andi Kleen wrote:
>>>> 2013-06-13  Andi Kleen  <ak@linux.intel.com>
>>>> 
>>>> 	* doc/extend.texi: Dont use __atomic_clear in HLE
>>>> 	example.  Fix typo.
>> I would like to to backport this to 4.8. Ok too?
> Ping!

That's documentation only, right?

Okay.

Gerald