Skip to content

Let GenerateDynCalls generate dynCalls for invokes #3192

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

Merged
merged 3 commits into from
Oct 2, 2020

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Oct 1, 2020

This moves dynCall generating functionaity for invokes from
EmscriptenGlueGenerator to GenerateDynCalls pass. So now
GenerateDynCalls pass will take care of all cases we need dynCalls:
functions in tables and invokes.

This moves dynCall generating functionaity for invokes from
`EmscriptenGlueGenerator` to `GenerateDynCalls` pass. So now
`GenerateDynCalls` pass will take care of all cases we need dynCalls:
functions in tables and invokes.
@aheejin aheejin requested a review from sbc100 October 1, 2020 23:32
(local.get $1)
(local.get $fptr)
)
)
Copy link
Member Author

@aheejin aheejin Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dynCalls are generated because the input file contains invoke_ functions. Before they were not targets for dynCall generations because EmscriptenGlueGenerator looked for the format generated by the LLVM backend, like __invoke_void_i32, and not invoke_vi. But now this pass is supposed to run after renaming invokes, this pass should look for invoke_ instead, resulting in this change.

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@aheejin aheejin merged commit ecaa818 into WebAssembly:master Oct 2, 2020
@aheejin aheejin deleted the dyncall_invoke branch October 2, 2020 16:25
aheejin added a commit that referenced this pull request Oct 11, 2020
Now that we are renaming invoke wrappers and `emscripten_longjmp_jmpbuf`
in the wasm backend, this deletes all related renaming routines and
relevant tests. Depends on #3192.

Addresses: #3043 and #3081

Companions:
https://reviews.llvm.org/D88697
emscripten-core/emscripten#12399
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