-
Notifications
You must be signed in to change notification settings - Fork 717
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
wasm2js: Add Table operations #6650
Conversation
@@ -2236,28 +2235,40 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m, | |||
visit(curr->right, EXPRESSION_RESULT)); | |||
} | |||
Ref visitTableGet(TableGet* curr) { | |||
unimplemented(curr); | |||
WASM_UNREACHABLE("unimp"); | |||
return ValueBuilder::makeSub(ValueBuilder::makeName(FUNCTION_TABLE), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we validate elsewhere that the module has only one table? If not, we might want to raise an error here and on the other operations if the table is not at index 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have an error up front about that, it is a built-in assumption throughout wasm2js.
binaryen/src/tools/wasm2js.cpp
Lines 960 to 963 in cdd94a0
// TODO: Remove this restriction when wasm2js can handle multiple tables | |
if (wasm->tables.size() > 1) { | |
Fatal() << "error: modules with multiple tables are not supported yet."; | |
} |
TableGet, Set, Size, Grow, Fill, Copy.
Also move "null" into shared-constants, to make the code
more consistent overall.