SCons: Fix clang-cl link/ar flags
#96813
Merged
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
|
If using |
|
It seems like it does. I'm truthfully not entirely sure what passing those arguments does, beyond that they're accepted as arguments. Excluding those link/ar flags entirely gives comprable buildtimes & identical filesizes. Incidentally, here's the outputs from said builds: release:
lto=none:
time: 04:09.81
exe: 58,270,720
console.exe: 304,640
lto=thin:
time: 04:46.66
exe: 68,056,064
console.exe: 304,640
debug:
lto=none:
time: 04:12.88
exe: 73,480,704
console.exe: 308,224
lto=thin:
time: 05:03.34
exe: 86,334,976
console.exe: 308,224 |
|
Could you check |
|
This PRmaster |
|
Oof. Then maybe we should prevent using |
|
Note that: may be a sign of mixing object files compiled with different settings. Did you try a scratch build with |
|
I tried a from-scratch build, though it's possible that some lingering remnants stuck around somewhere. It's something I'll dig into later, as it's an insane timesink to even attempt to debug |
|
Yeah and honestly, given current findings on LLVM "full" LTO, I'd be tempted to say we might not even want to offer it as an option. Maybe we should just have |
akien-mga
approved these changes
Sep 23, 2024
|
Thanks! |
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.
lto=autoprefer ThinLTO over full LTO for LLVM targets #96785While testing LTO in a
clang-clenvironment for the above PR, I realized that the link flags were incorrectly setup:lto=thinlto=fullThis was fixed by changing the arguments to MSVC syntax, as both the linker & AR tools accept the MSVC values.