Add test for EMSCRIPTEN_KEEPALIVE behavior - #27587
Conversation
Check that EMSCRIPTEN_KEEPALIVE retains functions and global variables, and exports them under their linkage name.
|
@sbc I have a test locally for implementing EMSCRIPTEN_KEEPALIVE on top of llvm/llvm-project#201966 but I realized that we can just pre-commit the test. I didn't see a test that explicitly covered this behavior. |
| 'O1': (['-O1'],), | ||
| 'O2': (['-O2'],), | ||
| 'O3': (['-O3', '-g'],), | ||
| 'lto': (['-flto'],), |
There was a problem hiding this comment.
do we actually need all of these different modes? are they adding interesting coverage?
There was a problem hiding this comment.
If you are going to have this many variants maybe better to include in test_core.py. Seems like a good candidate for a test we want to run in every mode.
| printf("kept func: %d\n", my_kept_func(10)); | ||
| return 0; | ||
| } | ||
| ''') |
There was a problem hiding this comment.
would it be better to have these C files out-of-line?
There was a problem hiding this comment.
My general policy is that I always prefer out-of-line files for more than a few lines (maybe more than 1/2 a screefull).
We don't have any hard and fast rules though.. perhaps we should.
|
I think |
| ''') | ||
| self.run_process([EMCC, 'main.c', '-o', 'main.html'] + args) | ||
| self.assertTrue(self.is_exported_in_wasm('my_kept_func', 'main.wasm')) | ||
| self.assertTrue(self.is_exported_in_wasm('my_kept_global', 'main.wasm')) |
There was a problem hiding this comment.
I don't think s_exported_in_wasm is really enough. What you want to check is that Module['foo'] exists and has the value you expect? i.e. call the function from the outside like that, or get the global address.
You can do Module['foo'] either in EM_JS or in --pre-js / --extern-pre-js , etc
There was a problem hiding this comment.
I updated test_export_global_address.c to C++ and made it cover more cases. It does still test global underscore-prefixed names though. Maybe I should just change some of them to use Module['foo'] instead?
There was a problem hiding this comment.
For native symbol the final exported name is always underscore-prefixed today.
I've tried to change that over time... but its tricky to say the least.
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
I doubt folks are actually using C++ mangled names in this way, but I guess its good to at least get a baseline for the current behaivour.
I think if we dropped support for exporting mangled names it probably wouldn't be the end of the world.
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
Probably needs a better title and description now that this is more of a test update?
|
Yeah, it's pretty straightforward to use either linkage name or AST name, but of course if you want to actually use an overload you need linkage name. |
2 participants
Check that EMSCRIPTEN_KEEPALIVE retains functions and global variables, and
exports them under their linkage name.