classification
Title: Support ICC in configure
Type: enhancement Stage: resolved
Components: Build Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alecsandru.patrascu, brett.cannon, doko, python-dev, r.david.murray, skrah, zach.ware
Priority: normal Keywords: patch

Created on 2015-12-09 07:45 by zach.ware, last changed 2015-12-22 03:06 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
configure_icc.diff zach.ware, 2015-12-09 07:45 review
configure_icc.v2.diff zach.ware, 2015-12-18 21:07 review
Messages (6)
msg256141 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-12-09 07:45
Attached is a patch that adds support for ICC to configure, making it easier to build with ICC on Unix and adjusting arguments to better fit ICC.

- Adds '--with-icc' argument to configure, which sets 'CC=icc' and 'CXX=icpc'
- Adds support for ICC PGO
- Prevents '-Wno-unused-result' from being added to CFLAGS if CC=icc (issue24709)
- Adds '-fp-model strict' to BASECFLAGS (I'm not 100% sure that's the right place, perhaps CFLAGS_NODIST would be better?).  Adding '-fp-model strict' clears up all the failures on the Ubuntu ICC Non-Debug buildbot, and most of the failures I get on OSX with -O3.  From a quick run of the 'math' benchmarks (perf.py -b math), adding '-fp-model strict' does not hurt performance (with the flag was always faster or negligibly slower).

Be sure to run autoreconf before testing.
msg256183 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-12-10 23:27
This patch looks correct to me, but I don't have much experience with configure.  Are the .dyn files icc's profiling output?

I'm adding Alecsandru and Brett as nosy since they worked on the PGO stuff.  I'm also adding Stefan since he's shown interest in icc.
msg256282 - (view) Author: Alecsandru Patrascu (alecsandru.patrascu) * Date: 2015-12-12 12:16
Thank you David for including me in this issue. 

On ICC, when executing the instrumented applications, it will generate dynamic information file that has a unique name and .dyn suffix. From my previous experience with GCC and CLANG PGO patch (issue24915), also the output of the autoconf will have to be included in the final diff.

Regarding the place for adding the '-fp-model strict' flags, I had the same dillema when working on the LTO patch (issue25702), because I needed the flags to be propagated to _all_ compilation and linking phases, and finally decided to add them to the CONFIGURE_CFLAGS, CONFIGURE_CFLAGS_NODIST, CONFIGURE_CPPFLAGS, CONFIGURE_LDFLAGS variables in the Makefile.pre.in file. You can try there also and keep the BASECFLAGS clean .
msg256708 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-12-18 21:07
After testing, I think I like '-fp-model strict' better in CFLAGS_NODIST.  On Ubuntu, `CC=gcc ./python -m test.test_distutils` passes when the flag is in CFLAGS_NODIST, but fails when it's in BASECFLAGS.  On OSX, it fails both ways, due to deployment target issues; I suspect that's probably solvable by setting some other environment variable correctly (I don't know enough about OSX to say for sure, though).  I don't think this patch needs to go as far as the LTO patch does to add the argument to CFLAGS; most third party extensions probably won't necessitate '-fp-model strict', those that do can add it themselves.

On the other hand, 2.7 doesn't have CFLAGS_NODIST (and I don't want to add it), so BASECFLAGS is about the only option there.

Barring objections, I'll commit this new patch in a few days.
msg256807 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-21 20:18
New changeset d7b5c2f99a99 by Zachary Ware in branch '2.7':
Issue #25827: Add support for ICC to configure
https://hg.python.org/cpython/rev/d7b5c2f99a99

New changeset c5e419464585 by Zachary Ware in branch '3.5':
Issue #25827: Add support for ICC to configure
https://hg.python.org/cpython/rev/c5e419464585

New changeset 29ea3827cfaa by Zachary Ware in branch 'default':
Issue #25827: Merge with 3.5
https://hg.python.org/cpython/rev/29ea3827cfaa
msg256822 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2015-12-22 03:06
The buildbots appear to be happy with this, so I'm closing the issue.  The intel-ubuntu-icc Non-Debug builders each had their first green build after this changeset.  The OSX ICC builder still has some issues, but they seem to be OSX specific (see issue25589).
History
Date User Action Args
2015-12-22 03:06:32zach.waresetstatus: open -> closed
resolution: fixed
messages: + msg256822

stage: patch review -> resolved
2015-12-21 20:18:08python-devsetnosy: + python-dev
messages: + msg256807
2015-12-18 21:07:24zach.waresetfiles: + configure_icc.v2.diff
nosy: + doko
messages: + msg256708

2015-12-12 12:16:19alecsandru.patrascusetmessages: + msg256282
2015-12-10 23:27:14r.david.murraysetnosy: + brett.cannon, skrah, alecsandru.patrascu
messages: + msg256183
2015-12-09 07:45:54zach.warecreate