build: Enable cross-language ThinLTO #4953
Draft
+11
−8
Conversation
This file contains 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
|
@zkbot try |
zkbot
added a commit
that referenced
this pull request
Jan 20, 2021
build: Enable cross-language ThinLTO Causes warnings at link time due to C++ and Rust using different target strings. These warnings do not break CI, because while we require `-Werror` to build on Linux, that only affects compilation, not linking. See rust-lang/rust#33147 for more details.
|
☀️ Test successful - pr-try |
daira
previously approved these changes
Jan 21, 2021
rex4539
suggested changes
Jan 26, 2021
There was a problem hiding this comment.
NACK 12ff63e
CXXLD zcashd
CXXLD zcash-cli
CXXLD zcash-tx
CXXLD bench/bench_bitcoin
CXXLD test/test_bitcoin
CXXLD zcash-gtest
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [zcash-cli] Error 254
make[2]: *** Waiting for unfinished jobs....
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [zcash-tx] Error 254
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [bench/bench_bitcoin] Error 254
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [zcashd] Error 254
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [test/test_bitcoin] Error 254
error: can't create module summary index for buffer: Invalid summary version 9. Version should be in the range [1-8].
clang: error: unable to execute command: Segmentation fault: 11
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [zcash-gtest] Error 254
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1
|
In addition to the above issue, this PR appears to slow down the test suites. We need to spend more time figuring out what ThinLTO is actually doing to the code. |
|
@zkbot try |
zkbot
added a commit
that referenced
this pull request
Apr 3, 2021
build: Enable cross-language ThinLTO Causes warnings at link time due to C++ and Rust using different target strings. These warnings do not break CI, because while we require `-Werror` to build on Linux, that only affects compilation, not linking. See rust-lang/rust#33147 for more details.
|
💥 Test timed out |
str4d
force-pushed
the
linker-plugin-lto
branch
from
e3c0417 to
35ba628
Compare
|
Rebased on current
|
|
Rebased on current |
Causes warnings at link time due to C++ and Rust using different target strings. See rust-lang/rust#33147 for more details.
daira
added
I-performance
|
Rebased on current |
daira
reviewed
Jun 16, 2023
| @@ -99,6 +99,7 @@ ZC_REQUIRE_TOOL(AR, ar) | |||
| ZC_REQUIRE_TOOL(RANLIB, ranlib) | |||
| ZC_REQUIRE_TOOL(STRIP, strip) | |||
| ZC_REQUIRE_PROG([GIT], [git]) | |||
| ZC_REQUIRE_PROG(CLANG, clang) | |||
There was a problem hiding this comment.
Why are we using the installed clang, rather than the pinned clang built under depends?
(I know why we aren't on macOS, but that should be the only exception.)
|
We should only do this if we have strong evidence of a performance improvement. Otherwise, it's just adding complexity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Causes warnings at link time due to C++ and Rust using different target
strings. These warnings do not break CI, because while we require
-Werrorto build on Linux, that only affects compilation, not linking.
See rust-lang/rust#33147 for more details.
Closes #6719.