
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2018-12-14 18:25 byvstinner, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 11164 | merged | vstinner,2018-12-14 18:38 | |
| PR 11179 | merged | miss-islington,2018-12-16 22:00 | |
| PR 11219 | closed | vstinner,2018-12-18 21:05 | |
| PR 11267 | merged | vstinner,2018-12-20 16:23 | |
| Messages (10) | |||
|---|---|---|---|
| msg331847 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-14 18:25 | |
Makefile.pre.in contains the rule:build_all_generate_profile:$(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS) $(PGO_PROF_GEN_FLAG)" LDFLAGS="$(LDFLAGS) $(PGO_PROF_GEN_FLAG)" LIBS="$(LIBS)"I'm not sure that CFLAGS_NODIST="$(CFLAGS) $(PGO_PROF_GEN_FLAG)" is correct: it overrides user $CFLAGS_NODIST variable. I suggest to replace it with CFLAGS_NODIST="$(CFLAGS_NODIST) $(PGO_PROF_GEN_FLAG)": add $(PGO_PROF_GEN_FLAG) to CFLAGS_NODIST, don't copy $CFLAGS to $CFLAGS_NODIST (and add $(PGO_PROF_GEN_FLAG)).The code comes frombpo-23390:commit2f90aa63666308e7a9b2d0a89110e0be445a393aAuthor: Gregory P. Smith <greg@krypto.org>Date: Wed Feb 4 02:11:56 2015 -0800 Fixesissue23390: make profile-opt causes -fprofile-generate and related flags to end up in distutils CFLAGS.(...) build_all_generate_profile:- $(MAKE) all CFLAGS="$(CFLAGS) -fprofile-generate" LIBS="$(LIBS) -lgcov"+ $(MAKE) all CFLAGS_NODIST="$(CFLAGS) -fprofile-generate" LDFLAGS="-fprofile-generate" LIBS="$(LIBS) -lgcov"(...)CFLAGS_NODIST has been added bybpo-21121:commitacb8c5234302f8057b331abaafb2cc8697daf58fAuthor: Benjamin Peterson <benjamin@python.org>Date: Sat Aug 9 20:01:49 2014 -0700 add -Werror=declaration-after-statement only to stdlib extension modules (closes#21121) Patch from Stefan Krah.This issue is related tobpo-35257: "Avoid leaking linker flags into distutils: add PY_LDFLAGS_NODIST". | |||
| msg331851 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-14 18:53 | |
I wrotePR 11164 to fix the issue.Example:$ git clean -fdx$ ./configure --with-pydebug$ make profile-opt CFLAGS_NODIST="-O1"(...)gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fprofile-generate -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -oPrograms/python.o ./Programs/python.c(...)=> CFLAGS_NODIST is missing: I don't see -O1 in the command line.With my change:$ git clean -fdx$ ./configure --with-pydebug$ make profile-opt CFLAGS_NODIST="-O1"(...)gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -O1 -fprofile-generate -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -oPrograms/python.o ./Programs/python.c(...)=> CFLAGS_NODIST is used: I see "-O1" in the command line. | |||
| msg331856 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-14 19:27 | |
I also tested CFLAGS, just in case.Current behavior:$ git clean -fdx$ ./configure --with-pydebug$ make profile-opt CFLAGS="-O1"(...)gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -O1 -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fprofile-generate -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -oPrograms/python.o ./Programs/python.c(...)=> CFLAGS is respected: I see -O1 in the command line.WithPR 11164:$ git clean -fdx$ ./configure --with-pydebug$ make profile-opt CFLAGS="-O1"(...)gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -O1 -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fprofile-generate -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -oPrograms/python.o ./Programs/python.c(...)=> CFLAGS is respected: I see -O1 in the command line. | |||
| msg331930 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-16 17:00 | |
New changeset640ed520dd6a43a8bf470b79542f58b5d57af9de by Victor Stinner in branch 'master':bpo-35499: make profile-opt don't override CFLAGS_NODIST (GH-11164)https://github.com/python/cpython/commit/640ed520dd6a43a8bf470b79542f58b5d57af9de | |||
| msg331940 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-16 22:24 | |
New changeset9a4758550d96030ee7e7f7c7c68b435db1a2a825 by Victor Stinner (Miss Islington (bot)) in branch '3.7':bpo-35499: make profile-opt don't override CFLAGS_NODIST (GH-11164) (GH-11179)https://github.com/python/cpython/commit/9a4758550d96030ee7e7f7c7c68b435db1a2a825 | |||
| msg332079 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-18 21:03 | |
Oh wait, "make build_all_generate_profile" and "make profile-opt" have another issue. They modify LDFLAGS, whereas PGO flags seem to be very specific to the compiler, not to the linker.I reopen the issue.build_all_generate_profile:$(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS_NODIST) $(PGO_PROF_GEN_FLAG)" LDFLAGS="$(LDFLAGS) $(PGO_PROF_GEN_FLAG)" LIBS="$(LIBS)"profile-opt: profile-run-stamp...$(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS_NODIST) $(PGO_PROF_USE_FLAG)" LDFLAGS="$(LDFLAGS)"LDFLAGS="$(LDFLAGS) $(PGO_PROF_GEN_FLAG)" of "make build_all_generate_profile" looks harmless: passing PGO flags to the linker works since gcc is used as the linker. Except that LDFLAGS is exported and used by distutils, and passing PGO flags to build third party code is not ok: seebpo-35257.For "make profile-opt", LDFLAGS="$(LDFLAGS)" looks useless.PGO flags depend on the compiler:* clang * PGO_PROF_GEN_FLAG="-fprofile-instr-generate" * PGO_PROF_USE_FLAG="-fprofile-instr-use=code.profclangd"* gcc: * PGO_PROF_GEN_FLAG="-fprofile-instr-generate" * PGO_PROF_USE_FLAG="-fprofile-instr-use=code.profclangd"* ICC: * PGO_PROF_GEN_FLAG="-prof-gen" * PGO_PROF_USE_FLAG="-prof-use"* Default: * PGO_PROF_GEN_FLAG="-fprofile-generate" * PGO_PROF_USE_FLAG="-fprofile-use -fprofile-correction"I don't think that any of these flags should be passed to the LDFLAGS. Passing these flags to CFLAGS should be enough. | |||
| msg332104 -(view) | Author: Gregory P. Smith (gregory.p.smith)*![]() | Date: 2018-12-19 00:24 | |
But the `build_all_generate_profile` build is an intermediate instrumented interpreter build, it isn't shipped and things like PGO often require flags that the linker sees in order to generate the instrumented binary.If those are left off of the link step, you won't have an instrumented binary and won't generate profile data. | |||
| msg332134 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2018-12-19 12:21 | |
> (...) things like PGO often require flags that the linker sees in order to generate the instrumented binary. If those are left off of the link step, you won't have an instrumented binary and won't generate profile data.Oh, I didn't try my PR...$ ./configure --enable-optimizations$ make.../usr/bin/ld: libpython3.8m.a(myreadline.o):(.data+0xa0): undefined reference to `__gcov_merge_add'...My PR simply doesn't work: we have to pass PGO flags to the linker. At least for the first step generating a profile.My bad, sorry, I close myPR 11219. | |||
| msg332253 -(view) | Author: Ned Deily (ned.deily)*![]() | Date: 2018-12-20 19:46 | |
New changeset782e1d537778d93eb4cba1343f71bfc51e7e3c00 by Ned Deily (Victor Stinner) in branch '3.6':bpo-35499: make profile-opt don't override CFLAGS_NODIST (GH-11164) (GH-11267)https://github.com/python/cpython/commit/782e1d537778d93eb4cba1343f71bfc51e7e3c00 | |||
| msg332486 -(view) | Author: Ned Deily (ned.deily)*![]() | Date: 2018-12-24 16:32 | |
New changeset7e4e4bd2b8245426fe733f3c57238acf41f17900 by Ned Deily (Miss Islington (bot)) in branch '3.7':bpo-35499: make profile-opt don't override CFLAGS_NODIST (GH-11164) (GH-11179)https://github.com/python/cpython/commit/7e4e4bd2b8245426fe733f3c57238acf41f17900 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:09 | admin | set | github: 79680 |
| 2018-12-24 16:32:11 | ned.deily | set | messages: +msg332486 |
| 2018-12-20 20:34:02 | ned.deily | set | pull_requests: -pull_request10503 |
| 2018-12-20 20:31:24 | vstinner | set | pull_requests: +pull_request10503 |
| 2018-12-20 20:15:16 | ned.deily | set | pull_requests: -pull_request10497 |
| 2018-12-20 19:46:11 | ned.deily | set | nosy: +ned.deily messages: +msg332253 |
| 2018-12-20 16:23:03 | vstinner | set | pull_requests: +pull_request10498 |
| 2018-12-20 14:18:34 | vstinner | set | pull_requests: +pull_request10497 |
| 2018-12-19 12:22:20 | vstinner | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2018-12-19 12:21:49 | vstinner | set | messages: +msg332134 |
| 2018-12-19 00:24:10 | gregory.p.smith | set | nosy: +gregory.p.smith messages: +msg332104 |
| 2018-12-18 21:05:17 | vstinner | set | stage: resolved -> patch review pull_requests: +pull_request10455 |
| 2018-12-18 21:03:29 | vstinner | set | status: closed -> open resolution: fixed -> (no value) messages: +msg332079 |
| 2018-12-18 00:52:33 | vstinner | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2018-12-16 22:24:09 | vstinner | set | messages: +msg331940 |
| 2018-12-16 22:00:20 | miss-islington | set | pull_requests: +pull_request10419 |
| 2018-12-16 17:00:46 | vstinner | set | messages: +msg331930 |
| 2018-12-14 19:27:36 | vstinner | set | messages: +msg331856 |
| 2018-12-14 18:53:30 | vstinner | set | messages: +msg331851 |
| 2018-12-14 18:38:51 | vstinner | set | keywords: +patch stage: patch review pull_requests: +pull_request10400 |
| 2018-12-14 18:25:57 | vstinner | create | |