Skip to content

bpo-37415: Fix stdatomic.h header check for ICC compiler#16717

Merged
vstinner merged 1 commit intopython:masterfrom
vstinner:icc_atomic
Oct 22, 2019
Merged

bpo-37415: Fix stdatomic.h header check for ICC compiler#16717
vstinner merged 1 commit intopython:masterfrom
vstinner:icc_atomic

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Oct 11, 2019 •
edited by bedevere-bot
Loading

Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

https://bugs.python.org/issue37415

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Oct 11, 2019

I removed ATOMIC_VAR_INIT() check from configure, since it's not used in Python.

@vstinner
bpo-37415: Fix stdatomic.h header check for ICC compiler
87ed388 
Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
@adamjstewart adamjstewart mentioned this pull request Oct 21, 2019
Merged
@adamjstewart
Copy link
Copy Markdown
Contributor

adamjstewart commented Oct 21, 2019

This should also fix https://bugs.python.org/issue35473

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Oct 21, 2019

@adamjstewart: Can you test this PR with ICC?

@adamjstewart
Copy link
Copy Markdown
Contributor

adamjstewart commented Oct 21, 2019

I'm still working on getting access to our cluster, but once I have access I can definitely test this PR!

@adamjstewart
Copy link
Copy Markdown
Contributor

adamjstewart commented Oct 21, 2019

I assume the runstatedir portions of the patch aren't actually needed to fix the intel build?

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Oct 21, 2019

I assume the runstatedir portions of the patch aren't actually needed to fix the intel build?

I'm running "autoconf". Depending who run "autoconf" to generate the latest configure in script and depending on my autoconf version, the runstatedir thing may or may not change. I never understood how to get a reliable output using autotools.

@adamjstewart
Copy link
Copy Markdown
Contributor

adamjstewart commented Oct 22, 2019

This patch seems to be working so far. I tried running the unit tests and only saw a few failures:

Ran 327 tests in 205.461s

OK (skipped=30)
4 tests failed again:
    test_asyncio test_distutils test_gdb test_imaplib

== Tests result: FAILURE then FAILURE ==

399 tests OK.

4 tests failed:
    test_asyncio test_distutils test_gdb test_imaplib

13 tests skipped:
    test_curses test_devpoll test_kqueue test_msilib test_ossaudiodev
    test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
    test_winreg test_winsound test_zipfile64

7 re-run tests:
    test_asyncio test_distutils test_gdb test_imaplib
    test_multiprocessing_fork test_multiprocessing_forkserver
    test_multiprocessing_spawn

Total duration: 20 min 47 sec
Tests result: FAILURE then FAILURE
make: *** [test] Error 2

I'm going to try to install anyway.

@adamjstewart
Copy link
Copy Markdown
Contributor

adamjstewart commented Oct 22, 2019

The install succeeded! With the minimal patch found here, I was able to build Python 3.7.4 with Intel 18.0.3.222 on Cray CNL5. The patch successfully applies to Python 3.6.7-3.6.8, 3.7.1-3.7.4, and 3.8.0 release tarballs. We may want to backport this to Python 3.6 if it's still supported. I believe this bug was likely introduced in Python 3.6.7, 3.7.1, and 3.8.0. Thanks for your help @vstinner!

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Oct 22, 2019

4 tests failed: test_asyncio test_distutils test_gdb test_imaplib

That's unrelated to https://bugs.python.org/issue37415

https://bugs.python.org/issue37415 is only about a compilation failure when using ICC.

@vstinner vstinner merged commit 028f734 into python:master Oct 22, 2019
@vstinner vstinner deleted the icc_atomic branch October 22, 2019 19:53
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Oct 22, 2019

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Oct 22, 2019

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Oct 22, 2019

GH-16892 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 22, 2019
@vstinner @miss-islington
bpo-37415: Fix stdatomic.h header check for ICC compiler (pythonGH-16717
f53a006 
)

Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
(cherry picked from commit 028f734)

Co-authored-by: Victor Stinner <[email protected]>
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Oct 22, 2019

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 22, 2019
@vstinner @miss-islington
bpo-37415: Fix stdatomic.h header check for ICC compiler (pythonGH-16717
0b5b972 
)

Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
(cherry picked from commit 028f734)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Oct 22, 2019

GH-16893 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Oct 22, 2019
@miss-islington @vstinner
bpo-37415: Fix stdatomic.h header check for ICC compiler (GH-16717)
b102e4f 
Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
(cherry picked from commit 028f734)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request Oct 22, 2019
@miss-islington @vstinner
bpo-37415: Fix stdatomic.h header check for ICC compiler (GH-16717)
dbcea39 
Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
(cherry picked from commit 028f734)

Co-authored-by: Victor Stinner <[email protected]>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@vstinner
bpo-37415: Fix stdatomic.h header check for ICC compiler (pythonGH-16717
ff0d623 
)

Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@vstinner @shihai1991
bpo-37415: Fix stdatomic.h header check for ICC compiler (pythonGH-16717
fa09d16 
)

Fix stdatomic.h header check for ICC compiler: the ICC implementation
lacks atomic_uintptr_t type which is needed by Python.

Test:

* atomic_int and atomic_uintptr_t types
* atomic_load_explicit() and atomic_store_explicit()
* memory_order_relaxed and memory_order_seq_cst constants

But don't test ATOMIC_VAR_INIT(): it's not used in Python.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner @adamjstewart @miss-islington @bedevere-bot @the-knights-who-say-ni