Skip to content

Enable link time -Os in opt builds#348

Merged
brentleyjones merged 3 commits intobazelbuild:masterfrom
cerisier:link_osize_opt_feature
Nov 11, 2024
Merged

Enable link time -Os in opt builds#348
brentleyjones merged 3 commits intobazelbuild:masterfrom
cerisier:link_osize_opt_feature

Conversation

@cerisier
Copy link
Copy Markdown
Contributor

@cerisier cerisier commented Nov 7, 2024 •
edited
Loading

ld starting Xcode 15 (ld_prime) now supports this option to perform more general code de-deduplication passes which it was not doing prior.

It is not documented anywhere but Xcode 15+ has this enabled by default in the default Release scheme (which hasOptimization Level = Fastest/Smallest -Os by default).

I'm adding this as a separate feature so that it can be disabled but there could be an argument whether to enable this by default !

Fun note: passing any character after -O performs the same as -Os...

brentleyjones
brentleyjones reviewed Nov 7, 2024
View reviewed changes
brentleyjones
brentleyjones reviewed Nov 8, 2024
View reviewed changes
@brentleyjones brentleyjones merged commit 27df79d into bazelbuild:master Nov 11, 2024
@fhanau
Copy link
Copy Markdown

fhanau commented Nov 19, 2024

@cerisier Can you provide an example where you see an effect on binary size from using the flag? I'm using Xcode 16 and it looks to me like it is on-by-default – when I add just -Wl,-Os there is no effect on binary size. If I pass -Wl,-O0 followed by -Wl,-Os the output has the same size as when no flags are passed, suggesting that -Os reverts the increase in binary size that would otherwise be caused by -O0 but doesn't have an effect just by itself.

Additionally this breaks the build when using lld as the linker (for example when having Homebrew lld installed and using --linkopt="-fuse-ld=lld") – I know that apple_support does not explicitly support lld but it would be nice to not have a regression for it.

@brentleyjones
Copy link
Copy Markdown
Collaborator

brentleyjones commented Nov 19, 2024

When using lld you can disable the feature. Per the PR description, passing any -O to the linker applies this, so you can't undo it in the Xcode build.

@fhanau
Copy link
Copy Markdown

fhanau commented Nov 20, 2024

Per the PR description, passing any -O to the linker applies this, so you can't undo it in the Xcode build.

The PR description is incorrect then – passing -O0 reverts the effect of using -Os or a different -O option for me locally.
Furthermore, the behavior of -Os or similar appears to be the default behavior – to be specific I'm seeing this when invoking the linker via the clang++ driver as opposed to via Xcode directly. If -Wl,-Os only specifies the default behavior, then there is no need to pass this flag in opt mode. Perhaps builds through Xcode default to the -O0 behavior in some configurations?

@jpsim jpsim mentioned this pull request Feb 18, 2025
Closed
@jpsim
Copy link
Copy Markdown

jpsim commented Feb 18, 2025

Heads up that I'm seeing some non-hermeticity with this change: #371

keith added a commit that referenced this pull request Feb 18, 2025
@keith
Revert "Enable link time -Os in opt builds (#348)"
682bd22 
Sidestep #371 until
it's fixed in Xcode. Users who still want this vs can pass
`--linkopt=-Os`

This reverts commit 27df79d.
@cerisier cerisier deleted the link_osize_opt_feature branch February 18, 2025 18:24
keith added a commit that referenced this pull request Feb 18, 2025
@keith
Revert "Enable link time -Os in opt builds (#348)" (#372)
1f3903f 
Sidestep #371 until
it's fixed in Xcode. Users who still want this vs can pass
`--linkopt=-Os`

This reverts commit 27df79d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brentleyjones brentleyjones brentleyjones approved these changes

@BalestraPatrick BalestraPatrick Awaiting requested review from BalestraPatrick

@keith keith Awaiting requested review from keith

@luispadron luispadron Awaiting requested review from luispadron

@mattrobmattrob mattrobmattrob Awaiting requested review from mattrobmattrob

@thii thii Awaiting requested review from thii

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cerisier @fhanau @brentleyjones @jpsim