bpo-35351: Pass link time optimization flags to CFLAGS_NODIST. by stratakis · Pull Request #10797 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation10 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

When using link time optimizations, the -flto flag is passed to
BASECFLAGS, which makes it propagate to distutils. Those flags
should be reserved for the interpreter and the stdlib extension
modules only, thus moving those flags to CFLAGS_NODIST.

https://bugs.python.org/issue35351

When using link time optimizations, the -flto flag is passed to BASECFLAGS, which makes it propagate to distutils. Those flags should be reserved for the interpreter and the stdlib extension modules only, thus moving those flags to CFLAGS_NODIST.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LDFLAGS are not forwarded like BASECFLAGS?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They actually are, this PR fixes the issue partially. In order to separate the LDFLAGS as well I plan to introduce LDFLAGS_NODIST, on a different PR if that one is merged, but it will require bigger scoping.

ok, makes sense. I'll run an lto based build with clang this morning to confirm it's still valid.

@stratakis build works fine on my config with lto + clang

Please add a NEWS entry in the Build category. Say something like "When building Python with clang and LTO, LTO flags are no longer passed into CFLAGS to build third-party C extensions."

I tested manually the change:

$ git clean -fdx
$./configure_debug CC=clang --with-lto --with-pydebug
$ make
$ bash ./python-config --cflags
-I/home/vstinner/prog/python/include/python3.8dm -I/home/vstinner/prog/python/include/python3.8dm  -Wno-unused-result -Wsign-compare -O0 -g -O0 -Wall

Output without the change:

-I/home/vstinner/prog/python/include/python3.8dm -I/home/vstinner/prog/python/include/python3.8dm  -flto -g -Wno-unused-result -Wsign-compare -O0 -g -O0 -Wall

I confirm that "-flto" is gone from "./python-config --cflags" output.

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Dec 4, 2018

…GH-10797)

When using link time optimizations, the -flto flag is passed to BASECFLAGS, which makes it propagate to distutils. Those flags should be reserved for the interpreter and the stdlib extension modules only, thus moving those flags to CFLAGS_NODIST. (cherry picked from commit f92c7aa)

Co-authored-by: stratakis cstratak@redhat.com

miss-islington added a commit that referenced this pull request

Dec 4, 2018

When using link time optimizations, the -flto flag is passed to BASECFLAGS, which makes it propagate to distutils. Those flags should be reserved for the interpreter and the stdlib extension modules only, thus moving those flags to CFLAGS_NODIST. (cherry picked from commit f92c7aa)

Co-authored-by: stratakis cstratak@redhat.com

Thanks @stratakis for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Dec 9, 2018

…GH-10797)

When using link time optimizations, the -flto flag is passed to BASECFLAGS, which makes it propagate to distutils. Those flags should be reserved for the interpreter and the stdlib extension modules only, thus moving those flags to CFLAGS_NODIST. (cherry picked from commit f92c7aa)

Co-authored-by: stratakis cstratak@redhat.com

miss-islington added a commit that referenced this pull request

Dec 9, 2018

When using link time optimizations, the -flto flag is passed to BASECFLAGS, which makes it propagate to distutils. Those flags should be reserved for the interpreter and the stdlib extension modules only, thus moving those flags to CFLAGS_NODIST. (cherry picked from commit f92c7aa)

Co-authored-by: stratakis cstratak@redhat.com