Skip to content

Add flags indicating whether a function shall always be inlined #1330

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

Closed
wants to merge 1 commit into from

Conversation

dcodeIO
Copy link
Contributor

@dcodeIO dcodeIO commented Dec 7, 2017

This PR adds a flags enum to Function, for now with a single value that code generators can set to force inlining of a function when the inlining pass is run.

C-API:

  • BinaryenFunctionGetFlags(func)
  • BinaryenFunctionSetFlags(func, flags)

JS-API:

  • getFunctionFlags(func)
  • setFunctionFlags(func, flags)

My use case is something like this, that is inlining array accesses that are functions in the sources, but really shouldn't be in WASM.

@kripken
Copy link
Member

kripken commented Dec 7, 2017

Interesting. I agree we should optimize that use case, however, adding function flags is something I've hoped we can avoid. If we do want to add them, it would be a large effort - they'd need some sort of text format representation, and maybe even binary format as well (which would need to be coordinated either with the wasm tool-conventions project, or force us to really fork the binary format, as has been discussed in the past).

Why don't those functions get inlined already? Perhaps we just need to tweak the heuristics? If it's not that simple, we could add a variant pass, InlineAggressively perhaps, which inlines a lot more and focuses on this use case. (If we can't manage that, I think we can look at more radical options like adding flags.)

Maybe you can provide the wast of that example, for investigation?

@dcodeIO
Copy link
Contributor Author

dcodeIO commented Dec 7, 2017

I see. When I think about it, my use case is more about code generation in general and not so much about optimization. Similar to the constant evaluation stuff earlier, I am just trying to reuse what Binaryen already has. Not so sure anymore if that makes a lot of sense here.

Ideally, I could use a tool to convert a ready-made function in Binaryen IR to a Binaryen IR block expression like the inlining pass does (let's say BinaryenFunctionAsExpression, exposing the functionality provided by static Expression* doInlining(Module* module, Function* into, InliningAction& action)), without ever adding the function and reuse the expression multiple times instead. Would that be something you'd consider useful as an alternative to this PR, or do you think it would be best to not rely on Binaryen in this case and let my compiler do the function->expression conversion manually?

@dcodeIO
Copy link
Contributor Author

dcodeIO commented Dec 10, 2017

Closing for now. Might create a new PR once I have a better idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants