Re: [PATCH v3] x86: Optimize atomic_compare_and_exchange_[val|bool]_acq [BZ #28537]

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Thu, Nov 4, 2021 at 11:15 AM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> From the CPU's point of view, getting a cache line for writing is more
> expensive than reading.  See Appendix A.2 Spinlock in:
>
> https://www.intel.com/content/dam/www/public/us/en/documents/white-papers/xeon-lock-scaling-analysis-paper.pdf
>
> The full compare and swap will grab the cache line exclusive and cause
> excessive cache line bouncing.  Load the current memory value via a
> volatile pointer first, which should be atomic and won't be optimized
> out by compiler, check and return immediately if writing cache line may
> fail to reduce cache line bouncing on contended locks.
>
> This fixes BZ# 28537.
>
> A GCC bug is opened:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103065
>
> The fixed compiler should define __HAVE_SYNC_COMPARE_AND_SWAP_LOAD_CHECK
> to indicate that compiler will generate the check with the volatile load.
> Then glibc can check __HAVE_SYNC_COMPARE_AND_SWAP_LOAD_CHECK to avoid the
> extra volatile load.
> ---
>  sysdeps/x86/atomic-machine.h | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/x86/atomic-machine.h b/sysdeps/x86/atomic-machine.h
> index 2692d94a92..597dc1cf92 100644
> --- a/sysdeps/x86/atomic-machine.h
> +++ b/sysdeps/x86/atomic-machine.h
> @@ -73,9 +73,20 @@ typedef uintmax_t uatomic_max_t;
>  #define ATOMIC_EXCHANGE_USES_CAS       0
>
>  #define atomic_compare_and_exchange_val_acq(mem, newval, oldval) \
> -  __sync_val_compare_and_swap (mem, oldval, newval)
> +  ({ volatile __typeof (*(mem)) *memp = (mem);                         \
> +     __typeof (*(mem)) oldmem = *memp, ret;                            \
> +     ret = (oldmem == (oldval)                                         \
> +           ? __sync_val_compare_and_swap (mem, oldval, newval)         \
> +           : oldmem);                                                  \
> +     ret; })
>  #define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
> -  (! __sync_bool_compare_and_swap (mem, oldval, newval))
> +  ({ volatile __typeof (*(mem)) *memp = (mem);                         \
> +     __typeof (*(mem)) oldmem = *memp;                                 \
> +     int ret;                                                          \
> +     ret = (oldmem == (oldval)                                         \
> +           ? !__sync_bool_compare_and_swap (mem, oldval, newval)       \
> +           : 1);                                                       \
> +     ret; })
>
>
>  #define __arch_c_compare_and_exchange_val_8_acq(mem, newval, oldval) \
> --
> 2.33.1
>

Worth noting on X86 any of the __atomic_fetch_* builtins aside from add/sub
are implemented with a CAS loop that may benefit from this:
https://godbolt.org/z/z87v9Kbcz