[PATCH 2/5] x86: Remove catomic_* locking primitives
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Sep 8 13:16:56 GMT 2025
On 08/09/25 09:57, Adhemerval Zanella Netto wrote:
>
>
> On 08/09/25 09:38, Uros Bizjak wrote:
>> Remove obsolete catomic_* locking primitives which don't map
>> to standard compiler builtins.
>>
>> There are still a couple of places in the tree that uses them
>> (malloc/arena.c and malloc/malloc.c).
>>
>> x86 didn't define __arch_c_compare_and_exchange_bool_* primitives
>> so fallback code used __arch_c_compare_and_exchange_val_* primitives
>> instead. This resulted in unoptimal code for
>> catomic_compare_and_exchange_bool_acq where superfluous
>> CMP was emitted after CMPXCHG, e.g. in arena_get2:
>
> The CMP is a single-thread optimization used not only on arena_get2, but on
> internal locks as well (lll_lock). It checks the TCB multiple_threads field
> to avoid using 'lock' variants if the process is not multithreaded.
>
> Wilco has implemented this optimization in multiple places, including malloc(),
> and in other areas as well, such as random(). However, this can not be easily
> done in a platform agnostic manner on some other places like internal lock
> (lll_lock*) because not all ABIs supports a signal-safe compare and exchange
> instruction.
>
> The question is whether x86 really wants to eliminate this optimization, since
> it still makes a difference for some symbols that always require internal locking.
And Florian has remind me that we already remove this [1], so my remarks is not valid
anymore.
>
> This simplifies the atomic code and allows glibc to consolidate its internal
> atomic implementation even further.
>
>
> I have started to track this last week and my plan was to be more conservative:
> remove all the non-required atomic operation on some targets (like the catomic,
> that is only used on a couple places and not required anymore on malloc code),
> cleanup x86 but keeping this single-thread optimization, move some target to
> use C11 compiler builtins (like powerpc), and then check how to proceed with
> further cleanups.
[1] https://sourceware.org/cgit/glibc/commit/?id=60d5e40ab200033a982a9fd7594a1f83dcdb94a0
>
>
>>
>> 775b8: 48 8d 4a 01 lea 0x1(%rdx),%rcx
>> 775bc: 48 89 d0 mov %rdx,%rax
>> 775bf: 64 83 3c 25 18 00 00 cmpl $0x0,%fs:0x18
>> 775c6: 00 00
>> 775c8: 74 01 je 775cb <arena_get2+0x35b>
>> 775ca: f0 48 0f b1 0d 75 3d lock cmpxchg %rcx,0x163d75(%rip) # 1db348 <narenas>
>> 775d1: 16 00
>> 775d3: 48 39 c2 cmp %rax,%rdx
>> 775d6: 74 7f je 77657 <arena_get2+0x3e7>
>>
>> that now becomes:
>>
>> 775b8: 48 8d 4a 01 lea 0x1(%rdx),%rcx
>> 775bc: 48 89 d0 mov %rdx,%rax
>> 775bf: f0 48 0f b1 0d 80 3d lock cmpxchg %rcx,0x163d80(%rip) # 1db348 <narenas>
>> 775c6: 16 00
>> 775c8: 74 7f je 77649 <arena_get2+0x3d9>
>>
>> OTOH, catomic_decrement does not fallback to atomic_fetch_add (, -1)
>> builtin but to the cmpxchg loop, so the generated code in arena_get2
>> regresses a bit, from using LOCK DECQ insn:
>>
>> 77829: 64 83 3c 25 18 00 00 cmpl $0x0,%fs:0x18
>> 77830: 00 00
>> 77832: 74 01 je 77835 <arena_get2+0x5c5>
>> 77834: f0 48 ff 0d 0c 3b 16 lock decq 0x163b0c(%rip) # 1db348 <narenas>
>> 7783b: 00
>>
>> to a cmpxchg loop:
>>
>> 7783d: 48 8b 0d 04 3b 16 00 mov 0x163b04(%rip),%rcx # 1db348 <narenas>
>> 77844: 48 8d 71 ff lea -0x1(%rcx),%rsi
>> 77848: 48 89 c8 mov %rcx,%rax
>> 7784b: f0 48 0f b1 35 f4 3a lock cmpxchg %rsi,0x163af4(%rip) # 1db348 <narenas>
>> 77852: 16 00
>> 77854: 0f 84 c9 fa ff ff je 77323 <arena_get2+0xb3>
>> 7785a: eb e1 jmp 7783d <arena_get2+0x5cd>
>>
>> Defining catomic_exchange_and_add using __atomic_fetch_add solves the
>> above issue and generates optimal:
>>
>> 77809: f0 48 83 2d 36 3b 16 lock subq $0x1,0x163b36(%rip) # 1db348 <narenas>
>> 77810: 00 01
>>
>> Depending on the target processor, the compiler may emit either
>> 'LOCK ADD/SUB $1, m' or 'INC/DEC $1, m' instruction, due to partial
>> flag register stall issue.
>>
>> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>> Cc: Florian Weimer <fweimer@redhat.com>
>> Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
>> Cc: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
>> Cc: Collin Funk <collin.funk1@gmail.com>
>> Cc: H.J.Lu <hjl.tools@gmail.com>
>> Cc: Carlos O'Donell <carlos@redhat.com>
>> ---
>> sysdeps/x86/atomic-machine.h | 112 ++---------------------------------
>> 1 file changed, 4 insertions(+), 108 deletions(-)
>>
>> diff --git a/sysdeps/x86/atomic-machine.h b/sysdeps/x86/atomic-machine.h
>> index 9b5019c4f1..d5b2d49031 100644
>> --- a/sysdeps/x86/atomic-machine.h
>> +++ b/sysdeps/x86/atomic-machine.h
>> @@ -20,18 +20,13 @@
>> #define _X86_ATOMIC_MACHINE_H 1
>>
>> #include <stdint.h>
>> -#include <tls.h> /* For tcbhead_t. */
>> #include <libc-pointer-arith.h> /* For cast_to_integer. */
>>
>> -#define LOCK_PREFIX "lock;"
>> -
>> #define USE_ATOMIC_COMPILER_BUILTINS 1
>>
>> #ifdef __x86_64__
>> # define __HAVE_64B_ATOMICS 1
>> # define SP_REG "rsp"
>> -# define SEG_REG "fs"
>> -# define BR_CONSTRAINT "q"
>> #else
>> /* Since the Pentium, i386 CPUs have supported 64-bit atomics, but the
>> i386 psABI supplement provides only 4-byte alignment for uint64_t
>> @@ -39,8 +34,6 @@
>> atomics on this platform. */
>> # define __HAVE_64B_ATOMICS 0
>> # define SP_REG "esp"
>> -# define SEG_REG "gs"
>> -# define BR_CONSTRAINT "r"
>> #endif
>> #define ATOMIC_EXCHANGE_USES_CAS 0
>>
>> @@ -49,76 +42,6 @@
>> #define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
>> (! __sync_bool_compare_and_swap (mem, oldval, newval))
>>
>> -
>> -#define __arch_c_compare_and_exchange_val_8_acq(mem, newval, oldval) \
>> - ({ __typeof (*mem) ret; \
>> - __asm __volatile ("cmpl $0, %%" SEG_REG ":%P5\n\t" \
>> - "je 0f\n\t" \
>> - "lock\n" \
>> - "0:\tcmpxchgb %b2, %1" \
>> - : "=a" (ret), "=m" (*mem) \
>> - : BR_CONSTRAINT (newval), "m" (*mem), "0" (oldval), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - ret; })
>> -
>> -#define __arch_c_compare_and_exchange_val_16_acq(mem, newval, oldval) \
>> - ({ __typeof (*mem) ret; \
>> - __asm __volatile ("cmpl $0, %%" SEG_REG ":%P5\n\t" \
>> - "je 0f\n\t" \
>> - "lock\n" \
>> - "0:\tcmpxchgw %w2, %1" \
>> - : "=a" (ret), "=m" (*mem) \
>> - : BR_CONSTRAINT (newval), "m" (*mem), "0" (oldval), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - ret; })
>> -
>> -#define __arch_c_compare_and_exchange_val_32_acq(mem, newval, oldval) \
>> - ({ __typeof (*mem) ret; \
>> - __asm __volatile ("cmpl $0, %%" SEG_REG ":%P5\n\t" \
>> - "je 0f\n\t" \
>> - "lock\n" \
>> - "0:\tcmpxchgl %2, %1" \
>> - : "=a" (ret), "=m" (*mem) \
>> - : BR_CONSTRAINT (newval), "m" (*mem), "0" (oldval), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - ret; })
>> -
>> -#ifdef __x86_64__
>> -# define __arch_c_compare_and_exchange_val_64_acq(mem, newval, oldval) \
>> - ({ __typeof (*mem) ret; \
>> - __asm __volatile ("cmpl $0, %%fs:%P5\n\t" \
>> - "je 0f\n\t" \
>> - "lock\n" \
>> - "0:\tcmpxchgq %q2, %1" \
>> - : "=a" (ret), "=m" (*mem) \
>> - : "q" ((int64_t) cast_to_integer (newval)), \
>> - "m" (*mem), \
>> - "0" ((int64_t) cast_to_integer (oldval)), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - ret; })
>> -# define do_add_val_64_acq(pfx, mem, value) do { } while (0)
>> -#else
>> -# define __arch_c_compare_and_exchange_val_64_acq(mem, newval, oldval) \
>> - ({ __typeof (*mem) ret = *(mem); \
>> - __atomic_link_error (); \
>> - ret = (newval); \
>> - ret = (oldval); \
>> - ret; })
>> -
>> -# define do_add_val_64_acq(pfx, mem, value) \
>> - { \
>> - __typeof (value) __addval = (value); \
>> - __typeof (mem) __memp = (mem); \
>> - __typeof (*mem) __oldval = *__memp; \
>> - __typeof (*mem) __tmpval; \
>> - do \
>> - __tmpval = __oldval; \
>> - while ((__oldval = pfx##_compare_and_exchange_val_64_acq \
>> - (__memp, __oldval + __addval, __oldval)) == __tmpval); \
>> - }
>> -#endif
>> -
>> -
>> /* Note that we need no lock prefix. */
>> #define atomic_exchange_acq(mem, newvalue) \
>> ({ __typeof (*mem) result; \
>> @@ -146,37 +69,10 @@
>> } \
>> result; })
>>
>> -#define __arch_decrement_body(lock, pfx, mem) \
>> - do { \
>> - if (sizeof (*mem) == 1) \
>> - __asm __volatile (lock "decb %b0" \
>> - : "=m" (*mem) \
>> - : "m" (*mem), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - else if (sizeof (*mem) == 2) \
>> - __asm __volatile (lock "decw %w0" \
>> - : "=m" (*mem) \
>> - : "m" (*mem), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - else if (sizeof (*mem) == 4) \
>> - __asm __volatile (lock "decl %0" \
>> - : "=m" (*mem) \
>> - : "m" (*mem), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - else if (__HAVE_64B_ATOMICS) \
>> - __asm __volatile (lock "decq %q0" \
>> - : "=m" (*mem) \
>> - : "m" (*mem), \
>> - "i" (offsetof (tcbhead_t, multiple_threads))); \
>> - else \
>> - do_add_val_64_acq (pfx, mem, -1); \
>> - } while (0)
>> -
>> -#define __arch_decrement_cprefix \
>> - "cmpl $0, %%" SEG_REG ":%P2\n\tje 0f\n\tlock\n0:\t"
>> -
>> -#define catomic_decrement(mem) \
>> - __arch_decrement_body (__arch_decrement_cprefix, __arch_c, mem)
>> +/* ??? Remove when catomic_exchange_and_add
>> + fallback uses __atomic_fetch_add. */
>> +#define catomic_exchange_and_add(mem, value) \
>> + __atomic_fetch_add (mem, value, __ATOMIC_ACQUIRE)
>>
>> /* We don't use mfence because it is supposedly slower due to having to
>> provide stronger guarantees (e.g., regarding self-modifying code). */
>
More information about the Libc-alpha
mailing list