[Bug target/107676] New: Nonsensical docs for -mrelax-cmpxchg-loop

[Bug target/107676] New: Nonsensical docs for -mrelax-cmpxchg-loop

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

            Bug ID: 107676
           Summary: Nonsensical docs for -mrelax-cmpxchg-loop
           Product: gcc
           Version: 12.2.1
            Status: UNCONFIRMED
          Keywords: documentation
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---
            Target: x86*

g:4d281ff7ddd8f6365943c0a622107f92315bb8a6 added:

+@item -mrelax-cmpxchg-loop
+@opindex mrelax-cmpxchg-loop
+Relax cmpxchg loop by emitting an early load and compare before cmpxchg,
+execute pause if load value is not expected. This reduces excessive
+cachline bouncing when and works for all atomic logic fetch builtins
+that generates compare and swap loop.


1) "cachline" is a typo
2) "when" ... when what?
3) What does "atomic logic fetch builtins" mean?
4) "generates" should be "generate"
5) Is a "cmpxchg loop" the same as a "compare and swap loop"? Can we use the
same term consistently?

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Is it really better to do a load and check rather than just hoist the first
cmpxchg out of the loop, and add the pause in the loop if retrying?

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #0)
> 3) What does "atomic logic fetch builtins" mean?

From the commit msg, it seems this means atomic_fetch_{or,xor,and,nand}.
But out of context in the GCC manual that isn't clear to me. We don't call them
that in the https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
section.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-11-14
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Maybe this is better:
For loops that are emitted by __atomic_compare_and_exchange (and other
__atomic* builtins which emit a compare and exechange loop, e.g. ....), emit an
atomic load before the loop and if the value was not the expected value, emit a
pause instruction. This might reduce execussive cache bouncing of the memory.

Note replace .... with other builtin which also emits a compare and exchange
loop.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't think __atomic_compare_exchange emits such a loop. This is about
__atomic_fetch_xor and friends, which do emit cmpxchg loops. But there are four
such functions to name.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> I don't think __atomic_compare_exchange emits such a loop. This is about
> __atomic_fetch_xor and friends, which do emit cmpxchg loops. But there are
> four such functions to name.

Oh yes right.
Then this:
For compare and exchange loops that are emitted by some __atomic_* builtins
(e.g. ....), emit an atomic load before the loop and if the value was not the
expected value, emit a pause instruction. This might reduce execussive cache
bouncing of the memory.


I think that is better wording than it was before. I hope the person who added
this option can take over this to get it closer to what it should be.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[wwwhhhyyy333 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #6 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
(In reply to Andrew Pinski from comment #5)
> (In reply to Jonathan Wakely from comment #4)
> > I don't think __atomic_compare_exchange emits such a loop. This is about
> > __atomic_fetch_xor and friends, which do emit cmpxchg loops. But there are
> > four such functions to name.
> 
> Oh yes right.
> Then this:
> For compare and exchange loops that are emitted by some __atomic_* builtins
> (e.g. ....), emit an atomic load before the loop and if the value was not
> the expected value, emit a pause instruction. This might reduce execussive
> cache bouncing of the memory.
> 
> 
> I think that is better wording than it was before. I hope the person who
> added this option can take over this to get it closer to what it should be.

Thanks for all the suggestions, a patch has been posted at
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606212.html

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexander Monakov <amonakov@gcc.gnu.org>:

https://gcc.gnu.org/g:85966f0d20baf50a767ac1110f537395441604a9

commit r13-4094-g85966f0d20baf50a767ac1110f537395441604a9
Author: Alexander Monakov <amonakov@ispras.ru>
Date:   Wed Nov 16 16:33:11 2022 +0300

    doc: fix description of -mrelax-cmpxchg-loop [PR 107676]

    gcc/ChangeLog:

            PR target/107676
            * doc/invoke.texi (-mrelax-cmpxchg-loop): Reword description.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |amonakov at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #8 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
Fixed.

[Bug target/107676] Nonsensical docs for -mrelax-cmpxchg-loop

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107676

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0