[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[Ned Deily]

Ned Deily  added the comment:


New changeset 7e4e4bd2b8245426fe733f3c57238acf41f17900 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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[Ned Deily]

Change by Ned Deily :


--
pull_requests:  -10503

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10503

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[Ned Deily]

Change by Ned Deily :


--
pull_requests:  -10497

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[Ned Deily]

Ned Deily  added the comment:


New changeset 782e1d537778d93eb4cba1343f71bfc51e7e3c00 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


--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10498

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10497

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:

> (...) 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 my PR 11219.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[Gregory P. Smith]

Gregory P. Smith  added the comment:

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.

--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10455
stage: resolved -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:

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: see bpo-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.

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 9a4758550d96030ee7e7f7c7c68b435db1a2a825 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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[miss-islington]

Change by miss-islington :


--
pull_requests: +10419

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 640ed520dd6a43a8bf470b79542f58b5d57af9de 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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:

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 -o 
Programs/python.o ./Programs/python.c
(...)

=> CFLAGS is respected: I see -O1 in the command line.


With PR 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 -o 
Programs/python.o ./Programs/python.c
(...)

=> CFLAGS is respected: I see -O1 in the command line.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

STINNER Victor  added the comment:

I wrote PR 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 -o 
Programs/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 -o 
Programs/python.o ./Programs/python.c
(...)

=> CFLAGS_NODIST is used: I see "-O1" in the command line.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +10400
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35499] "make profile-opt" overrides CFLAGS_NODIST

[STINNER Victor]

New submission from STINNER Victor :

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 from bpo-23390:

commit 2f90aa63666308e7a9b2d0a89110e0be445a393a
Author: Gregory P. Smith 
Date:   Wed Feb 4 02:11:56 2015 -0800

Fixes issue23390: 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 by bpo-21121:

commit acb8c5234302f8057b331abaafb2cc8697daf58f
Author: Benjamin Peterson 
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 to bpo-35257: "Avoid leaking linker flags into distutils: 
add PY_LDFLAGS_NODIST".

--
components: Build
messages: 331847
nosy: vstinner
priority: normal
severity: normal
status: open
title: "make profile-opt" overrides CFLAGS_NODIST
versions: Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com