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

Would it be possible to have a xxx__cdeps: ['malloc', 'free'] feature? #15211

Closed
juj opened this issue Oct 4, 2021 · 2 comments
Closed

Would it be possible to have a xxx__cdeps: ['malloc', 'free'] feature? #15211

juj opened this issue Oct 4, 2021 · 2 comments

Comments

@juj
Copy link
Collaborator

juj commented Oct 4, 2021

We have this longstanding issue that if any JS function is to depend on a C function which is not depended on to by any other C function, one will need to add an explicit dependency between that JS and C function in the file tools/deps_info.py in the form

  'javascript_function_name': ['c_function_name'],

Would it be possible for us to switch to a model where in the JS libraries, we'd annotate with a foo__cdeps: ['malloc', 'free'] directive all the C functions that the given JS function depends on?

The way this would work is that the all the JS library functions would be parsed for their cdeps directives beforehand so we'd know which C functions to keep alive for the build?

That way we'd be able to delete the file tools/deps_info.py, and have user JS libraries live at the same expressiveness level as system JS libraries are.

E.g. currently in my WebGPU bindings library project, I want to depend on function malloc() from my JS library function wgpu_shader_module_get_compilation_info_async().

The WebGPU bindings library is a general library, and I don't know if the application itself will depend on malloc() or not. The demos in that repository do not. So in order to make the library work, I need to edit tools/deps_info.json to add the malloc() dependency in there - but I couldn't ask users to have to modify their Emscripten installations in order to be able to use the WebGPU library.

A wgpu_shader_module_get_compilation_info_async__cdeps: ['malloc'], directive would fix this issue cleanly?

@sbc100
Copy link
Collaborator

sbc100 commented Oct 5, 2021

Great idea in general. If we can make it work it would be great.

I have a coupe of notes:

  1. Firstly, note that deps_info does not only contain JS functions on the left hand side. It can also contains C functions names when the dependency is indirect through a system library. See

    # This usually works the way you want, but note that it happens *before* stage
    # 2 and not after it. That is, we look for js_func before linking in system
    # libraries. If you have a situation where
    #
    # user_code => other_libc_func => js_func => libc_func
    #
    # then the deps_info entry must contain
    #
    # "other_libc_func": ["libc_func"]
    . I don't think this makes a huge difference to this proposal but worth remembering.

  2. Secondly, I think this would require processing the JS libraries both before native linking as well as after. The deps_info processing fundamentally requires running before the native linker (on order to know what to keep alive) and the rest of the JS library processing fundamentally runs after (once we know what we actually need to include). I already do a pre-wasm-ld processing of the JS libraries in order to implement LLD_REPORT_UNDEFINED (

    emscripten/emcc.py

    Lines 2544 to 2545 in 8e16c4a

    if settings.LLD_REPORT_UNDEFINED and settings.ERROR_ON_UNDEFINED_SYMBOLS:
    js_syms = get_all_js_syms()
    ). I've been toying with the idea of making this the default but we need to measure the performance impact of reading the library JS files twice. For large programs its completely negligible but for tiny programs maybe not so much.

@sbc100
Copy link
Collaborator

sbc100 commented Mar 1, 2023

The code feature here has now landed as of #18849.

I still have a bunch of followup work which should eventually enable us to completely remove the reverse deps systems.

@sbc100 sbc100 closed this as completed Mar 1, 2023
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

No branches or pull requests

2 participants