Implement atomic operations on non-x86 platforms

[VitordeLima]

BPO	22038
Nosy	@vstinner, @koobs
Files	atomic.patch: A patch implemeting atomic operating using the GCC builtins atomicv2.patch: A patch implemeting atomic operating using the GCC builtins and the stdatomic.h header atomicv3.patch atomicv4.patch atomicv5.patch atomic_pointer.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2014-07-22.17:24:12.307>
labels = ['interpreter-core', 'type-feature']
title = 'Implement atomic operations on non-x86 platforms'
updated_at = <Date 2015-03-17.21:59:03.297>
user = 'https://bugs.python.org/VitordeLima'

bugs.python.org fields:

activity = <Date 2015-03-17.21:59:03.297>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2014-07-22.17:24:12.307>
creator = 'Vitor.de.Lima'
dependencies = []
files = ['36035', '36151', '37500', '37694', '37704', '37706']
hgrepos = []
issue_num = 22038
keywords = ['patch']
message_count = 25.0
messages = ['223676', '223740', '224235', '232813', '232836', '232839', '232901', '233706', '233707', '233745', '233942', '233944', '233955', '233958', '233959', '233960', '233991', '233993', '234023', '234029', '234030', '234031', '234037', '237945', '238348']
nosy_count = 9.0
nosy_names = ['vstinner', 'Arfrever', 'neologix', 'python-dev', 'koobs', 'John.Malmberg', 'Vitor.de.Lima', 'gustavotemple', 'lbianc']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22038'
versions = ['Python 3.4', 'Python 3.5']

[VitordeLima]

The atomic operations listed in the pyatomic.h header file were implemented only for the x86 architecture, this patch uses the atomic bultins available in GCC >= 4.7 to implement such operations, allowing it to work properly in other platforms.

[vstinner]

+ #define __ATOMIC_RELAXED 0

You should use the "_Py_" prefix for these constants, to avoid conflicts in applications.

(You may also replace tabs with spaces, the PEP-7 says "Use 4-space indents and no tabs at all." but I also prefer to avoid tabs in other places.)

I tested your patch on Fedora 20 (Linux kernel 3.14.8, GCC 4.8.2, glibc 2.18) on x86_64 ("Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz") and the whole Python test suite pass.

GCC 4.9 (released a few month ago) provides the <stdatomic.h> header:
https://gcc.gnu.org/gcc-4.9/changes.html

pyatomic.h contains this comment:

/* XXX: When compilers start offering a stdatomic.h with lock-free
atomic_int and atomic_address types, include that here and rewrite
the atomic operations in terms of it. */

But using <stdatomic.h> header can be done in a separated issue.

[VitordeLima]

Implemented a new version of the patch using either gcc builtins or the stdatomic.h header (this is detected by the configure script).

[gustavotemple]

ping

[vstinner]

Fedora 20 provides GCC 4.7 but no stdatomic.c: HAVE_BUILTIN_ATOMIC set (HAVE_STD_ATOMIC unset).

Fedora 21 provides GCC 4.9 with stdatomic.c: HAVE_BUILTIN_ATOMIC and HAVE_STD_ATOMIC are set.

I tested atomicv2.patch on Fedora 20 (x86_64) and Fedora 21 (x86_64): the whole test suite pass.

[vstinner]

atomicv2.patch:

_Atomic int _value;

Why not using the atomic_int type from stdatomic.h here?

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

"__atomic_store_n(): The valid memory model variants are __ATOMIC_RELAXED, __ATOMIC_SEQ_CST, and __ATOMIC_RELEASE."

I understand that _Py_atomic_store_explicit() only accept some values for order. An assertion should be added here, maybe for any implementation. Something like:

#define _Py_atomic_store_explicit(ATOMIC_VAL, NEW_VAL, ORDER) \
    (assert((ORDER) == __ATOMIC_RELAXED                       \
            || (ORDER) == __ATOMIC_SEQ_CST                    \
            || (ORDER) == __ATOMIC_RELEASE),                  \
     __atomic_store_n(&(ATOMIC_VAL)->_value, NEW_VAL, ORDER))

Same remark for _Py_atomic_load_explicit():

"__atomic_load_n(): The valid memory model variants are __ATOMIC_RELAXED, __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE, and __ATOMIC_CONSUME."

[gustavotemple]

@Haypo, done: atomicv3.patch

[python-dev]

New changeset fbe87fb071a6 by Victor Stinner in branch 'default':
Issue bpo-22038: pyatomic.h now uses stdatomic.h or GCC built-in functions for
https://hg.python.org/cpython/rev/fbe87fb071a6

[vstinner]

atomicv3.patch is wrong for GCC builtin atomic operations: the parenthesis is not closed. I fixed this typo in the commit.

Vitor & Gustavo: Thanks for the patch, it's now applied to Python 3.5.

I tested it on Fedora 21 (x86_64). I disabled manually HAVE_STD_ATOMIC in pyconfig.h to test the two new options (stdatomic header & GCC builtins).

[gustavotemple]

Thank you, Victor!

[vstinner]

I reopen the issue because Python cannot be compiled anymore on the Builder AMD64 FreeBSD 10.0 3.x:

http://buildbot.python.org/all/builders/AMD64%20FreeBSD%2010.0%203.x/builds/2947/steps/configure/logs/stdio

checking for stdatomic.h... yes
checking for GCC >= 4.7 __atomic builtins... yes

http://buildbot.python.org/all/builders/AMD64%20FreeBSD%2010.0%203.x/builds/2947/steps/compile/logs/stdio

--- Parser/tokenizer_pgen.o ---
In file included from Parser/tokenizer_pgen.c:2:
In file included from Parser/tokenizer.c:4:
In file included from Include/Python.h:53:
Include/pyatomic.h:37:5: error: _Atomic cannot be applied to incomplete type 'void'
_Atomic void *_value;
^