Skip to content

Allow JS library dependencies to be added in source code. #17854

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 1 commit into from
Oct 7, 2022

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Sep 14, 2022

This change introduces a new EM_JS_DEPS macros that can be used to specific the JS library dependencies of user code in EM_JS and/or EM_ASM blocks.

This is especially important for library authors who don't want to have their users maintain link-time list of required symbols.

See #14729

@sbc100 sbc100 force-pushed the inline_js_deps branch 2 times, most recently from c1da048 to 6221d6d Compare September 14, 2022 18:41
@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 14, 2022

I guess we should try to make a more generic version of this that would satisfy #14729 fully?

@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 14, 2022

Depends on this llvm change: https://reviews.llvm.org/D133884

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 4, 2022

Depends on WebAssembly/binaryen#5109 to avoid minor codesize regressions.

@sbc100 sbc100 force-pushed the inline_js_deps branch 2 times, most recently from 0282262 to 3c6126c Compare October 4, 2022 04:04
@sbc100 sbc100 requested review from kripken, juj and dschuff and removed request for juj October 4, 2022 22:00
@brendandahl
Copy link
Collaborator

To make it more generic and potentially useable for marking methods async (or any extra attributes we want). What about introducing a function attribute annotation that is preserved and extractable in metadata.
e.g.

#define EM_ATTRIBUTE(KEY, VALUE) __attribute__((annotate(#KEY "-" VALUE)))

Then we could have

#define EM_DEPS(DEPS) EM_ATTRIBUTE(DEPS, deps)
#define EM_ASYNC() EM_ATTRIBUTE(ASYNC, "")

EM_DEPS("$allocateUTF8OnStack") int main() {

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 5, 2022

To make it more generic and potentially useable for marking methods async (or any extra attributes we want). What about introducing a function attribute annotation that is preserved and extractable in metadata. e.g.

#define EM_ATTRIBUTE(KEY, VALUE) __attribute__((annotate(#KEY "-" VALUE)))

Then we could have

#define EM_DEPS(DEPS) EM_ATTRIBUTE(DEPS, deps)
#define EM_ASYNC() EM_ATTRIBUTE(ASYNC, "")

EM_DEPS("$allocateUTF8OnStack") int main() {

That looks like an awesome idea. I don't think that annotate thing exists though does it? Or would we have to invent it? It would have to pipe its way all the way into the final binary somehow? It sounds like a fair amount of llvm-side work with custom sections etc.

@brendandahl
Copy link
Collaborator

Clang supports annotate (it generates IR for it), but yeah we'd have to pipe it all the way through and then remove it like the em_js stuff. This is the page I saw about it awhile ago when I was looking into some bindings options.

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 5, 2022

Clang supports annotate (it generates IR for it), but yeah we'd have to pipe it all the way through and then remove it like the em_js stuff. This is the page I saw about it awhile ago when I was looking into some bindings options.

That certainly sounds like a great long term plan but needs a fair amount of work in llvm and wasm-ld to pipe those though I think. I still think it would be reasonable to land this change which can be used in the interim. WDYT? I guess the downside is it might need to keep this mechanism alive even after the new one arrives. (Actually maybe that isn't true.. I think we can define EM_JS_DEPS in terms of your proposed EM_DEPS?)

@sbc100 sbc100 requested review from kripken and aheejin October 5, 2022 17:51
@brendandahl
Copy link
Collaborator

Landing this in mean time sounds good. I saw you were doing some LLVM work, so I thought maybe it wouldn't be much more to do the annotation part (I have no idea).

Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

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

Nice!

*
* Dependencies declared in this way will be included if-and-only-if the object
* file (translation unit) in which they exist is included by the linker, so
* it makes sense co-locate them with the EM_JS or EM_ASM code they correspond
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* it makes sense co-locate them with the EM_JS or EM_ASM code they correspond
* it makes sense to co-locate them with the EM_JS or EM_ASM code they correspond


/*
* EM_JS_DEPS: Use this macro to declare indirect dependencies on JS symbols.
* The first argument is just unique name for the set of dependencies. The
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The first argument is just unique name for the set of dependencies. The
* The first argument is just a unique name for the set of dependencies. The

@juj
Copy link
Collaborator

juj commented Oct 7, 2022

This looks good. I wonder if this could be expanded to also cover JS library functions -> C functions dependencies?

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 7, 2022

This looks good. I wonder if this could be expanded to also cover JS library functions -> C functions dependencies?

I would love to find a way to do that yes. Such dependencies have to be present and wasm-ld time though. i.e. wasm-ld has to resolve them while its linking, not output them.

It would be some kind of pattern like this:

void my_extern_symbol();

DEPS(my_extern_symbol, foo, bar);   // <- tel the linker that if it references my_extern_symbol, then it also needs to export foo and bar.

One way to do that is to create some kind of dummy shared library or object file for each JS library, before running the linker. But I think this dummy object would probably have to be in a format that currently doesn't exist. It would be more like a linker script. I very much like this idea, but it needs fleshing out.

This change introduces a new EM_JS_DEPS macros that can be used to
specific the JS library dependencies of user code in EM_JS and/or EM_ASM
blocks.

This is especially important for library authors who don't want to have
their users maintain link-time list of required symbols.

See #14729
@sbc100 sbc100 merged commit 974c73e into main Oct 7, 2022
@sbc100 sbc100 deleted the inline_js_deps branch October 7, 2022 19:58
@dschuff
Copy link
Member

dschuff commented Oct 7, 2022

This should probably be documented somwhere other than just the release notes, presumably in https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html ?

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.

6 participants