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

Wrapper/glue functions such as std::fs::FileType::is_file are not always inlined #75052

Open
Diomendius opened this issue Aug 2, 2020 · 3 comments
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@Diomendius
Copy link

This function should compile to the exact same assembly as FileType::is_file itself, but doesn't: (godbolt.org link)

pub fn is_file(x: std::fs::FileType) -> bool {
    x.is_file()
}
example::is_file:
        push    rax
        mov     dword ptr [rsp + 4], edi
        lea     rdi, [rsp + 4]
        call    qword ptr [rip + std::fs::FileType::is_file@GOTPCREL]
        pop     rcx
        ret

FileType::is_file is only 2-3 instructions and should surely meet the inlining threshold. Other functions such as std::fs::Metadata::ino (provided by std::os::unix::fs::MetadataExt) are also not inlined, despite ino() being just a single load.

This also prevents simple optimizations; despite being a pure function, calling is_file() twice results in two separate call instructions just to load, mask and compare the same unchanged var: (godbolt.org link)

When compiling a binary with LTO enabled, these functions get inlined as they should, as far as I can tell, but otherwise there seems to be some kind of boundary preventing the compiler from inlining them.

None of the functions in question are marked #[inline], but neither is Vec::is_empty for example, yet that function gets inlined just fine.

This bug is present as early as rust 1.1.0 and is present in the current nightly:

rustc 1.47.0-nightly (05762e3d6 2020-08-01)
binary: rustc
commit-hash: 05762e3d6f5facafdd47efdf4203021fadf61bb1
commit-date: 2020-08-01
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0
@Diomendius Diomendius added the C-bug Category: This is a bug. label Aug 2, 2020
@Mark-Simulacrum Mark-Simulacrum added the I-slow Issue: Problems and improvements with respect to performance of generated code. label Aug 2, 2020
@Mark-Simulacrum
Copy link
Member

FileType::is_file, being non-generic and not marked #[inline], is not codegen'd into the final binary today which makes it impossible for LLVM to inline it.

@Diomendius
Copy link
Author

I was under the impression that only applied to cross-crate inlining, but looking at this with fresh eyes, I see that std is, in fact, a crate. Vec::is_empty is generic, which I didn't pay attention to before, so that explains the discrepancy.

I also did not find when searching earlier that this issue is mostly a duplicate of #37538, except that the functions in question are in std in this case.

I thought there was some plan towards improving the situation with cross-crate inlining, but I can't find any issues referencing this.

At least in this case the relevant functions relate to filesystem operations, and so are unlikely to form a bottleneck in real-world code, but it does raise the question of whether there are any other trivial non-generic functions in std not marked #[inline].

@AngelicosPhosphoros
Copy link
Contributor

AngelicosPhosphoros commented Apr 4, 2021

I was under the impression that only applied to cross-crate inlining

Only if code builded with -C codegen-units=1 because otherwise crate can be splitted into few translations units and compiler cannot inline between them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

3 participants