[2/2] Fix HLE example in manual

Submitted by Andi Kleen on June 20, 2013, 1:20 p.m.

Details

Message ID 1371734413-12372-2-git-send-email-andi@firstfloor.org
State New
Headers show

Commit Message

Andi Kleen June 20, 2013, 1:20 p.m.
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(-)

Comments

Richard Henderson June 20, 2013, 6:05 p.m.
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~
Andi Kleen June 22, 2013, 4:34 p.m.
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
Andi Kleen Aug. 10, 2013, 7:38 p.m.
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
Andi Kleen Sept. 8, 2013, 7:48 p.m.
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
Gerald Pfeifer Sept. 9, 2013, 8:40 a.m.
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

Patch hide | download patch | download mbox

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