The glib library uses atomic for the first argument. Maybe it's a better name than address or pointer, what do you think?

I slightly prefer address or ptr rather than atomic

• https://en.cppreference.com/w/c/atomic uses obj name.
• https://www.ibm.com/docs/en/zos/2.1.0?topic=c11-atomic-load also uses obj name (C11 functions)
• pycore_atomic.h uses ATOMIC_VAL name
• pycore_atomic_func.h uses var name

I dislike the address name. What you pass is not a void* pointer or an uintptr_t address, but a reference to an atomic variable. I suggest to use atomic or obj name.

For add operation, the C11 API uses arg for the second parameter name. I'm fine with value.

I don't think obj makes as much sense in this API because, unlike the C11 API, they are not pointers to atomic objects (e.g., _Atomic type). They are just pointers to plain C variables. I think that's an important distinction.

If you dislike address, I can change it to ptr, which we commonly use and is a bit more concise.

For context, the GCC documentation for built-in atomic functions uses ptr. This is the API that pyatomic.h most closely matches.

https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

Not sure it's worth exposing atomic ops for all int sizes and signednesses?

I expect to use at least one atomic operation on each of the data types here (but not every atomic op on every data type). I tried to be consistent on what's defined because it makes understanding what's available easier and testing easier.

C11 passes expected as a pointer, so that it's updated with the actual value when the latter doesn't match the former. Why not keep that convention here?

The motivation was two-fold: First, many of the _Py_atomic_compare_exchange calls in the nogil fork use constants as expected and this would be more verbose if it needs to be a pointer, e.g.:

_Py_atomic_compare_exchange_uint8(&m->v, LOCKED, UNLOCKED)

vs.

uint8_t expected = LOCKED:
_Py_atomic_compare_exchange_uint8(&m->v, &expected, UNLOCKED)

Second, I find this style (no pointer for expected) to be a bit less error-prone. I've been tripped up once or twice by having expected be modified when I didn't expect it.

I don't feel terribly strongly about this, so if there is a general preference for sticking closer to the C11-style API here, I can change it.

AFAIU the main motivation for the C11 style APIs is for retry loops in lockless data structure implementations. A simplistic example (this may be embarassingly wrong):

struct ListNode;

typedef struct ListNode {
  int value;
  struct ListNode* next;
} ListNode;

void ListAppend(ListNode* list, int new_value) {
  ListNode* new_node = (ListNode*) malloc(sizeof ListNode);
  new_node->value = new_value;
  new_node->next = NULL;
  ListNode* expected = NULL;
  while (_Py_atomic_compare_exchange_ptr(&list->next, &expected, new_node)) {
    list = expected;
    expected = NULL;
  }
}

If the second argument is a reference (&expected), does it mean that it changes the value of the second argument (*expected)?

For C11 atomic_exchange(), the second argument is not a pointer, but a value (integer), no? https://en.cppreference.com/w/c/atomic/atomic_exchange

C11 atomic_compare_exchange_strong() and atomic_compare_exchange_weak() use a pointer for expected. But this API writes into *expected if the *obj is not equal to *expected.

The behavior of atomic_compare_exchange_* family is as if the following was executed atomically:

if (memcmp(obj, expected, sizeof *obj) == 0) {
    memcpy(obj, &desired, sizeof *obj);
    return true;
} else {
    memcpy(expected, obj, sizeof *obj);
    return false;
}

For this header fie, I would prefer to not have two flavors, the API is already quite long! I would prefer to have a single flavor. If there is an usecase where setting expected is relevant, I suggest to use a pointer for the second argument.

In short, I agree to change the API to int _Py_atomic_compare_exchange_int32(int32_t *obj, int32_t *expected, int32_t desired). The behavior should be well documented.

obj, expected and desired names come from the C11 API.

I'll make the second argument a pointer like the C11 API.

Should it be void** address for clarity?

The problem is that void ** requires an explicit cast for almost every use, because things like PyObject ** are not implicitly convertible to void **. In other words, currently we can write things like:

PyObject *old_exc = _Py_atomic_exchange_ptr(&tstate->async_exc, exc);

but if address was void **address, we'd have to write:

PyObject *old_exc = _Py_atomic_exchange_ptr((void **)&tstate->async_exc, exc);

A large part of the Python C API uses macro to convert arguments to PyObject*. Would it make sense to do the same here?

#define _Py_atomic_exchange_ptr(atomic, value) _Py_atomic_exchange_ptr(_Py_CAST(void**, atomic), (value))

I don't see the benefit of that style over the current approach, and it would silently allow passing some integer types to _Py_atomic_exchange_ptr.

Right, in that case I'm fine with the surprising void* type.

I'm not an expert, but why is it useful to expose "release" operations if no "acquire" operations are exposed?

You can always use stronger orderings for correctness (i.e., "seq_cst" everywhere instead of "acquire"). From a performance view, "release" is substantially faster than "seq_cst" stores on x86/x86-64, but "acquire" generates the same code as "seq_cst" loads on both x86/x86-64 and aarch64.

I don't think CPython support is limited to x86 and ARM variants, so the set of atomic ops exposed should probably be made consistent nevertheless?

Also, using "seq_cst" in combination with "release" will probably make the code more difficult to reason about, than if "acquire" is exposed (and memory ordering is already hard to reason about!).

I'll add a _Py_atomic_load_ptr_acquire for consistency (and I think I can remove the _Py_atomic_store_uint64_release).

I would prefer a more elaborated documentation, "fence" is kind of weak.

__faststorefence doc:

Guarantees that every previous memory reference, including both load and store memory references, is globally visible before any subsequent memory reference.

_ReadWriteBarrier doc:

Limits the compiler optimizations that can reorder memory accesses across the point of the call.

dmb ish:

The data memory barrier ensures that all preceding writes are issued before any subsequent memory operations (including speculative memory access).

The challenge I have is that it's really hard to describe what fences do in a way that's helpful and accurate. The above documentation is too strong for C11 fences.

There's https://en.cppreference.com/w/c/atomic/atomic_thread_fence, but I find it vague. And the C++ documentation (https://en.cppreference.com/w/cpp/atomic/atomic_thread_fence) is more detailed but really hard to understand.

Please add one or two of these links here. It's ok to have references to external doc, it's better than no doc :-)