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.

Sign up for GitHub

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

gh-90536: Add support for the BOLT post-link binary optimizer #95908

Merged
merged 11 commits into from
Aug 18, 2022
Merged

gh-90536: Add support for the BOLT post-link binary optimizer #95908

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef8e98d
Add support for the BOLT post-link binary optimizer
kmod Aug 10, 2022
1448a68
Simplify the build flags
kmod Aug 12, 2022
c546374
Add a NEWS entry
kmod Aug 12, 2022
c12dbea
Update Makefile.pre.in
kmod Aug 16, 2022
ce25757
Update configure.ac
kmod Aug 16, 2022
ded38f0
Add myself to ACKS
kmod Aug 16, 2022
cc17806
Add docs
kmod Aug 16, 2022
0050190
Other review comments
kmod Aug 16, 2022
83da8c4
fix tab/space issue
kmod Aug 16, 2022
7f14cd1
Make it more clear that --enable-bolt is experimental
kmod Aug 17, 2022
f0f5e53
Add link to bolt's github page
kmod Aug 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,15 @@ profile-opt: profile-run-stamp
-rm -f profile-clean-stamp
$(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS_NODIST) $(PGO_PROF_USE_FLAG)" LDFLAGS_NODIST="$(LDFLAGS_NODIST)"

bolt-opt: @PREBOLT_RULE@
rm -f *.fdata
@LLVM_BOLT@ $(BUILDPYTHON) -instrument -instrumentation-file-append-pid -instrumentation-file=$(abspath $(BUILDPYTHON).bolt) -o $(BUILDPYTHON).bolt_inst
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@LLVM_BOLT@ $(BUILDPYTHON) -instrument -instrumentation-file-append-pid -instrumentation-file=$(abspath $(BUILDPYTHON).bolt) -o $(BUILDPYTHON).bolt_inst
@LLVM_BOLT@ ./$(BUILDPYTHON) -instrument -instrumentation-file-append-pid -instrumentation-file=$(abspath $(BUILDPYTHON).bolt) -o $(BUILDPYTHON).bolt_inst

./$(BUILDPYTHON).bolt_inst $(PROFILE_TASK) || true
@MERGE_FDATA@ $(BUILDPYTHON).*.fdata > $(BUILDPYTHON).fdata
@LLVM_BOLT@ $(BUILDPYTHON) -o $(BUILDPYTHON).bolt -data=$(BUILDPYTHON).fdata -update-debug-sections -reorder-blocks=ext-tsp -reorder-functions=hfsort+ -split-functions=3 -icf=1 -inline-all -split-eh -reorder-functions-use-hot-size -peepholes=all -jump-tables=aggressive -inline-ap -indirect-call-promotion=all -dyno-stats -use-gnu-stack -frame-opt=hot
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@LLVM_BOLT@ $(BUILDPYTHON) -o $(BUILDPYTHON).bolt -data=$(BUILDPYTHON).fdata -update-debug-sections -reorder-blocks=ext-tsp -reorder-functions=hfsort+ -split-functions=3 -icf=1 -inline-all -split-eh -reorder-functions-use-hot-size -peepholes=all -jump-tables=aggressive -inline-ap -indirect-call-promotion=all -dyno-stats -use-gnu-stack -frame-opt=hot
@LLVM_BOLT@ ./$(BUILDPYTHON) -o $(BUILDPYTHON).bolt -data=$(BUILDPYTHON).fdata -update-debug-sections -reorder-blocks=ext-tsp -reorder-functions=hfsort+ -split-functions=3 -icf=1 -inline-all -split-eh -reorder-functions-use-hot-size -peepholes=all -jump-tables=aggressive -inline-ap -indirect-call-promotion=all -dyno-stats -use-gnu-stack -frame-opt=hot

rm -f *.fdata
mv $(BUILDPYTHON).bolt $(BUILDPYTHON)

# Compile and run with gcov
.PHONY=coverage coverage-lcov coverage-report
coverage:
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2022-08-12-13-06-03.gh-issue-90536.qMpF6p.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Use the BOLT post-link optimizer to improve performance, particularly on
medium-to-large applications.
259 changes: 259 additions & 0 deletions configure
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,57 @@ if test "$Py_LTO" = 'true' ; then
LDFLAGS_NODIST="$LDFLAGS_NODIST $LTOFLAGS"
fi

# Enable bolt flags
Py_BOLT='false'
AC_MSG_CHECKING(for --enable-bolt)
AC_ARG_ENABLE(bolt, AS_HELP_STRING(
[--enable-bolt],
[enable usage of the llvm-bolt post-link optimizer (default is no)]),
[
if test "$enableval" != no
then
Py_BOLT='true'
AC_MSG_RESULT(yes);
else
Py_BOLT='false'
AC_MSG_RESULT(no);
fi],
[AC_MSG_RESULT(no)])

AC_SUBST(PREBOLT_RULE)
if test "$Py_BOLT" = 'true' ; then
PREBOLT_RULE="${DEF_MAKE_ALL_RULE}"
DEF_MAKE_ALL_RULE="bolt-opt"
DEF_MAKE_RULE="build_all"

# These flags are required for bolt to work:
CFLAGS_NODIST="$CFLAGS_NODIST -fno-reorder-blocks-and-partition"
LDFLAGS_NODIST="$LDFLAGS_NODIST -Wl,--emit-relocs"

# These flags are required to get good performance from bolt:
CFLAGS_NODIST="$CFLAGS_NODIST -fno-pie"
# We want to add these no-pie flags to linking executables but not shared libraries:
LINKCC="$LINKCC -fno-pie -no-pie"
Copy link
Contributor

Choose a reason for hiding this comment

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

@kmod are those flags required for Bolt to have proper impact? Asking as in Fedora we link all the packages with -pie and there is a conflict with this flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it's been quite a while and I don't really recall. I'd guess that I added them because the answer was yes, but I think the bolt team was actively working on improving pie support at the time so there's a good chance that the answer could have changed since the time of this PR.

Copy link

Choose a reason for hiding this comment

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

Yes, the support for PIE binaries with computed gotos is not merged yet: llvm/llvm-project#120267

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that be resolved via https://github.com/python/cpython/pull/128511/files though? Hence the -no-pie and -fno-pie flags would be redundant here?

Copy link

Choose a reason for hiding this comment

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

Yes, what you've linked is a workaround. With that, you can enable PIE. Once that PR I've linked is merged, you can drop skip-funcs and hopefully enjoy better performance.


AC_SUBST(LLVM_BOLT)
AC_PATH_TOOL(LLVM_BOLT, llvm-bolt, '', ${llvm_path})
if test -n "${LLVM_BOLT}" -a -x "${LLVM_BOLT}"
then
AC_MSG_RESULT("Found llvm-bolt")
else
AC_MSG_ERROR([llvm-bolt is required for a --enable-bolt build but could not be found.])
fi

AC_SUBST(MERGE_FDATA)
AC_PATH_TOOL(MERGE_FDATA, merge-fdata, '', ${llvm_path})
if test -n "${MERGE_FDATA}" -a -x "${MERGE_FDATA}"
then
AC_MSG_RESULT("Found merge-fdata")
else
AC_MSG_ERROR([merge-fdata is required for a --enable-bolt build but could not be found.])
fi
fi

# Enable PGO flags.
AC_SUBST(PGO_PROF_GEN_FLAG)
AC_SUBST(PGO_PROF_USE_FLAG)
Expand Down