Skip to content
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

PGO: compiler warnings for source files with no profile data #117804

Closed
erlend-aasland opened this issue Apr 12, 2024 · 4 comments
Closed

PGO: compiler warnings for source files with no profile data #117804

erlend-aasland opened this issue Apr 12, 2024 · 4 comments
Labels
build The build process and cross-build

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Apr 12, 2024

PGO builds do not generated profile data for all source files. This means that for a PGO build, you may end up with compiler warnings for source files with no profile data. Do we want to silence these warnings? If we do, it may be harder to spot if PGO builds fail to include profile data correctly.

Spotted while working on #117752.

Linked PRs

@erlend-aasland
Copy link
Contributor Author

Example warning:

gcc -bundle -undefined dynamic_lookup      Modules/syslogmodule.o   -o Modules/syslog.cpython-313-darwin.so
gcc  -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -fprofile-instr-use=/Users/erlend.aasland/src/python/build-main/code.profclangd -I../main/Include/internal -I../main/Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../main/Include     -c ../main/Modules/termios.c -o Modules/termios.o
warning: no profile data available for file "termios.c" [-Wprofile-instr-unprofiled]

@zooba
Copy link
Member

zooba commented Apr 12, 2024

Unless they're actually disruptive (breaking builds), I'd rather leave them there by default.

Seems easy enough to add the suppression to CFLAGS manually, yeah? Should we just doc it?

@zooba
Copy link
Member

zooba commented Apr 12, 2024

It's one of these cases where if we suppress them once, we'll forget about coverage entirely, which is worse than simply continuing to not fix it (someone might be motivated to fix it in the future if they see it)

@erlend-aasland
Copy link
Contributor Author

Seems easy enough to add the suppression to CFLAGS manually, yeah? Should we just doc it?

I would be fine with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build
Projects
None yet
Development

No branches or pull requests

2 participants