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

Cranelift: develop an inlining pass #4127

Open
cfallin opened this issue May 10, 2022 · 3 comments
Open

Cranelift: develop an inlining pass #4127

cfallin opened this issue May 10, 2022 · 3 comments
Labels
cranelift Issues related to the Cranelift code generator enhancement

Comments

@cfallin
Copy link
Member

cfallin commented May 10, 2022

In the 2022 roadmap, we described the need to add inlining to Cranelift. This need comes mainly from anticipated future workloads in some Cranelift applications. For example, when used as a Wasm backend, multi-module use-cases will become more common as the component model becomes a reality. In such use-cases, no inlining would have been done by the Wasm toolchain; execution in Wasmtime/Cranelift is the first time that modules "meet" and can cross-inline calls, or at least interface-type adapter shim functions.

We will likely want to be a little careful about the inlining heuristic and associated costs, in order to preserve our general "fast compilation" focus. Perhaps we want to only inline explicitly-marked-as-inlinable functions. Or perhaps a bottom-up-over-callgraph-SCCs approach (like LLVM) is still the right one, but with a low cutoff threshold for function size to inline.

@cfallin cfallin added enhancement cranelift Issues related to the Cranelift code generator labels May 10, 2022
@fitzgen
Copy link
Member

fitzgen commented May 11, 2022

We will likely want to be a little careful about the inlining heuristic and associated costs, in order to preserve our general "fast compilation" focus.

I think we will want to use information about the modules that functions come from when deciding whether to inline.

For example, inlining a tiny function from module A into a function in module B makes a lot of sense.

However inlining a tiny function from module A into another function in module A doesn't make sense. If that was profitable, the Wasm-producing toolchain (LLVM 99.99% of the time) would have presumably already done it, and because the toolchain didn't the function was most likely marked the equivalent of #[inline(never)]; we just don't have that attribute/metadata anymore by the time we are compiling the Wasm and therefore don't know why the decision not to inline was made.

Note that this means that the same function could be considered a good candidate for inlining at one call site and a bad candidate for inlining at another call site.

Perhaps we want to only inline explicitly-marked-as-inlinable functions.

Who would do mark functions as inlinable? The Wasm won't have any such annotations and it isn't really something that the Wasm frontend can determine on its own in general (mayyyyyyyyybe by looking at Wasm branch hinting annotation and only inlining for hot call sites in hot blocks? Seems easy to get poor heuristics this way and I'd rather intoduce an inlining hint annotation Wasm proposal than seriously pursue this).

@cfallin
Copy link
Member Author

cfallin commented May 11, 2022

Who would do mark functions as inlinable? The Wasm won't have any such annotations and it isn't really something that the Wasm frontend can determine on its own in general (mayyyyyyyyybe by looking at Wasm branch hinting annotation and only inlining for hot call sites in hot blocks? Seems easy to get poor heuristics this way and I'd rather intoduce an inlining hint annotation Wasm proposal than seriously pursue this).

For this use-case I'm mostly anticipating Interface Types-like applications: generated adapter functions/thunks/stubs can be explicitly marked as "please inline". This seems like some nice low-hanging fruit in that (i) the Cranelift user (here Wasmtime) might know that some functions are small, by construction, and (ii) there are relatively few of them and they are only one level deep, so this is not blowing up compile-time by inlining everywhere.

@bjorn3
Copy link
Contributor

bjorn3 commented May 11, 2022

However inlining a tiny function from module A into another function in module A doesn't make sense.

For wasm you are mostly right, although someone may have used #[inline(never)] for debuginfo reasons or to hide UB from the compiler, in which case inlining at clif ir level should be fine I think. For cg_clif inlining within the same module is absolutely necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator enhancement
Development

No branches or pull requests

3 participants