[PATCH] x86: Use more __sync builtins in atomic-machine.h
Uros Bizjak
ubizjak@gmail.com
Mon Sep 1 17:37:42 GMT 2025
V pon., 1. sep. 2025 18:06 je oseba Adhemerval Zanella Netto <
adhemerval.zanella@linaro.org> napisala:
>
>
> On 29/08/25 10:17, Uros Bizjak wrote:
> > Several macros in atomic-machine.h can be directly implemented
> > using the corresponding __sync builtins:
> >
> > atomic_exchange_acq() with __sync_lock_test_and_set ()
> > atomic_add() with __sync_add_and_fetch ()
> > atomic_and() with __sync_and_and_fetch ()
> > atomic_or() with __sync_or_and_fetch ()
> > atomic_full_barrierr() with __sync_synchronize ()
> >
> > Depending on the target processor, the compiler may emit
> > either "LOCK ADD $1, m" or "INC $1, m" instruction for
> > the following two macros, due to partial flag register
> > stalls:
> >
> > atomic_increment() with __sync_add_and_fetch ()
> > atomic_decrement() with __sync_sub_and_fetch ()
> >
> > The compiler is also able to synthesize optimal code for
> > the following macros:
> >
> > atomic_add_negative() with ( __sync_add_and_fetch () < 0 )
> > atomic_add_zero() with ( __sync_add_and_fetch () == 0 )
> > atomic_increment_and_test()
> > with ( __sync_add_and_fetch () == 0 )
>
> There are the old-style atomic macros and we are moving the code to the
> one modeled after C11 atomic (USE_ATOMIC_COMPILER_BUILTINS).
>
> The atomic_exchange_acq is only used to implement atomic_exchange_rel,
> but neither is used anywhere (besides misc/tst-atomic.c). Same for
> atomic_add,
> atomic_add_negative, atomic_add_zero, atomic_decrement_and_test,
> atomic_and,
> and atomic_or.
>
> The only macro that is actually used is atomic_full_barrier() at only one
> place for Linux (nptl/pthread_mutex_setprioceiling.c, Hurd use in other
> places) and maybe we can use a different atomic operation here (a release
> store, as pthread_mutex_lock).
>
> So maybe it would be better to start remove these and other macros for the
> architectures that define USE_ATOMIC_COMPILER_BUILTINS (as x86) and
> reevaluate
> if we can start to use USE_ATOMIC_COMPILER_BUILTINS on the missing one.
>
Florian and I have had some offline discussion about these atomics and came
to the same conclusion. Apparently, catomic macros are unused as well, and
removing also these unused macros shrinks atomic-machine.h considerably. I
have tested the removal patch, and it works for x86, but it really needs a
thorough review from glibc person. Those macros in effect substitute
generic locking primitives, so removals could result in unoptimal code
without breaking the build.
( I am AFK this week, I'll post the patch to remove unused macros for x86
early next week.)
Thanks,
Uros.
>
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: H.J.Lu <hjl.tools@gmail.com>
> > Cc: Florian Weimer <fweimer@redhat.com>
> > Cc: Carlos O'Donell <carlos@redhat.com>
> > ---
> > sysdeps/x86/atomic-machine.h | 123 ++++-------------------------------
> > 1 file changed, 11 insertions(+), 112 deletions(-)
> >
> > diff --git a/sysdeps/x86/atomic-machine.h b/sysdeps/x86/atomic-machine.h
> > index ac59f77e43..b467385c61 100644
> > --- a/sysdeps/x86/atomic-machine.h
> > +++ b/sysdeps/x86/atomic-machine.h
> > @@ -147,32 +147,8 @@
> > #endif
> >
> >
> > -/* Note that we need no lock prefix. */
> > #define atomic_exchange_acq(mem, newvalue) \
> > - ({ __typeof (*mem) result;
> \
> > - if (sizeof (*mem) == 1)
> \
> > - __asm __volatile ("xchgb %b0, %1"
> \
> > - : "=q" (result), "=m" (*mem)
> \
> > - : "0" (newvalue), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 2)
> \
> > - __asm __volatile ("xchgw %w0, %1"
> \
> > - : "=r" (result), "=m" (*mem)
> \
> > - : "0" (newvalue), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 4)
> \
> > - __asm __volatile ("xchgl %0, %1"
> \
> > - : "=r" (result), "=m" (*mem)
> \
> > - : "0" (newvalue), "m" (*mem));
> \
> > - else if (__HAVE_64B_ATOMICS)
> \
> > - __asm __volatile ("xchgq %q0, %1"
> \
> > - : "=r" (result), "=m" (*mem)
> \
> > - : "0" ((int64_t) cast_to_integer (newvalue)),
> \
> > - "m" (*mem));
> \
> > - else
> \
> > - {
> \
> > - result = 0;
> \
> > - __atomic_link_error ();
> \
> > - }
> \
> > - result; })
> > + __sync_lock_test_and_set (mem, newvalue)
> >
> >
> > #define __arch_exchange_and_add_body(lock, pfx, mem, value) \
> > @@ -246,7 +222,7 @@
> > } while (0)
> >
> > # define atomic_add(mem, value) \
> > - __arch_add_body (LOCK_PREFIX, atomic, __arch, mem, value)
> > + __sync_add_and_fetch (mem, value)
> >
> > #define __arch_add_cprefix \
> > "cmpl $0, %%" SEG_REG ":%P3\n\tje 0f\n\tlock\n0:\t"
> > @@ -256,51 +232,11 @@
> >
> >
> > #define atomic_add_negative(mem, value) \
> > - ({ _Bool __result;
> \
> > - if (sizeof (*mem) == 1)
> \
> > - __asm __volatile (LOCK_PREFIX "addb %b2, %0"
> \
> > - : "=m" (*mem), "=@ccs" (__result)
> \
> > - : IBR_CONSTRAINT (value), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 2)
> \
> > - __asm __volatile (LOCK_PREFIX "addw %w2, %0"
> \
> > - : "=m" (*mem), "=@ccs" (__result)
> \
> > - : "ir" (value), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 4)
> \
> > - __asm __volatile (LOCK_PREFIX "addl %2, %0"
> \
> > - : "=m" (*mem), "=@ccs" (__result)
> \
> > - : "ir" (value), "m" (*mem));
> \
> > - else if (__HAVE_64B_ATOMICS)
> \
> > - __asm __volatile (LOCK_PREFIX "addq %q2, %0"
> \
> > - : "=m" (*mem), "=@ccs" (__result)
> \
> > - : "ir" ((int64_t) cast_to_integer (value)),
> \
> > - "m" (*mem));
> \
> > - else
> \
> > - __atomic_link_error ();
> \
> > - __result; })
> > + ( __sync_add_and_fetch (mem, value) < 0 )
> >
> >
> > #define atomic_add_zero(mem, value) \
> > - ({ _Bool __result;
> \
> > - if (sizeof (*mem) == 1)
> \
> > - __asm __volatile (LOCK_PREFIX "addb %b2, %0"
> \
> > - : "=m" (*mem), "=@ccz" (__result)
> \
> > - : IBR_CONSTRAINT (value), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 2)
> \
> > - __asm __volatile (LOCK_PREFIX "addw %w2, %0"
> \
> > - : "=m" (*mem), "=@ccz" (__result)
> \
> > - : "ir" (value), "m" (*mem));
> \
> > - else if (sizeof (*mem) == 4)
> \
> > - __asm __volatile (LOCK_PREFIX "addl %2, %0"
> \
> > - : "=m" (*mem), "=@ccz" (__result)
> \
> > - : "ir" (value), "m" (*mem));
> \
> > - else if (__HAVE_64B_ATOMICS)
> \
> > - __asm __volatile (LOCK_PREFIX "addq %q2, %0"
> \
> > - : "=m" (*mem), "=@ccz" (__result)
> \
> > - : "ir" ((int64_t) cast_to_integer (value)),
> \
> > - "m" (*mem));
> \
> > - else
> \
> > - __atomic_link_error ();
> \
> > - __result; })
> > + ( __sync_add_and_fetch (mem, value) == 0 )
> >
> >
> > #define __arch_increment_body(lock, pfx, mem) \
> > @@ -329,7 +265,7 @@
> > do_add_val_64_acq (pfx, mem, 1);
> \
> > } while (0)
> >
> > -#define atomic_increment(mem) __arch_increment_body (LOCK_PREFIX,
> __arch, mem)
> > +#define atomic_increment(mem) __sync_add_and_fetch (mem, 1)
> >
> > #define __arch_increment_cprefix \
> > "cmpl $0, %%" SEG_REG ":%P2\n\tje 0f\n\tlock\n0:\t"
> > @@ -339,26 +275,7 @@
> >
> >
> > #define atomic_increment_and_test(mem) \
> > - ({ _Bool __result;
> \
> > - if (sizeof (*mem) == 1)
> \
> > - __asm __volatile (LOCK_PREFIX "incb %b0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else if (sizeof (*mem) == 2)
> \
> > - __asm __volatile (LOCK_PREFIX "incw %w0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else if (sizeof (*mem) == 4)
> \
> > - __asm __volatile (LOCK_PREFIX "incl %0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else if (__HAVE_64B_ATOMICS)
> \
> > - __asm __volatile (LOCK_PREFIX "incq %q0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else
> \
> > - __atomic_link_error ();
> \
> > - __result; })
> > + ( __sync_add_and_fetch (mem, 1) == 0 )
> >
> >
> > #define __arch_decrement_body(lock, pfx, mem) \
> > @@ -387,7 +304,7 @@
> > do_add_val_64_acq (pfx, mem, -1);
> \
> > } while (0)
> >
> > -#define atomic_decrement(mem) __arch_decrement_body (LOCK_PREFIX,
> __arch, mem)
> > +#define atomic_decrement(mem) __sync_sub_and_fetch (mem, 1)
> >
> > #define __arch_decrement_cprefix \
> > "cmpl $0, %%" SEG_REG ":%P2\n\tje 0f\n\tlock\n0:\t"
> > @@ -397,24 +314,7 @@
> >
> >
> > #define atomic_decrement_and_test(mem) \
> > - ({ _Bool __result;
> \
> > - if (sizeof (*mem) == 1)
> \
> > - __asm __volatile (LOCK_PREFIX "decb %b0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else if (sizeof (*mem) == 2)
> \
> > - __asm __volatile (LOCK_PREFIX "decw %w0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else if (sizeof (*mem) == 4)
> \
> > - __asm __volatile (LOCK_PREFIX "decl %0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - else
> \
> > - __asm __volatile (LOCK_PREFIX "decq %q0"
> \
> > - : "=m" (*mem), "=@cce" (__result)
> \
> > - : "m" (*mem));
> \
> > - __result; })
> > + ( __sync_sub_and_fetch (mem, 1) == 0 )
> >
> >
> > #define atomic_bit_set(mem, bit) \
> > @@ -496,7 +396,7 @@
> > #define __arch_cprefix \
> > "cmpl $0, %%" SEG_REG ":%P3\n\tje 0f\n\tlock\n0:\t"
> >
> > -#define atomic_and(mem, mask) __arch_and_body (LOCK_PREFIX, mem, mask)
> > +#define atomic_and(mem, mask) __sync_and_and_fetch (mem, mask)
> >
> > #define catomic_and(mem, mask) __arch_and_body (__arch_cprefix, mem,
> mask)
> >
> > @@ -527,14 +427,13 @@
> > __atomic_link_error ();
> \
> > } while (0)
> >
> > -#define atomic_or(mem, mask) __arch_or_body (LOCK_PREFIX, mem, mask)
> > +#define atomic_or(mem, mask) __sync_or_and_fetch (mem, mask)
> >
> > #define catomic_or(mem, mask) __arch_or_body (__arch_cprefix, mem, mask)
> >
> > /* We don't use mfence because it is supposedly slower due to having to
> > provide stronger guarantees (e.g., regarding self-modifying code).
> */
> > -#define atomic_full_barrier() \
> > - __asm __volatile (LOCK_PREFIX "orl $0, (%%" SP_REG ")" ::: "memory")
> > +#define atomic_full_barrier() __sync_synchronize ()
> > #define atomic_read_barrier() __asm ("" ::: "memory")
> > #define atomic_write_barrier() __asm ("" ::: "memory")
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20250901/e0f418d5/attachment-0001.htm>
More information about the Libc-alpha
mailing list