build: add --enable-lto configuration option
#23152
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Concept ACK, conditional on that we are going to try make this be default for the release binaries (I don't mean in this PR). I'm generally not a fan of configure options that simply add cflags/linkerflags, but I think that's something for which to make an exception.
Agree. Function/data garbage collection is mostly an executable size concern, it doesn't, besides possibly better (and sometimes worse!) cache access patterns, affect performance. |
I've added EDIT: Looks like this will need some changes to the Guix toolchains. |
|
Is there a possibility to use -flto=jobserver? That makes GCC use make's parallel scheduler (so the argument to -j is available for multiple parallel compilations). It requires the linker's command in Makefile to be prepended with +. Alternatively, some way of setting N for -flto=N. Doing the entire LTO stage single-threadedly (the default, I think) would be very slow, especially on machines with lots of cores. Of course, none of this is a requirement to evaluate whether we want LTO or not. |
Which reminds me—should we enable lto for the depends build as well? I guess it's another separate decision, but it would allow for the most optimization opportunities. |
|
Concept ACK Some LTO results from measurements made back in 2018 can be found in #14277 :) |
|
Concept ACK |
|
There seems to be a patch for automake to support -flto=jobserver, but it's not yet accepted: https://www.mail-archive.com/automake-patches@gnu.org/msg07973.html |
|
I ran all of the benchmarks with clang++ 12.0.1, g++ 11.1.0 with and without
|
|
@martinus Man, that's a wildly inconsistent set of differences... I don't see anything really dramatic, though. |
|
I've now calculated the geometric mean of all the benchmark results:
So on average clang++ seems to benefit in the benchmarks, but for g++ the change is not significant. I'd say one needs to do more real world benchmarks to see if it's a benefit. One other interesting observeration: When compiling with The |
Great find @martinus! Enabling more intelligent compiler reasoning is a very good reason for LTO :) After some testing I can confirm that the uninitialized read above is reachable from testing code. Can be verified by applying the following patch: diff --git a/src/random.cpp b/src/random.cpp
index 174f4cef3..73e946783 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -693,13 +693,30 @@ FastRandomContext::FastRandomContext(bool fDeterministic) noexcept : requires_se
rng.SetKey(seed.begin(), 32);
}
+// Force Valgrind use-of-uninitialized memory (UUM) violation if `o` is uninitialized.
+//
+// As suggested by @guidovranken in https://github.com/bitcoin/bitcoin/issues/22064
+template<typename T>
+void ForceValgrindWarningIfUninitialized(const T& o) {
+ static_assert(std::is_trivially_copyable<T>::value);
+ FILE* f = fopen("/dev/null", "wb");
+ fwrite(&o, sizeof(o), 1, f);
+ fclose(f);
+}
+
FastRandomContext& FastRandomContext::operator=(FastRandomContext&& from) noexcept
{
+ ForceValgrindWarningIfUninitialized(from.requires_seed);
requires_seed = from.requires_seed;
+ ForceValgrindWarningIfUninitialized(from.rng);
rng = from.rng;
+ ForceValgrindWarningIfUninitialized(from.bytebuf);
std::copy(std::begin(from.bytebuf), std::end(from.bytebuf), std::begin(bytebuf));
+ ForceValgrindWarningIfUninitialized(from.bytebuf_size);
bytebuf_size = from.bytebuf_size;
+ ForceValgrindWarningIfUninitialized(from.bitbuf);
bitbuf = from.bitbuf;
+ ForceValgrindWarningIfUninitialized(from.bitbuf_size);
bitbuf_size = from.bitbuf_size;
from.requires_seed = true;
from.bytebuf_size = 0;And running |
|
I ran a few Running
Running
My takeaways:
|
martinus
added a commit
to martinus/bitcoin
that referenced
this pull request
Oct 4, 2021
Building with `--enable-lto`in bitcoin#23152 produced a `-Wmaybe-uninitialized` warning in FastRandomContext. This commit initializes all members where they are defined. The `FastRandomContext(const uint256& seed)` constructor does not initialize the `uint64_t bitbuf` member, and in `FastRandomContext& operator=(FastRandomContext&& from) noexcept;` the uninitialized variable is copied with `bitbuf = from.bitbuf;` which technically is undefined behavior. This happens in `setup_common.cpp` in the line `ctx = FastRandomContext(seed);`.
martinus
added a commit
to martinus/bitcoin
that referenced
this pull request
Oct 4, 2021
Building in bitcoin#23152 produces two -Wmaybe-uninitialized warnings in the fuzz code. As far as I can say these are false positives, but initializing them doesn't hurt and gets rid of the warnings.
0f9fccc to
7a47588
Compare
|
@martinus thanks for the testing so far. For more benchmarking / testing, I've added an additional commit which adds a I've also added a commit that will partially fix using LTO in the Guix builds. After discussing with @dongcarl , we've realized that using the |
maflcko
reviewed
Oct 5, 2021
contrib/guix/libexec/build.sh
Outdated
| @@ -237,7 +237,7 @@ mkdir -p "$OUTDIR" | |||
| ########################### | |||
|
|
|||
| # CONFIGFLAGS | |||
| CONFIGFLAGS="--enable-reduce-exports --disable-bench --disable-gui-tests --disable-fuzz-binary" | |||
| CONFIGFLAGS="--enable-lto --enable-reduce-exports --disable-bench --disable-gui-tests --disable-fuzz-binary" | |||
There was a problem hiding this comment.
If no runtime benefit can be observed with recent gcc, it would be surprising to see a benefit of lto with gcc-8. Lto might not be worth it for the release binaries unless they are also switched to use clang?
There was a problem hiding this comment.
If it's not slower, something could still be said for smaller binaries.
There was a problem hiding this comment.
And I think what would still be interesting is to build the depends with -lto too, so that calls into dependencies can be optimized, and this might be able to shave off big parts of Qt.
|
For GCC 8 it may be useful to try enabling |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
ACK 7a47588 I tested/benched both The differences I saw for a mid-chain 40,000 block IBD weren't as dramatic as I expected based on the results above. The dbcache here is the default (pretty low at 300), so I'll try rerunning with a much higher dbcache to see if that makes a difference. In any case, this changeset obviously works. Worth noting that the assumevalid value (000000000000000000176c192f42ad13ab159fdb20198b87e7ba3c001e47b876) I use here straddles the run, since its height is 522,000. Edit: just to be clear, these results show that LTO (or, really, clang-compiled binaries) are somehow marginally slower in this test than gcc. Kinda weird?
lto-clang-12 vs. thin-lto-clang-12 vs. gcc-9 (master) vs. clang-12 (master) (absolute)
lto-clang-12 vs. thin-lto-clang-12 vs. gcc-9 (master) vs. clang-12 (master) (relative)
|
|
I think the numbers are actually not too different from mine. lto-clang-12 uses the least amount of CPU, and gcc-9 the most. I think that's the important metrics here, the actual runtime difference could be due to random chance (e.g. waiting for leveldb writes). |
|
7a47588 to
c621df2
Compare
I've added this. In newer GCC, in regards to this option I see:
Looks like it was removed when Also added
I've added In regards to Guix. The Linux HOSTS are all building. The macOS build is failing (cross-compiling outside Guix works). See full output here: Undefined symbols for architecture x86_64:
"RandomInit()", referenced from:
_main in lto.o
"SetupChainParamsBaseOptions(ArgsManager&)", referenced from:
_main in lto.o
"ArgsManager::AddArg(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned int, OptionsCategory const&)", referenced from:
_main in lto.o
"ArgsManager::AddCommand(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)", referenced from:
_main in lto.o
"HelpRequested(ArgsManager const&)", referenced from:
_main in lto.o
"SetupHelpOptions(ArgsManager&)", referenced from:
_main in lto.o
"FormatFullVersion()", referenced from:
_main in lto.o
"SetupEnvironment()", referenced from:
_main in lto.o
<trim>The mingw-w64 is failing with: /bin/sh ../libtool --tag=CXX --preserve-dup-deps --mode=link x86_64-w64-mingw32-g++ -std=c++17 -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -flto -fPIE -pipe -O2 -O2 -g -fno-ident -fno-extended-identifiers -fvisibility=hidden -Wl,--exclude-libs,ALL -Wl,--enable-reloc-section -Wl,--dynamicbase -Wl,--nxcompat -Wl,--high-entropy-va -pie -flto -all-static -pthread -lpthread -L/bitcoin/depends/x86_64-w64-mingw32/lib -Wl,--no-insert-timestamp -Wl,--major-subsystem-version -Wl,6 -Wl,--minor-subsystem-version -Wl,1 -o bitcoin-cli.exe bitcoin_cli-bitcoin-cli.o bitcoin-cli-res.o libbitcoin_cli.a univalue/libunivalue.la libbitcoin_util.a crypto/libbitcoin_crypto_base.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a crypto/libbitcoin_crypto_shani.a -L/bitcoin/depends/x86_64-w64-mingw32/lib -lboost_system-mt-s-x64 -lboost_filesystem-mt-s-x64 -L/bitcoin/depends/x86_64-w64-mingw32/lib -levent -lws2_32 -lssp -liphlpapi -lshlwapi -lws2_32 -ladvapi32 -luuid -loleaut32 -lole32 -lcomctl32 -lshell32 -lwinmm -lcomdlg32 -lgdi32 -luser32 -lkernel32
libtool: link: x86_64-w64-mingw32-g++ -std=c++17 -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -flto -fPIE -pipe -O2 -O2 -g -fno-ident -fno-extended-identifiers -fvisibility=hidden -Wl,--exclude-libs -Wl,ALL -Wl,--enable-reloc-section -Wl,--dynamicbase -Wl,--nxcompat -Wl,--high-entropy-va -pie -flto -static -pthread -Wl,--no-insert-timestamp -Wl,--major-subsystem-version -Wl,6 -Wl,--minor-subsystem-version -Wl,1 -o bitcoin-cli.exe bitcoin_cli-bitcoin-cli.o bitcoin-cli-res.o -lpthread -L/bitcoin/depends/x86_64-w64-mingw32/lib libbitcoin_cli.a univalue/.libs/libunivalue.a libbitcoin_util.a crypto/libbitcoin_crypto_base.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a crypto/libbitcoin_crypto_shani.a -lboost_system-mt-s-x64 -lboost_filesystem-mt-s-x64 -levent -lws2_32 /gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/x86_64-w64-mingw32/lib/libssp.a -liphlpapi -lshlwapi -lws2_32 -ladvapi32 -luuid -loleaut32 -lole32 -lcomctl32 -lshell32 -lwinmm -lcomdlg32 -lgdi32 -luser32 -lkernel32 -pthread
x86_64-w64-mingw32-ld: libbitcoin_util_a-request.o (symbol from plugin):(.gnu.linkonce.t._ZNSt6vectorINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESaIS5_EEaSERKS7_+0x0): multiple definition of `std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator=(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/vector.tcc:186: first defined here
x86_64-w64-mingw32-ld: libbitcoin_util_a-request.o (symbol from plugin):(.gnu.linkonce.t._ZNSt6vectorI8UniValueSaIS0_EEaSERKS2_+0x0): multiple definition of `std::vector<UniValue, std::allocator<UniValue> >::operator=(std::vector<UniValue, std::allocator<UniValue> > const&)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/vector.tcc:186: first defined here
x86_64-w64-mingw32-ld: libbitcoin_util_a-system.o (symbol from plugin):(.gnu.linkonce.t._ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_8UniValueESt10_Select1stIS9_ESt4lessIS5_ESaIS9_EE24_M_get_insert_unique_posERS7_+0x0): multiple definition of `std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue> > >::_M_get_insert_unique_pos(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/stl_tree.h:2051: first defined here
x86_64-w64-mingw32-ld: libbitcoin_util_a-system.o (symbol from plugin):(.gnu.linkonce.t._ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_8UniValueESt10_Select1stIS9_ESt4lessIS5_ESaIS9_EE8_M_eraseEPSt13_Rb_tree_nodeIS9_E+0x0): multiple definition of `std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue> > >::_M_erase(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, UniValue> >*)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/stl_tree.h:1873: first defined here
x86_64-w64-mingw32-ld: libbitcoin_util_a-system.o (symbol from plugin):(.gnu.linkonce.t._ZNSt6vectorI8UniValueSaIS0_EE17_M_realloc_insertIJRKS0_EEEvN9__gnu_cxx17__normal_iteratorIPS0_S2_EEDpOT_+0x0): multiple definition of `void std::vector<UniValue, std::allocator<UniValue> >::_M_realloc_insert<UniValue const&>(__gnu_cxx::__normal_iterator<UniValue*, std::vector<UniValue, std::allocator<UniValue> > >, UniValue const&)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/vector.tcc:413: first defined here
x86_64-w64-mingw32-ld: libbitcoin_util_a-settings.o (symbol from plugin):(.gnu.linkonce.t._ZNSt6vectorI8UniValueSaIS0_EE15_M_range_insertIN9__gnu_cxx17__normal_iteratorIPKS0_S2_EEEEvNS5_IPS0_S2_EET_SB_St20forward_iterator_tag+0x0): multiple definition of `void std::vector<UniValue, std::allocator<UniValue> >::_M_range_insert<__gnu_cxx::__normal_iterator<UniValue const*, std::vector<UniValue, std::allocator<UniValue> > > >(__gnu_cxx::__normal_iterator<UniValue*, std::vector<UniValue, std::allocator<UniValue> > >, __gnu_cxx::__normal_iterator<UniValue const*, std::vector<UniValue, std::allocator<UniValue> > >, __gnu_cxx::__normal_iterator<UniValue const*, std::vector<UniValue, std::allocator<UniValue> > >, std::forward_iterator_tag)'; univalue/.libs/libunivalue.a(libunivalue_la-univalue.o):/gnu/store/c1mhzb9f961bdc9z4jbsn44c8fk2an5y-gcc-cross-x86_64-w64-mingw32-8.4.0/include/c++/bits/vector.tcc:672: first defined here
collect2: error: ld returned 1 exit status |
c621df2 to
d33237e
Compare
Guix builds
|
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com> Co-authored-by: Elichai Turkel <elichai.turkel@gmail.com>
d33237e to
68e5aaf
Compare
|
I've rebased this, and reduced the changes back to just adding the I plan on adding proper LTO support to depends as a follow up. |
|
Code review ACK 68e5aaf |
|
I tried running a build with With gcc 9.3.0 I get the following error: |
martinus
added a commit
to martinus/bitcoin
that referenced
this pull request
Nov 18, 2021
Building with `--enable-lto`in bitcoin#23152 produced a `-Wmaybe-uninitialized` warning in FastRandomContext. This commit fixes the move constructor: There was undefined behavior due to uninitialized copy, and uninitialized reads in bytebuf. The `FastRandomContext(const uint256& seed)` constructor did not initialize the `uint64_t bitbuf` member, and in `FastRandomContext& operator=(FastRandomContext&& from) noexcept;` the uninitialized variable was copied with `bitbuf = from.bitbuf;` which technically is undefined behavior. This happens in `setup_common.cpp` in the line `ctx = FastRandomContext(seed);`.
Builds with Apple Clang are working ok, but I see the same issue with LLVM Clang 13. Will take a look.
Interesting. Anything non-standard about your build? I've completed builds using this branch with GCC 10.3.0 and 9.3.0. |
The issue is using We should be able to override the |
I've retried on Ubuntu 20.04 with just No flag overrides. The only thing special isthat it's an out-of-tree build. It still fails in the linking step. It could still be the environment I can retry in a fresh VM. Edit: that works, fine, will try to find out what is different. |
|
This currently works for GCC 9.x+, no change to release builds, still completely opt in. I'm planning on following up with improved support for Clang, and depends support shortly, but am going to merge this now. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Nov 25, 2021
68e5aaf build: add `--enable-lto` configuration option (fanquake) Pull request description: It's been 5 years since using LTO was first suggested for use when building Bitcoin Core, and it's time to revisit it again. Compilers, and their LTO implementations, have matured, and Bitcoin Core has come a long way in terms of pruning dependencies which may have proved troublesome (i.e Boost previously had issues when using LTO). We'll have even less Boost code after moving to `std::filesystem` (bitcoin#20744). Experimenting with LTO came up on IRC last night: > sipa: jamesob: i'm interested in knowing whether "-flto" and/or "-fdata-sections -ffunction-sections -Wl,--gc-sections" are possible/beneficial with our current compiler suite; what would be a good way to have your test infrastructure benchmark things? So this PR just adds the bare minimum to make it easier to configure, compile and perform some bench-marking using `-flto`. This PR doesn't do anything depends wise, however if we decide this is what we want to do, I'll expand the changes here. I had previously had a PR open (bitcoin#18605) to perform link time garbage collection (`-ffunction-sections -fdata-sections` & `-Wl,--gc-sections`), however moving straight to using LTO would be preferable. Note that our minimum required set of compilers, GCC 8.1 and Clang 7, all support the `-flto` option. Related bitcoin#18579. Previous discussion: bitcoin#10616, bitcoin#14277. Previous related PRs: bitcoin#10800 (`-flto`), bitcoin#16791 (ThinLTO). Guix build: ```bash bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum 1f3a7c5be4169aaa444b481d3e65a7bb72da9007fee6e6c416ded2e70f97374b guix-build-68e5aafde3e8/output/aarch64-linux-gnu/SHA256SUMS.part fa8f4cf223d9aaf0b2c1ef55ce61256a19cd1ad7f42b99d0b98c9a52fe6ad8ba guix-build-68e5aafde3e8/output/aarch64-linux-gnu/bitcoin-68e5aafde3e8-aarch64-linux-gnu-debug.tar.gz 9a9967078cd1849b4e85db619e1f55d305c6d44e9e013067c0e8d62c1ba54087 guix-build-68e5aafde3e8/output/aarch64-linux-gnu/bitcoin-68e5aafde3e8-aarch64-linux-gnu.tar.gz 18c71f30722102baaf3dfda67f7c7aac38723510b142e8df8ee7063c5d499368 guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/SHA256SUMS.part 0854cc0d17c045a118df2a24e4cf36d727e7e7e2dea37c2492ee21b71cb79b4b guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/bitcoin-68e5aafde3e8-arm-linux-gnueabihf-debug.tar.gz 215256897dde4e8412ed60473376c694a80c5479fb08039107fb62435f2816ef guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/bitcoin-68e5aafde3e8-arm-linux-gnueabihf.tar.gz 5fad0d9d12bc514ec46ed5d66fd29b7da1376a4a69c3b692936f1ab2356e2f85 guix-build-68e5aafde3e8/output/dist-archive/bitcoin-68e5aafde3e8.tar.gz 4f32989d4ab1946048ca7caee9a983fa875be262282562f5a3e040f4bf92158e guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/SHA256SUMS.part ae45df309ae8ada52891efac0a369a69fed4ab93847a7bc4150a62230df4c8d7 guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/bitcoin-68e5aafde3e8-powerpc64-linux-gnu-debug.tar.gz 0ced227de15cb578567131271e2effe80681b4d7a436c92bf1caec735a576fa4 guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/bitcoin-68e5aafde3e8-powerpc64-linux-gnu.tar.gz 26fc5d2ccc1bc17ee0a146cacada6f4909d90c136ae640c8337332adce414ee0 guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/SHA256SUMS.part 9956b544d90a62a8ba9fc9dc6b6b7f0efe193357332ec19e88053a89d4aab37e guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/bitcoin-68e5aafde3e8-powerpc64le-linux-gnu-debug.tar.gz be8e39ceea1d36086ce5fa93bfb138c68d3bdf0dd6950b192dfa27a65cce3836 guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/bitcoin-68e5aafde3e8-powerpc64le-linux-gnu.tar.gz a7755edc394972885c4c77a7798007e5ba4126b177c4ff6224275c4fb8f3b1c4 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/SHA256SUMS.part b6d252993d8aae7582ad6385fe53c61c54c284c68ece6cb2b2d1ac9554e06139 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/bitcoin-68e5aafde3e8-riscv64-linux-gnu-debug.tar.gz bb4860f3bbd815f800333124ff901d880741792ab47097f49bda3a6931144da0 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/bitcoin-68e5aafde3e8-riscv64-linux-gnu.tar.gz 3dd17deed5c5935fb28b62dfc7afca5caab0d67862cdcbf3337edae73e1d0c4c guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/SHA256SUMS.part fa2d68c54fda0816188c81ce2201a77340b82645da2ffe412526f92c297a82df guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx-unsigned.dmg f6e5accdcd201f522b6426e4d8cc9b3643d4d43a57d268fa0e79ea9a34cfac01 guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx-unsigned.tar.gz 4e5a127df957d1c73b65925d685f6620e7bc5667efcb6dcd98be76effc22fc12 guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx64.tar.gz 56ccd216a69acafacbdc6bae0bdcc1faa50b6a51be1aebfa7068206c88b3241a guix-build-68e5aafde3e8/output/x86_64-linux-gnu/SHA256SUMS.part 77b93dd5fad322636853e5b0244ffafd97cc97f3b4b4ee755d5f830b75d77d13 guix-build-68e5aafde3e8/output/x86_64-linux-gnu/bitcoin-68e5aafde3e8-x86_64-linux-gnu-debug.tar.gz 1feda932fc127b900316a232432b91e46e57ee12a81e12a7d888fdc3296219c1 guix-build-68e5aafde3e8/output/x86_64-linux-gnu/bitcoin-68e5aafde3e8-x86_64-linux-gnu.tar.gz aa7c53ab4164b3736049065c3c24391fc5bd7f26b4bda4aa877c378f0636a125 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/SHA256SUMS.part 5e76148e67aef7e91e70074bfadc08e94373449ac3b966f4343b04d230c778fd guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win-unsigned.tar.gz 34123e3d818beeb70113caeda66945bc7cb9d9e987515d5b149bd17b4b38da90 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64-debug.zip 2bba7f40a2b23c6ea3d47c4f564ab54201bf27f7f57103a98cc9bceea4e70c4d guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64-setup-unsigned.exe 0e7e124144af4a92a4344cf70a3b7c06fbd2b8782aee7ede7263893afa3a5ef0 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64.zip ``` ACKs for top commit: laanwj: Code review ACK 68e5aaf Tree-SHA512: 5c25249cc178b9d54159e268390c974b739df9458d773e23c14b14d808f87f7afe314058b3c068601a9132042321973b0c9b6f81becb925665eca2738ae9a613
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's been 5 years since using LTO was first suggested for use when building Bitcoin Core, and it's time to revisit it again. Compilers, and their LTO implementations, have matured, and Bitcoin Core has come a long way in terms of pruning dependencies which may have proved troublesome (i.e Boost previously had issues when using LTO). We'll have even less Boost code after moving to
std::filesystem(#20744).Experimenting with LTO came up on IRC last night:
So this PR just adds the bare minimum to make it easier to configure, compile and perform some bench-marking using
-flto. This PR doesn't do anything depends wise, however if we decide this is what we want to do, I'll expand the changes here.I had previously had a PR open (#18605) to perform link time garbage collection (
-ffunction-sections -fdata-sections&-Wl,--gc-sections), however moving straight to using LTO would be preferable.Note that our minimum required set of compilers, GCC 8.1 and Clang 7, all support the
-fltooption.Related #18579.
Previous discussion: #10616, #14277.
Previous related PRs: #10800 (
-flto), #16791 (ThinLTO).Guix build: