classification
Title: Fixing a bug related to LTO only build
Type: enhancement Stage: resolved
Components: Build Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: cstratak, gregory.p.smith, ned.deily, octavian.soldea, vstinner
Priority: normal Keywords:

Created on 2017-09-05 21:54 by octavian.soldea, last changed 2018-12-09 09:08 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3110 merged octavian.soldea, 2017-09-05 22:48
PR 10261 merged cstratak, 2018-10-31 11:49
Messages (11)
msg301381 - (view) Author: Octavian Soldea (octavian.soldea) * Date: 2017-09-05 21:54
Hi All,

This is Octavian Soldea from the Scripting Languages Optimization team at Intel Corporation. I would like to submit a patch to fix an LTO build issue. With "./configure --with-lto" followed by "make", CPython should be built with LTO or Link-Time-Optimization. However, currently, no impact is made to the build process, meaning the flag is not effective. This patch has been created out of a python-dev discussion thread: http://www.mail-archive.com/python-dev@python.org/msg96469.html. The LTO flag was originally introduced by my team member Alecsandru Patrascu back in 2015: https://bugs.python.org/issue25702.

Before the change:

./configure --with-lto
make
gcc command line output example:
gcc -pthread -c -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -I. -I./Include -DPy_BUILD_CORE -o Python/ceval.o Python/ceval.c
Note no "lto" flag was present. This is the same result as the default build without the LTO flag.

After changes:

./configure --with-lto
make
expected gcc command line output example =>
gcc -pthread -c -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -I. -I./Include -DPy_BUILD_CORE -o Python/ceval.o Python/ceval.c

Thanks,
Octavian
msg301385 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-05 22:13
Would you mind to create a pull request?
msg301722 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-09-08 19:14
New changeset 4c81401b3a9ffa48fc9e1ffff9963cbad5111e33 by Gregory P. Smith (octaviansoldea) in branch 'master':
bpo-31354: Let configure --with-lto work on all builds
https://github.com/python/cpython/commit/4c81401b3a9ffa48fc9e1ffff9963cbad5111e33
msg329444 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-07 22:05
New changeset f6b9459996f5166300982d0427119eb326e74dac by Victor Stinner (stratakis) in branch '3.6':
[3.6] bpo-31354: Let configure --with-lto work on all builds (GH-10261)
https://github.com/python/cpython/commit/f6b9459996f5166300982d0427119eb326e74dac
msg329844 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018-11-13 14:19
This change exports -flto in cflags.

You can check it with python3-config --cflags after this commit.

Which means that every c extension which will use those cflags will also utilize -flto which wasn't happening before, thus I'd say it's a regression.
msg329876 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-11-14 00:49
Is it an actual problem to compile extension modules with -flto?  (I realize as an extension module there isn't a huge benefit to the concept unless it happens to have multiple source files or link against a non-shared library)
msg329897 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018-11-14 10:16
I'd say yes. In general python may have been compiled with -flto, but it's still a bit buggy from the compilers' side. It doesn't work well always or at all depending on the toolchain, and even if python was compiled with -flto, that doesn't mean that all c extensions compiled using that python build, would work properly with -flto.

In my opinion that should be a flag reserved for python only, if used for its compilation, and not having it propagated to c extensions, similarly to the PGO related flags.
msg329925 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-11-14 18:17
This does sound like a regression bug that should be fixed in all branches.  Another issue is that, if it is propagated to third-party extension module builds through distutils, there is no guarantee that the extension module is being built with the same compiler as Python itself was.
msg331346 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-12-07 19:43
This issue is still open and blocking the upcoming 3.7.2rc1 and 3.6.8rc1 releases.
msg331351 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018-12-07 20:39
Hi Ned,

I recently pushed a fix on the master and 3.7 for this exact issue: https://bugs.python.org/issue35351 but it builds on top of https://github.com/python/cpython/pull/10922 which is not yet in 3.6.

Thus 3.7 is fine for the rc cutoff.

However there is another side effect of that change with the linker flags also leaking: https://bugs.python.org/issue35257 but it proved to be more complicated than what I initially thought to fix it.

PR here: https://github.com/python/cpython/pull/10900

The issue is that I figured it wouldn't make it to the 3.6 branch as it would require extensive review due to the complexity of the build system and the requirement for extensive review to catch any possible regressions, thus it would be late for the rc cutoff.

It would take some days to figure out everything I believe. Would it be possible to push that after rc1? Or maybe not fixing that would be better, deadline wise? I consider this an important issue but some more feedback is always welcome.

Side note on your comment: The compiler used (CC) also seems to leak into distutils instead of the system default CC, on linux with 'python3 setup.py build'.
msg331418 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-12-09 09:08
OK, I think we are in better shape for 3.6.8 now.  I've merged the backports to 3.6 of Issue28015 / PR 9908 (LTO with clang) and Issue35351 / PR 10797 (prevent LTO CFLAGS leakage into distutils).  So, at the moment, master, 3.7, and 3.6 should behave the same in this area, although I'm a little bit leery of having all this backporting just before 3.6 stops accepting bugfixes.  As far as I can tell, what remains is resolving Issue35257 (prevent LTO LDFLAGS leakage into distutils).

>Side note on your comment: The compiler used (CC) also seems to leak
>into distutils instead of the system default CC, on linux with
>'python3 setup.py build'.

Yes, it does by design as do many other of the common build Makefile / environment variables.  The point I was trying to make applies more to binary distributions of Python, like with the python.org macOS installer.  In cases like that, the binary distribution is designed to install and run on multiple OS versions and that means that the compiler, and OS version for that matter, used to build the python executable and standard library extension modules are often very different from those on the end user's system when building third-party extension modules.
History
Date User Action Args
2018-12-09 09:08:30ned.deilysetstatus: open -> closed
priority: release blocker -> normal
messages: + msg331418

resolution: fixed
stage: needs patch -> resolved
2018-12-07 20:39:04cstrataksetmessages: + msg331351
2018-12-07 19:43:23ned.deilysetmessages: + msg331346
2018-11-14 23:35:38gregory.p.smithsetversions: + Python 3.6, Python 3.8
2018-11-14 23:35:08gregory.p.smithsetpriority: normal -> release blocker
2018-11-14 18:17:18ned.deilysetstatus: closed -> open

nosy: + ned.deily
messages: + msg329925

resolution: fixed -> (no value)
stage: resolved -> needs patch
2018-11-14 10:16:26cstrataksetmessages: + msg329897
2018-11-14 00:49:31gregory.p.smithsetmessages: + msg329876
2018-11-13 14:19:39cstrataksetnosy: + cstratak
messages: + msg329844
2018-11-07 22:05:16vstinnersetmessages: + msg329444
2018-10-31 11:49:48cstrataksetpull_requests: + pull_request9572
2017-09-08 19:16:29gregory.p.smithsetstatus: open -> closed
assignee: gregory.p.smith
resolution: fixed
stage: resolved
2017-09-08 19:14:37gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg301722
2017-09-05 22:48:57octavian.soldeasetpull_requests: + pull_request3376
2017-09-05 22:13:35vstinnersetnosy: + vstinner
messages: + msg301385
2017-09-05 21:54:13octavian.soldeacreate