[PATCH] Clarify __atomic_compare_exchange_n docs

[PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

Someone on IRC incorrectly parsed the docs at
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/_005f_005fatomic-Builtins.html#index-g_t_005f_005fatomic_005fcompare_005fexchange_005fn-3536
as:

  IF
  (
   desired is written into *ptr
   AND
   the execution is considered to conform to the memory model
   specified by success_memmodel.
  )
  {
   true is returned
  }
  otherwise ...

rather than the intended:

  IF ( desired is written into *ptr )
  {
   true is returned
   AND
   the execution is considered to conform to the memory model
   specified by success_memmodel.
  }
  otherwise ...

So they asked:

> What is otherwise, here? Can I make the function return false even
> when 'desired' has been written into 'ptr'? How do I do it? I could
> not write an example, so far.

This patch rewords it to avoid the ambiguity.

I've also replaced the rather clunky "the operation is considered to
conform to" phrasing. (It's only _considered_ to? So does it or doesn't
it use that memory order?) Instead I've used the terminology from the
C and C++ standards, which say "memory is affected according to".

OK for trunk?


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1698 bytes --]

commit 370a92b7f4d318957a70d0d3f1185f1c6f282ff3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Sep 29 12:45:21 2015 +0100

    	* doc/extend.texi (__atomic Builtins): Clarify compare_exchange
    	effects.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8406945..0de94f2 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9353,17 +9353,17 @@ This compares the contents of @code{*@var{ptr}} with the contents of
 @code{*@var{expected}}. If equal, the operation is a @emph{read-modify-write}
 operation that writes @var{desired} into @code{*@var{ptr}}.  If they are not
 equal, the operation is a @emph{read} and the current contents of
-@code{*@var{ptr}} is written into @code{*@var{expected}}.  @var{weak} is true
+@code{*@var{ptr}} are written into @code{*@var{expected}}.  @var{weak} is true
 for weak compare_exchange, and false for the strong variation.  Many targets 
 only offer the strong variation and ignore the parameter.  When in doubt, use
 the strong variation.
 
-True is returned if @var{desired} is written into
-@code{*@var{ptr}} and the operation is considered to conform to the
+If @var{desired} is written into @code{*@var{ptr}} then true is returned
+and memory is affected according to the
 memory order specified by @var{success_memorder}.  There are no
 restrictions on what memory order can be used here.
 
-False is returned otherwise, and the operation is considered to conform
+Otherwise, false is returned and memory is affected according
 to @var{failure_memorder}. This memory order cannot be
 @code{__ATOMIC_RELEASE} nor @code{__ATOMIC_ACQ_REL}.  It also cannot be a
 stronger order than that specified by @var{success_memorder}.

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Sandra Loosemore]

On 09/29/2015 06:00 AM, Jonathan Wakely wrote:
> Someone on IRC incorrectly parsed the docs at
> https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/_005f_005fatomic-Builtins.html#index-g_t_005f_005fatomic_005fcompare_005fexchange_005fn-3536
>
> as:
>
>   IF
>   (
>    desired is written into *ptr
>    AND
>    the execution is considered to conform to the memory model
>    specified by success_memmodel.
>   )
>   {
>    true is returned
>   }
>   otherwise ...
>
> rather than the intended:
>
>   IF ( desired is written into *ptr )
>   {
>    true is returned
>    AND
>    the execution is considered to conform to the memory model
>    specified by success_memmodel.
>   }
>   otherwise ...
>
> So they asked:
>
>> What is otherwise, here? Can I make the function return false even
>> when 'desired' has been written into 'ptr'? How do I do it? I could
>> not write an example, so far.
>
> This patch rewords it to avoid the ambiguity.
>
> I've also replaced the rather clunky "the operation is considered to
> conform to" phrasing. (It's only _considered_ to? So does it or doesn't
> it use that memory order?) Instead I've used the terminology from the
> C and C++ standards, which say "memory is affected according to".
>
> OK for trunk?

This is OK, as far as it goes, but while we're at it, can we do 
something to fix the description of the weak parameter?

> @@ -9353,17 +9353,17 @@ This compares the contents of @code{*@var{ptr}} with the contents of
>  @code{*@var{expected}}. If equal, the operation is a @emph{read-modify-write}
>  operation that writes @var{desired} into @code{*@var{ptr}}.  If they are not
>  equal, the operation is a @emph{read} and the current contents of
> -@code{*@var{ptr}} is written into @code{*@var{expected}}.  @var{weak} is true
> +@code{*@var{ptr}} are written into @code{*@var{expected}}.  @var{weak} is true
>  for weak compare_exchange, and false for the strong variation.  Many targets
>  only offer the strong variation and ignore the parameter.  When in doubt, use
>  the strong variation.

What is "weak compare_exchange", and what is "the strong variation", and 
how do they differ in terms of behavior?

-Sandra

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Andrew Haley]

On 09/29/2015 04:21 PM, Sandra Loosemore wrote:
> What is "weak compare_exchange", and what is "the strong variation", and 
> how do they differ in terms of behavior?

It's in C++11 29.6.5:

Remark: The weak compare-and-exchange operations may fail spuriously,
that is, return false while leaving the contents of memory pointed to
by expected before the operation is the same that same as that of the
object and the same as that of expected after the operation. [ Note:
This spurious failure enables implementation of compare-and-exchange
on a broader class of machines, e.g., load- locked store-conditional
machines. A consequence of spurious failure is that nearly all uses of
weak compare-and-exchange will be in a loop.  When a
compare-and-exchange is in a loop, the weak version will yield better
performance on some platforms. When a weak compare-and-exchange would
require a loop and a strong one would not, the strong one is
preferable. — end note ]

The classic use of this is for shared counters: you don't care if you
miss an occasional count but you don't want the counter to go
backwards.

Whether we should replicate all of the C++11 language is perhaps
something we should discuss.

Andrew.

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 01/10/15 12:28 +0100, Andrew Haley wrote:
>On 09/29/2015 04:21 PM, Sandra Loosemore wrote:
>> What is "weak compare_exchange", and what is "the strong variation", and
>> how do they differ in terms of behavior?
>
>It's in C++11 29.6.5:
>
>Remark: The weak compare-and-exchange operations may fail spuriously,
>that is, return false while leaving the contents of memory pointed to
>by expected before the operation is the same that same as that of the
>object and the same as that of expected after the operation. [ Note:
>This spurious failure enables implementation of compare-and-exchange
>on a broader class of machines, e.g., load- locked store-conditional
>machines. A consequence of spurious failure is that nearly all uses of
>weak compare-and-exchange will be in a loop.  When a
>compare-and-exchange is in a loop, the weak version will yield better
>performance on some platforms. When a weak compare-and-exchange would
>require a loop and a strong one would not, the strong one is
>preferable. — end note ]
>
>The classic use of this is for shared counters: you don't care if you
>miss an occasional count but you don't want the counter to go
>backwards.
>
>Whether we should replicate all of the C++11 language is perhaps
>something we should discuss.

I would suggest we don't try to reproduce the standard definition, but
just say the weak version can fail spuriously and the strong can't.
IMHO this isn't the place to educate people in the fine points of
low-level atomics. As it says, "when in doubt use the strong
variation".

i.e. apply this in addition to my earlier suggestion.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 773 bytes --]

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 0de94f2..ce1b4ae 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9354,7 +9354,8 @@ This compares the contents of @code{*@var{ptr}} with the contents of
 operation that writes @var{desired} into @code{*@var{ptr}}.  If they are not
 equal, the operation is a @emph{read} and the current contents of
 @code{*@var{ptr}} are written into @code{*@var{expected}}.  @var{weak} is true
-for weak compare_exchange, and false for the strong variation.  Many targets 
+for weak compare_exchange, which may fail spuriously, and false for
+the strong variation, which never fails spuriously.  Many targets 
 only offer the strong variation and ignore the parameter.  When in doubt, use
 the strong variation.
 

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Andrew Haley]

On 10/01/2015 06:32 PM, Jonathan Wakely wrote:
> I would suggest we don't try to reproduce the standard definition, but
> just say the weak version can fail spuriously and the strong can't.
> IMHO this isn't the place to educate people in the fine points of
> low-level atomics. As it says, "when in doubt use the strong
> variation".
> 
> i.e. apply this in addition to my earlier suggestion.

"If you don't already know what a weak CAS is you probably should
not even think of using it."  :)

Andrew.

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

On 01/10/15 18:34 +0100, Andrew Haley wrote:
>On 10/01/2015 06:32 PM, Jonathan Wakely wrote:
>> I would suggest we don't try to reproduce the standard definition, but
>> just say the weak version can fail spuriously and the strong can't.
>> IMHO this isn't the place to educate people in the fine points of
>> low-level atomics. As it says, "when in doubt use the strong
>> variation".
>>
>> i.e. apply this in addition to my earlier suggestion.
>
>"If you don't already know what a weak CAS is you probably should
>not even think of using it."  :)

Exactly. These are not the built-ins you're looking for. :)

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Sandra Loosemore]

On 10/01/2015 05:28 AM, Andrew Haley wrote:
> On 09/29/2015 04:21 PM, Sandra Loosemore wrote:
>> What is "weak compare_exchange", and what is "the strong variation", and
>> how do they differ in terms of behavior?
>
> It's in C++11 29.6.5:
>
> Remark: The weak compare-and-exchange operations may fail spuriously,
> that is, return false while leaving the contents of memory pointed to
> by expected before the operation is the same that same as that of the
> object and the same as that of expected after the operation. [ Note:
> This spurious failure enables implementation of compare-and-exchange
> on a broader class of machines, e.g., load- locked store-conditional
> machines. A consequence of spurious failure is that nearly all uses of
> weak compare-and-exchange will be in a loop.  When a
> compare-and-exchange is in a loop, the weak version will yield better
> performance on some platforms. When a weak compare-and-exchange would
> require a loop and a strong one would not, the strong one is
> preferable. — end note ]
>
> The classic use of this is for shared counters: you don't care if you
> miss an occasional count but you don't want the counter to go
> backwards.
>
> Whether we should replicate all of the C++11 language is perhaps
> something we should discuss.

Hmmmm, yes.  Looking at the section as a whole, is it a bug in the 
implementation that the built-ins only "approximately match" the C++11 
requirements?  If there were an exact correspondence, it would only be 
necessary to point at the standard (I think it would be more helpful to 
mention <stdatomic.h> here than to cite a specific section number), 
identify what C++11 names the built-ins map onto, and to document any 
implementation-defined behavior allowed by the standard and GCC extensions.

-Sandra

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

On 01/10/15 11:57 -0600, Sandra Loosemore wrote:
>Hmmmm, yes.  Looking at the section as a whole, is it a bug in the 
>implementation that the built-ins only "approximately match" the C++11 
>requirements?

AFAIK they exactly match, so I don't know why the docs say that.

>If there were an exact correspondence, it would only be 
>necessary to point at the standard (I think it would be more helpful 
>to mention <stdatomic.h> here than to cite a specific section number), 

That's the C11 header, the C++11 header is <atomic>.

>identify what C++11 names the built-ins map onto,

It's pretty straightforward, I'm not sure we need to say much:

__atomic_xxx(_n)? -> atomic_xxx


>and to document any 
>implementation-defined behavior allowed by the standard and GCC 
>extensions.

There is nothing implementation-defined in the atomics clause, just
some wooly requirements like "Implementations should make atomic
stores visible to atomic loads within a reasonable amount of time."

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 35 bytes --]

There's a typo in an example too.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 440 bytes --]

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index ce1b4ae..599ad87 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9471,7 +9471,7 @@ alignment.  A value of 0 indicates typical alignment should be used.  The
 compiler may also ignore this parameter.
 
 @smallexample
-if (_atomic_always_lock_free (sizeof (long long), 0))
+if (__atomic_always_lock_free (sizeof (long long), 0))
 @end smallexample
 
 @end deftypefn

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 65 bytes --]

Here's the latest version of the patch, including the typo fix.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2188 bytes --]

commit 96468d6b7e782501459bad306b31d45bc0ba5155
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Oct 3 13:59:47 2015 +0100

    Clarify __atomic_compare_exchange effects
    
    	* doc/extend.texi (__atomic Builtins): Clarify compare_exchange
    	effects.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8406945..599ad87 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9353,17 +9353,18 @@ This compares the contents of @code{*@var{ptr}} with the contents of
 @code{*@var{expected}}. If equal, the operation is a @emph{read-modify-write}
 operation that writes @var{desired} into @code{*@var{ptr}}.  If they are not
 equal, the operation is a @emph{read} and the current contents of
-@code{*@var{ptr}} is written into @code{*@var{expected}}.  @var{weak} is true
-for weak compare_exchange, and false for the strong variation.  Many targets 
+@code{*@var{ptr}} are written into @code{*@var{expected}}.  @var{weak} is true
+for weak compare_exchange, which may fail spuriously, and false for
+the strong variation, which never fails spuriously.  Many targets 
 only offer the strong variation and ignore the parameter.  When in doubt, use
 the strong variation.
 
-True is returned if @var{desired} is written into
-@code{*@var{ptr}} and the operation is considered to conform to the
+If @var{desired} is written into @code{*@var{ptr}} then true is returned
+and memory is affected according to the
 memory order specified by @var{success_memorder}.  There are no
 restrictions on what memory order can be used here.
 
-False is returned otherwise, and the operation is considered to conform
+Otherwise, false is returned and memory is affected according
 to @var{failure_memorder}. This memory order cannot be
 @code{__ATOMIC_RELEASE} nor @code{__ATOMIC_ACQ_REL}.  It also cannot be a
 stronger order than that specified by @var{success_memorder}.
@@ -9470,7 +9471,7 @@ alignment.  A value of 0 indicates typical alignment should be used.  The
 compiler may also ignore this parameter.
 
 @smallexample
-if (_atomic_always_lock_free (sizeof (long long), 0))
+if (__atomic_always_lock_free (sizeof (long long), 0))
 @end smallexample
 
 @end deftypefn

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

On 03/10/15 14:00 +0100, Jonathan Wakely wrote:
>Here's the latest version of the patch, including the typo fix.

Is this patch OK for trunk?

The original thread faded out, it's split across two months in the
archives:

https://gcc.gnu.org/ml/gcc-patches/2015-09/msg02190.html
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00307.html

While we might be able to make further improvements I think this is
better than what we have now (and I'm hoping not to spend much more
on it ;-)


commit 96468d6b7e782501459bad306b31d45bc0ba5155
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Oct 3 13:59:47 2015 +0100

    Clarify __atomic_compare_exchange effects
    
    	* doc/extend.texi (__atomic Builtins): Clarify compare_exchange
    	effects.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8406945..599ad87 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9353,17 +9353,18 @@ This compares the contents of @code{*@var{ptr}} with the contents of
 @code{*@var{expected}}. If equal, the operation is a @emph{read-modify-write}
 operation that writes @var{desired} into @code{*@var{ptr}}.  If they are not
 equal, the operation is a @emph{read} and the current contents of
-@code{*@var{ptr}} is written into @code{*@var{expected}}.  @var{weak} is true
-for weak compare_exchange, and false for the strong variation.  Many targets 
+@code{*@var{ptr}} are written into @code{*@var{expected}}.  @var{weak} is true
+for weak compare_exchange, which may fail spuriously, and false for
+the strong variation, which never fails spuriously.  Many targets 
 only offer the strong variation and ignore the parameter.  When in doubt, use
 the strong variation.
 
-True is returned if @var{desired} is written into
-@code{*@var{ptr}} and the operation is considered to conform to the
+If @var{desired} is written into @code{*@var{ptr}} then true is returned
+and memory is affected according to the
 memory order specified by @var{success_memorder}.  There are no
 restrictions on what memory order can be used here.
 
-False is returned otherwise, and the operation is considered to conform
+Otherwise, false is returned and memory is affected according
 to @var{failure_memorder}. This memory order cannot be
 @code{__ATOMIC_RELEASE} nor @code{__ATOMIC_ACQ_REL}.  It also cannot be a
 stronger order than that specified by @var{success_memorder}.
@@ -9470,7 +9471,7 @@ alignment.  A value of 0 indicates typical alignment should be used.  The
 compiler may also ignore this parameter.
 
 @smallexample
-if (_atomic_always_lock_free (sizeof (long long), 0))
+if (__atomic_always_lock_free (sizeof (long long), 0))
 @end smallexample
 
 @end deftypefn

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Sandra Loosemore]

On 01/13/2016 07:27 AM, Jonathan Wakely wrote:
> On 03/10/15 14:00 +0100, Jonathan Wakely wrote:
>> Here's the latest version of the patch, including the typo fix.
>
> Is this patch OK for trunk?
>
> The original thread faded out, it's split across two months in the
> archives:
>
> https://gcc.gnu.org/ml/gcc-patches/2015-09/msg02190.html
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00307.html
>
> While we might be able to make further improvements I think this is
> better than what we have now (and I'm hoping not to spend much more
> on it ;-)

I apologize for losing track of this patch during the previous 
discussion.  The version you have now is OK to commit.

-Sandra

Re: [PATCH] Clarify __atomic_compare_exchange_n docs

[Jonathan Wakely]

On 13/01/16 10:16 -0700, Sandra Loosemore wrote:
>On 01/13/2016 07:27 AM, Jonathan Wakely wrote:
>>On 03/10/15 14:00 +0100, Jonathan Wakely wrote:
>>>Here's the latest version of the patch, including the typo fix.
>>
>>Is this patch OK for trunk?
>>
>>The original thread faded out, it's split across two months in the
>>archives:
>>
>>https://gcc.gnu.org/ml/gcc-patches/2015-09/msg02190.html
>>https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00307.html
>>
>>While we might be able to make further improvements I think this is
>>better than what we have now (and I'm hoping not to spend much more
>>on it ;-)
>
>I apologize for losing track of this patch during the previous 
>discussion.  The version you have now is OK to commit.

No problem, I'd forgotten about it myself until I was clearing out
some local Git branches. It's committed now.