This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Emit .import_global assembler directives
ClosedPublic

Authored by jgravelle-google on Nov 18 2016, 4:01 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jgravelle-google retitled this revision from to [WebAssembly] Emit .import_global assembler directives.
jgravelle-google added reviewers: dschuff, sunfish.
jgravelle-google added a subscriber: llvm-commits.
mgrang added a subscriber: mgrang.Nov 18 2016, 4:07 PM
mgrang added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
136 ↗(On Diff #78596)

nit: You can leave out the braces here.

sunfish edited edge metadata.Nov 19 2016, 10:01 AM

This is ok, for now at least.

I was imagining that we'd eventually want to just take all undefined global symbols and automatically generate imports for them. Do you think this would work?

I was imagining that we'd eventually want to just take all undefined global symbols and automatically generate imports for them. Do you think this would work?

In my mind this makes it more explicit what the intent is, whereas auto-generating imports makes it unclear whether an undefined symbol is an error, or if it will be linked in, and we won't know until run-time. Even then we won't know whether the module is trying to import an invalid symbol, or whether the environment isn't providing that symbol. To me this makes it clearer what the module expects.

Although long-term we want to be outputting nearly-valid wasm as part of our .o format. I think at that point we'd still want to explicitly model imports/exports.

dschuff edited edge metadata.Nov 28 2016, 11:35 AM

I think this is fine for now too. For the longer term, I kind of think it depends on the use case.

For the case of something that looks like a traditional self-contained C program, I think we'd want to have undefined symbols behave like they do in traditional toolchains (which will be less surprising, and diagnose errors at link time rather than at load time).

For dylibs meant to be loaded by LLVM-built "main" modules, we probably just want something explicit (in the source) like dllimport or ELF visibility or whatever.

For a new/hybrid use case of a wasm library meant to be used from JS, that will be sort of like linking a dylib but not exactly (may have a different ABI, etc).

For the dylib/JS-module use case it might be useful to have the linker automatically convert undefined symbols to imports (as is the default in ELF for example). Although given the popularity and usefulness of -fvisibility=hidden, maybe it's not what we actually want. In any case I think we may want some explicit way to model it in the contract between the compiler and the linker, to at least allow for different ways to make it work? Anyway, we should probably discuss more in a github issue or something.

dschuff accepted this revision.Nov 28 2016, 11:36 AM
dschuff edited edge metadata.
This revision is now accepted and ready to land.Nov 28 2016, 11:36 AM
This revision was automatically updated to reflect the committed changes.