-
Notifications
You must be signed in to change notification settings - Fork 65
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.
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
Optimize build for libbz2.a
#88
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Jiahao XU <[email protected]>
Signed-off-by: Jiahao XU <[email protected]>
Signed-off-by: Jiahao XU <[email protected]>
Signed-off-by: Jiahao XU <[email protected]>
bzip2-sys/build.rs
Outdated
.flag_if_supported("-fdata-sections") | ||
.flag_if_supported("-fmerge-all-constants") | ||
.flag_if_supported("-Wl,--gc-sections") | ||
.flag_if_supported("-Wl,--icf=safe"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to have -ffunction-sections
and co. -Wl,--gc-sections
and -Wl,--icf=safe
can be omitted though as those only apply to linking by the cc crate, but we only build a staticlib here. They don't have any effect on linking done by rustc.
@@ -41,6 +41,10 @@ fn main() { | |||
} | |||
} | |||
|
|||
if cfg!(feature = "thin") { | |||
cfg.opt_level_str("z"); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably forward the OPT_LEVEL
env var instead of adding a feature.
.flag_if_supported("-fmerge-all-constants") | ||
.flag_if_supported("-Wl,--gc-sections") | ||
.flag_if_supported("-Wl,--icf=safe"); | ||
.flag_if_supported("-fmerge-all-constants"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't notice this commit. Maybe squash it into the first commit?
} else { | ||
cfg.flag_if_supported("-flto"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've asked at https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/read.20lto.20config.20from.20build.20script if there is a way to read the LTO state from cargo without having to use feature flags. In any case this won't actually LTO between bzip2 and any rust code. That is only possible when using the rust port of bzip2 rather than the C original or by using -Clinker-plugin-lto
and making sure that the exact right clang version is used to build the C code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out the cc crate itself nowadays handles -Clto
and -Copt-level
already, so the second and third commit can be dropped. Only the first and fourth commit (which should be squashed) are still necessary.
--gc-sections
to remove unused symbols fromlibbz2.a
fat-lto
&thin-lto
thin
to use opt-levelz
Signed-off-by: Jiahao XU [email protected]