Skip to content

bpo-35351: Pass link time optimization flags to CFLAGS_NODIST. #10797

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 4, 2018

Conversation

stratakis
Copy link
Contributor

@stratakis stratakis commented Nov 29, 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.

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.
@@ -6626,7 +6626,7 @@ $as_echo "$as_me: llvm-ar found via xcrun: ${LLVM_AR}" >&6;}
LTOFLAGS="$LTOFLAGS -g"
fi

BASECFLAGS="$BASECFLAGS $LTOFLAGS"
CFLAGS_NODIST="$CFLAGS_NODIST $LTOFLAGS"
LDFLAGS="$LDFLAGS $LTOFLAGS"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@serge-sans-paille
Copy link
Contributor

serge-sans-paille commented Nov 30, 2018 via email

@serge-sans-paille
Copy link
Contributor

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

@vstinner
Copy link
Member

vstinner commented Dec 3, 2018

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."

@vstinner
Copy link
Member

vstinner commented Dec 4, 2018

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.

@vstinner vstinner merged commit f92c7aa into python:master Dec 4, 2018
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-10893 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 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 <[email protected]>
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 <[email protected]>
@miss-islington
Copy link
Contributor

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 <[email protected]>
@bedevere-bot
Copy link

GH-11046 is a backport of this pull request to the 3.6 branch.

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 <[email protected]>
@stratakis stratakis deleted the flto-fix branch June 18, 2020 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants