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

Always-inline closure passed via impl FnOnce doesn't inline #96929

Open
calebzulawski opened this issue May 11, 2022 · 2 comments
Open

Always-inline closure passed via impl FnOnce doesn't inline #96929

calebzulawski opened this issue May 11, 2022 · 2 comments
Labels
A-closures Area: Closures (`|…| { … }`) A-codegen Area: Code generation C-bug Category: This is a bug.

Comments

@calebzulawski
Copy link
Member

An example (try it in debug configuration)

use std::arch::asm;

#[inline(always)]
fn call<T>(f: impl FnOnce() -> T) -> T {
    f()
}

#[inline(always)]
const fn identity<T>(f: T) -> T {
    f
}

pub fn f() {
    let closure = identity(#[inline(always)] || unsafe { asm!("pause") });
    call(closure)
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=17e7dce927938012e15cd503caeb0cce

I expected to see this happen: call_once marked alwaysinline

Instead, this happened: call_once marked inlinehint

The closure itself is inlined, however the call_once is not, and when using impl FnOnce, effectively defeats the purpose of the #[inline(always)]. Fn and FnMut don't appear to have this issue (or LLVM is more likely to inline them, at least).

Meta

rustc --version --verbose:

rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0
@calebzulawski calebzulawski added the C-bug Category: This is a bug. label May 11, 2022
@workingjubilee workingjubilee added A-codegen Area: Code generation A-closures Area: Closures (`|…| { … }`) labels Jul 7, 2023
@RalfJung
Copy link
Member

RalfJung commented Nov 25, 2024

FnOnce uses a shim to convert the environment from by-val to by-ref. Probably that shim does not get the inline(always) attribute? But probably it should. Not sure how hard that is, I don't know much about the inner workings of our closure lowering.

Cc @compiler-errors @lcnr

@lcnr
Copy link
Contributor

lcnr commented Nov 27, 2024

It should definitely be possible 🤔 we 'just' need InstanceKind::ClosureOnceShim to inherit the codegen_fn_attrs of the closure self_type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-codegen Area: Code generation C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants