Proposing user-defined inlining

[dcodeIO]

At AssemblyScript, we have an @inline decorator to force inlining of certain meant-to-be-internal features. This mostly arose from necessity because we decided to implement string concatenation like someString + someString as actual functions (here: operator overloads) instead of hard coding these operations into the compiler, and we'd like these to be always inlined, even without any other optimizations being performed. Similarly, indexed access on arrays is implemented the same way.

We are currently performing these inlining steps manually on the compiler side before creating Binaryen IR, but this has led us to a bunch of quite cumbersome code handling this arguments, operator overload special cases etc. etc.. That's sad because Binaryen already has inlining functionality much better than ours, that we'd love to reuse for our purposes. Would make our code a lot cleaner and smaller.

This PR aims at proposing a mechanism that could be used to accomplish this, something I named "UserFlags" that can be used to indicate something similar to __attribute__((always_inline)) that LLVM, which we don't have, would normally handle. It's far from perfect, and I must admit that I am not exactly sure how this should look internally (i.e. user requested inlining should probably have its own pass so it can be performed independently from actual inlining).

Thoughts? Is this something you'd be ok with supporting as an API a user can opt to use?

[dcodeIO]

Another way this could be done on our side is to mark each call that shall be inlined, instead of marking a function, if that would fit better.

[kripken]

Interesting, and I agree this is important and we should support this use case.

However, I'd prefer to not add anything to wasm.h and the core IR. Instead, I'd rather add parameters to passes. That is, I'd like to have a way to not just do (not literally this code, but in spirit) runPass("inlining") but also runPass("directed-inlining", FunctionList("name$a", "name$b")), that is, we'd have a variant of inlining that inlines exactly what it was asked, and users can run it with a parameter of those names.

This is important for other stuff too (like ExtractFunction, which receives the param through an env var currently...), but we just haven't had a nice enough design idea for it. I think we should focus on fixing that, and then adding this for inlining would be easy.

[dcodeIO]

I see, yeah. Considering that passes currently don't take any parameters (in the C and JS APIs), an alternative could be to keep a record of all calls that shall be inlined while constructing the IR normally (on our side), and once the module has been created, call something like BinaryenInline(moduleRef, callExprRef) for each call expression previously recorded for inlining. This way directed inlining could be performed before (and independently of) running any passes on a per-call basis while leaving bookkeeping to the user (nothing in wasm.h or anywhere else in Binaryen), plus the ability to return an error code or something per inlining operation. Once all inlining is complete and returned an OK status, the user could then remove the functions that are no longer necessary before optimizing. Wdyt?

[kripken]

That seems reasonable, yeah. We can move the core inlining logic into src/ir/inlining.h, use that from the pass (which would keep the logic for deciding where to inline), and add a C API call to the inlining.h method.

Why would an error code be necessary for the inliner?

And, why would the user need to remove functions that are no longer necessary - the optimizer can do that automatically? (I think the remove-unsed-module-elements will)

[dcodeIO]

Why would an error code be necessary for the inliner?

Was thinking in terms of trying to inline a function into itself or something like that, preventing it from being removed. Just a true/ false return from BinaryenInline is enough for my use case.

And, why would the user need to remove functions that are no longer necessary

In our use case, that'd make sense when not running any passes and the user wants just the "untouched" output (i.e., "remove-unsed-module-elements" could remove other things as well). Up to the user ofc, could also just rely on the pass to do it.

[kripken]

Makes sense, although probably we should design the interface so errors are not possible, or are fatal. For example, maybe we want ForceInline("name") which would inline that function into all callsites. It would avoid doing so if that would cause recursion, etc.

[dcodeIO]

Closing in favor of #1898