• a test

In D147190#4234228, @dschuff wrote:

I guess the new test covers the stub->bitcode lib dependence case. Is there a test that covers the codgen->libcall->stub case?

• feedback

In D147190#4234186, @dschuff wrote:

i.e. it seems like it would be simpler to just support processing the stubs once. or maybe more specifically, to handle them the same as other archive libraries (i.e. participate in the iterate-to-convergence phase of symbol resolution but not get special treatment for LTO?)

In D147190#4234179, @dschuff wrote:

Do we really need to support stub libcalls (i.e. the kind that can be generated at codegen time)? These should just be compiler-rt and maybe a few core libc functions, right? Do we expect those to be implemented in JS and/or the environment?

• comments

In D76547#4231604, @aaron.ballman wrote:

In D76547#4231501, @sbc100 wrote:

In D76547#4231476, @aaron.ballman wrote:

In D76547#4231422, @sbc100 wrote:

The reason __attribute__((export_name("foo"))) doesn't work in all use cases is that we have a lot of existing code that uses the EMSCRIPTEN_KEEPALIVE macro. We also have run into other folks who want to include this is some kind of FOO_API, or EXPORT_API type macros. Its not possible to have such a macro map to the existing export_name since they don't include the symbol name: e.g:

EMSCRIPTEN_KEEPALIVE int foo();`

JNI_EXPORT int myfunct();

In these cases we need something that uses the llvm symbol name for the export.

I think there's two ways we could address this without adding a new attribute (maybe you've thought of this and have reasons for this to be a bad suggestion):

• It seems that export_name doesn't care if you put in an empty string for the argument, so we could treat that case as meaning "export with the name of the symbol this attribute is attached to"
• We could allow export_name to take zero or one argument. The one-argument form is the same as it is today, but the zero argument form exports with the name of the symbol the attribute is attached to.

Do you think either of those could work?

Yes, I think the second one would be ideal. The first one is slightly less idea because it prevents something being exported with the empty string as its name (wasm allows such things).

Ah, yeah, that's a good reason to avoid the first one.

Can an attribute take an optional argument? That would be great solution. For my initial version of this change I did look into making export_name(DEFAULT) work (note the lack of quotes around the word DEFAULT here), but I could not find way to make a single attribute take both a string *or* a constant.

Yup, it takes only a little bit of extra work to do right. Have the attribute's argument list take a VariadicStringArgument instead of StringArgument so you can pass zero or more arguments, then have the attribute handler in SemaDeclAttr.cpp diagnose if the attribute is given > 1 argument. The rest should be things like documentation or fall out somewhat naturally (there will be accessors added to the WebAssemblyExportNameAttr class that let you access the arguments with iterators, and a size accessor as well, so you can tell if the semantic attribute does/does not have an argument).

In D76547#4231476, @aaron.ballman wrote:

In D76547#4231422, @sbc100 wrote:

The reason __attribute__((export_name("foo"))) doesn't work in all use cases is that we have a lot of existing code that uses the EMSCRIPTEN_KEEPALIVE macro. We also have run into other folks who want to include this is some kind of FOO_API, or EXPORT_API type macros. Its not possible to have such a macro map to the existing export_name since they don't include the symbol name: e.g:

EMSCRIPTEN_KEEPALIVE int foo();`

JNI_EXPORT int myfunct();

In these cases we need something that uses the llvm symbol name for the export.

I think there's two ways we could address this without adding a new attribute (maybe you've thought of this and have reasons for this to be a bad suggestion):

• It seems that export_name doesn't care if you put in an empty string for the argument, so we could treat that case as meaning "export with the name of the symbol this attribute is attached to"
• We could allow export_name to take zero or one argument. The one-argument form is the same as it is today, but the zero argument form exports with the name of the symbol the attribute is attached to.

Do you think either of those could work?

The reason __attribute__((export_name("foo"))) doesn't work in all use cases is that we have a lot of existing code that uses the EMSCRIPTEN_KEEPALIVE macro. We also have run into other folks who want to include this is some kind of FOO_API, or EXPORT_API type macros. Its not possible to have such a macro map to the existing export_name since they don't include the symbol name: e.g:

I just figured out that this cannot replace the current use of __attribute__((used)) in emscripten because function attributes only work for functions and we need this mechanism to work for global data addresses too. There is simply no way to do something like Fn->addFnAttr("wasm-exported"); for a GlobalValue that isn't a function (as far as I can tell).

• limit to emscripten

I've limited to new attribute to only the emcripten triple.

• update test

In D76547#1945094, @sbc100 wrote:

What about your idea of using the default keyword rather than adding a new clang attr? I quite liked that approach.

• rebase

Any objections to landing this?

• feedback

• remove unused method

• error -> assert

WDYT Dan? My guess is that wasi-sdk doesn't currently need to express these reverse dependencies, so its not currently relevant.

In D145308#4181854, @dschuff wrote:

I think this feature could also be useful in non-emscripten environments, e.g. you could have a stub library for each WASI API provided by the embedder. Does wasi-sdk just use --allow-undefined for this now? Stub libraries seem nicer than that.
@sunfish WDYT, does this look useful?

• stub libraries

What you say ", TLS + dynamic linking will not work at the moment." do you mean just the debug info won't work? Or it won't work at all?

• rebase

• feedback

Feedback

• feedback

• Revert part

• revert ELF

In general, when making these kind of generic changes, would it be possible to also include wasm-ld (lld/wasm) so that we keep things as in-sync as possible?

Fixes

• dead code

• typos

lgtm % some minor nits

• feedback

ptal

Working on adding a test now. This is needed to run wasm64 + pthread + dynamic linking tests on the emscripten side.

I think this change is find to land as is. If we decided to simplify that .s format we can possibly simplify the parser code later.

I wonder if its too late to change the requirement such that .functype should always come before the label? This would make the code simpler and the assembly format more consistent maybe?