Add EMSCRIPTEN_EXPORT option

[kripken]

From the mailing list discussion, EMSCRIPTEN_KEEPALIVE isn't the best name, so adding another alias makes sense. Also use this in the docs.

[dschuff]

can we eventually get rid of EMSCRIPTEN_KEEPALIVE?

[dschuff]

oh, this is attribute(used)). Hm I guess keepalive really is the best name for what the attribute actually does. Then the weird thing is that this also means export. So yeah it's even more important to have EMSCRIPTEN_EXPORT so that if we want to change the mechanism by which we export (e.g. to minimize the number of exports that aren't needed) we can.

[floooh]

Just my 2ct: I think the name EMSCRIPTEN_EXPORT is useful because it gives an association to the EXPORTED_FUNCTIONS linker-stage command line option, and it also is similar to DLL_EXPORT (from the Windows world).

[kripken]

I think we never needed to differentiate exporting from keeping alive, doing both always made sense in practice. If we do want to split them up as @dschuff suggests, we probably shouldn't merge this as it would mean we need to change what this macro does later.

[floooh]

I'm fine with not merging this yet if EMSCRIPTEN_EXPORT would need to change later anyway. I have my code annotated now with EMSCRIPTEN_KEEPALIVE and it's working fine.

[sbc100]

Are we still interested in landing this?

[kripken]

I'm not sure.

Are we adding clang intrinsics for wasm imports and exports? Those would probably be better than this if so.

[dschuff]

does attribute(used)) still have any special meaning other than "don't DCE this function"? i.e. does it force thing to be exported? i know we've changed a lot of the logic around exporting things (e.g. especially differentiating things exported from a library vs the compiled executable).

[sbc100]

Yes, currently attribute(used)) means force wasm export. We probably want to have a separate attribute which is wasm-specific.

It seems like whatever the final mechanism we decide to under the hood we still want an emscripten-specific macro like this for some added indirection and explicitness.

[sbc100]

The deletion of the incoming branch triggered the automatic closing of this branch. My apologies. This was an unexpected side effect.

Re-opening and re-targeting to master.

[sbc100]

I think we should still land this. Can you rebase?

[kripken]

I think maybe we want to do this as part of a larger exporting refactoring, #8380 so best to wait on that (for which I'm not sure when I'll have time).

[stale]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

[dschuff]

Are we still using __attribute__((used))? if so we should probably keep this open?

[kripken]

Yes, it would be good to get around to this. But I think #8380 is the better idea, to do a larger refactoring of export names. Unless there is incremental benefit to this PR, I think we can close it.

[sbc100]

I think having a better name than EMSCRIPTEN_KEEPALIVE to use in the source code is interdependently a good change. #8380 seems to deal only with external/link-time settings, rather than source annotations.

[stale]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

[sbc100]

I still think we should do this

[Thaina]

Is EMSCRIPTEN_KEEPALIVE actually equivalence to EMSCRIPTEN_EXPORT and -sEXPORTED_FUNCTIONS ?

I am using 2.0.19 in unity and EMSCRIPTEN_KEEPALIVE annotation seem to not export PoC function in my .c file. I need to use -sEXPORTED_FUNCTIONS args. And so I feel uncomfortable to have no explicit EMSCRIPTEN_EXPORT annotation that really export the function I want

[sbc100]

Is EMSCRIPTEN_KEEPALIVE actually equivalence to EMSCRIPTEN_EXPORT and -sEXPORTED_FUNCTIONS ?

I am using 2.0.19 in unity and EMSCRIPTEN_KEEPALIVE annotation seem to not export PoC function in my .c file. I need to use -sEXPORTED_FUNCTIONS args. And so I feel uncomfortable to have no explicit EMSCRIPTEN_EXPORT annotation that really export the function I want

Yes EMSCRIPTEN_KEEPALIVE works just like -sEXPORTED_FUNCTIONS. Note, though, that if you code lives in a library then the object file needs to be referenced or the linker will ignore it. Is your object file part of a .a library? If so, you maybe need to -Wl,--whole-achive -lfoo -Wl,--no-whole-archive.

[Thaina]

Thank you for your clarification. But then I could say it then not equivalence. Because with -sEXPORTED_FUNCTIONS arguments I don't need to use anymore special complicate flags even that function was not used anywhere else

To be honest I try to use EMSCRIPTEN_KEEPALIVE because I don't want to add any flags into emscripten compile process. As I am using emscripten inside unity and modifying compiler flags is somewhat inconvenient. So if using EMSCRIPTEN_KEEPALIVE still require compiler flags it then defeat the purpose

I hope there could be some annotation that can explicitly force function to being export and not being trim out of optimization process. And I wish EMSCRIPTEN_EXPORT would do that

[sbc100]

Thank you for your clarification. But then I could say it then not equivalence. Because with -sEXPORTED_FUNCTIONS arguments I don't need to use anymore special complicate flags even that function was not used anywhere else

To be honest I try to use EMSCRIPTEN_KEEPALIVE because I don't want to add any flags into emscripten compile process. As I am using emscripten inside unity and modifying compiler flags is somewhat inconvenient. So if using EMSCRIPTEN_KEEPALIVE still require compiler flags it then defeat the purpose

I hope there could be some annotation that can explicitly force function to being export and not being trim out of optimization process. And I wish EMSCRIPTEN_EXPORT would do that

Is the object file in question part of a .a achive?

[Thaina]

Thank you for your clarification. But then I could say it then not equivalence. Because with -sEXPORTED_FUNCTIONS arguments I don't need to use anymore special complicate flags even that function was not used anywhere else
To be honest I try to use EMSCRIPTEN_KEEPALIVE because I don't want to add any flags into emscripten compile process. As I am using emscripten inside unity and modifying compiler flags is somewhat inconvenient. So if using EMSCRIPTEN_KEEPALIVE still require compiler flags it then defeat the purpose
I hope there could be some annotation that can explicitly force function to being export and not being trim out of optimization process. And I wish EMSCRIPTEN_EXPORT would do that

Is the object file in question part of a .a achive?

I am not so sure and I guess it's not. There is no obvious .a archive generated from unity that I can see. The only result from unity is .wasm .loader.js .framework.js and .data

And what I know is only that append -sEXPORTED_FUNCTIONS to PlayerSettings.WebGL.emscriptenArgs make it work. While putting EMSCRIPTEN_KEEPALIVE on the C function did not work

[sbc100]

And what I know is only that append -sEXPORTED_FUNCTIONS to PlayerSettings.WebGL.emscriptenArgs make it work. While putting EMSCRIPTEN_KEEPALIVE on the C function did not work

Well this I think we can assume that unity is building .a archive under the hood since that is the only way that EMSCRIPTEN_KEEPALIVE wouldn't have the same effect as -sEXPORTED_FUNCTIONS. @juj would know for sure.

Sadly, for symbols that live inside of archive files, when the linker skips the entire object file all the information in it is completely skipped, including any symbol attributes. Making EMSCRIPTEN_KEEPALIVE for symbol in archive files which are skipped would be highly non trivial. The solutions we have today are:

1. Make sure the object file in question is references by some other symbol which is included (this triggers the object file to be included)
2. Force the include of all objects using --whole-archive
3. Force the include of the symbol using -ufoo (where foo is the name of the sybmol).

Sadly both (2) and (3 ) require custom command line flags. (1) might be an option is you have at least one object file that you know will be included than you can include dummy references to symbols in question. For example:

void* dummy = &myfunc;

Include that line in a file that you know will be included at link time will cause the linker to include the object file that contains myfunc.

[Thaina]

Thank you for your suggestion. Still I wish that there could be some non hacky way for this. IMHO I think the function I expected to be export should be defined at the function declaration itself, not the compiler flags, regardless of archive generation method

I would try to see which way is suitable for developing in unity webgl. Thanks for all enlightened information

[sbc100]

Should we land this ancient PR? I think it probably useful regardless of what we do with command line flags.

[sbc100]

Make this into a .c file avoid this extern "C" block.

[dschuff]

Yeah, I think this is worth having (and worth naming correctly) if we're confident we can support it going forward (and I think we can, even if LLVM manages to replace __attribute__(used) I'm sure there would be some replacement).
We should have some documentation on how and where it can be used; e.g. only functions? (does it need go on declaration, definition, or both?), how to use with data, tables? can it go on classes, etc

[kripken]

If we are certain we can replace attribute(used) then adding this alias for EMSCRIPTEN_KEEPALIVE seems fine. If we aren't certain, though, then I still think it is safer to first see what happens with the attributes, as that would give us the option for a new EMSCRIPTEN_EXPORT to use a new attribute, avoiding a breaking change.

[dschuff]

I think the idea would be not that we are sure that we can replace attribute(used) (because IIUC the current problem is that other attribute-based options only apply to functions and not to global data), but that we want to replace the EMSCRIPTEN_KEEPALIVE name, and if EMSCRIPTEN_EXPORT only ends up ever expanding to attribute(used) as EMSCRIPTEN_KEEPALIVE does today, then that would be ok.

Are you saying that in the future we might want to switch to a different mechanism other than attribute(used), which might end up being a breaking change, so we should avoid introducing new user-facing names until that time?

[kripken]

@dschuff

Are you saying that in the future we might want to switch to a different mechanism other than attribute(used), which might end up being a breaking change, so we should avoid introducing new user-facing names until that time?

Sorry, I thought I'd replied to this long-ago notification, but it seems I did not. Yes, that was what I was saying.