Skip to content
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

"-Z merge-functions" does not enable the MergeFunctions pass under opt-level=s #98215

Closed
jrose-signal opened this issue Jun 17, 2022 · 4 comments · Fixed by #100035
Closed

"-Z merge-functions" does not enable the MergeFunctions pass under opt-level=s #98215

jrose-signal opened this issue Jun 17, 2022 · 4 comments · Fixed by #100035
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug.

Comments

@jrose-signal
Copy link

jrose-signal commented Jun 17, 2022

I tried compiling my crate with RUSTFLAGS="-Z merge-functions=aliases", but saw no difference in output size, whereas RUSTFLAGS="-C passes=mergefunc" showed a 1% improvement (not much but I'd take it). I think this is because of this code:

merge_functions: match sess
.opts
.debugging_opts
.merge_functions
.unwrap_or(sess.target.merge_functions)
{
MergeFunctions::Disabled => false,
MergeFunctions::Trampolines | MergeFunctions::Aliases => {
sess.opts.optimize == config::OptLevel::Default
|| sess.opts.optimize == config::OptLevel::Aggressive
}
},

which is probably intended to exclude opt-level=0 builds but should really at least allow Size (s) and SizeMin (z), if not enable them by default.

@jrose-signal jrose-signal added the C-bug Category: This is a bug. label Jun 17, 2022
@nikic nikic added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jun 18, 2022
@workingjubilee
Copy link
Member

Hm. Is there any chance we could get the crate or reduced code that can demonstrate the problem? I can probably dig up some code that LLVM merges functions on from my own snippets, just want to be sure.

@jrose-signal
Copy link
Author

This reproduces with just

pub fn foo() -> i32 {
    1
}

pub fn bar() -> i32 {
    1
}
% rustc -C opt-level=2 --emit llvm-ir --crate-type=rlib -o opt-level-2.ll test.rust
% rustc -C opt-level=s --emit llvm-ir --crate-type=rlib -o opt-level-s.ll test.rust

@workingjubilee
Copy link
Member

Wait, really? Wow.

@jrose-signal
Copy link
Author

Yeah, it is entirely possible the full extent of the fix is "change the code (that Jordan found naively crawling through rustc) to a match over sess.opts.optimize, and make it reasonable".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants