-
Notifications
You must be signed in to change notification settings - Fork 820
Implement table.copy #6078
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
Implement table.copy #6078
Changes from all commits
a94a8a7
3faeaca
6facfe1
5c1fa9f
db7a9ca
a32d777
43ca1c2
96f6c3e
ae50708
a64b249
20ff1e6
1d9fed3
f02648d
01caf65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1392,6 +1392,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> { | |
| Flow visitTableSize(TableSize* curr) { WASM_UNREACHABLE("unimp"); } | ||
| Flow visitTableGrow(TableGrow* curr) { WASM_UNREACHABLE("unimp"); } | ||
| Flow visitTableFill(TableFill* curr) { WASM_UNREACHABLE("unimp"); } | ||
| Flow visitTableCopy(TableCopy* curr) { WASM_UNREACHABLE("unimp"); } | ||
| Flow visitTry(Try* curr) { WASM_UNREACHABLE("unimp"); } | ||
| Flow visitThrow(Throw* curr) { | ||
| NOTE_ENTER("Throw"); | ||
|
|
@@ -2210,6 +2211,10 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> { | |
| NOTE_ENTER("TableFill"); | ||
| return Flow(NONCONSTANT_FLOW); | ||
| } | ||
| Flow visitTableCopy(TableCopy* curr) { | ||
| NOTE_ENTER("TableCopy"); | ||
| return Flow(NONCONSTANT_FLOW); | ||
| } | ||
| Flow visitLoad(Load* curr) { | ||
| NOTE_ENTER("Load"); | ||
| return Flow(NONCONSTANT_FLOW); | ||
|
|
@@ -3027,6 +3032,57 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> { | |
| return Flow(); | ||
| } | ||
|
|
||
| Flow visitTableCopy(TableCopy* curr) { | ||
| NOTE_ENTER("TableCopy"); | ||
| Flow dest = self()->visit(curr->dest); | ||
| if (dest.breaking()) { | ||
| return dest; | ||
| } | ||
| Flow source = self()->visit(curr->source); | ||
| if (source.breaking()) { | ||
| return source; | ||
| } | ||
| Flow size = self()->visit(curr->size); | ||
| if (size.breaking()) { | ||
| return size; | ||
| } | ||
| NOTE_EVAL1(dest); | ||
| NOTE_EVAL1(source); | ||
| NOTE_EVAL1(size); | ||
| Address destVal(dest.getSingleValue().getUnsigned()); | ||
| Address sourceVal(source.getSingleValue().getUnsigned()); | ||
| Address sizeVal(size.getSingleValue().getUnsigned()); | ||
|
|
||
| auto destInfo = getTableInterfaceInfo(curr->destTable); | ||
| auto sourceInfo = getTableInterfaceInfo(curr->sourceTable); | ||
| auto destTableSize = destInfo.interface->tableSize(destInfo.name); | ||
| auto sourceTableSize = sourceInfo.interface->tableSize(sourceInfo.name); | ||
| if (sourceVal + sizeVal > sourceTableSize || | ||
| destVal + sizeVal > destTableSize || | ||
| // FIXME: better/cheaper way to detect wrapping? | ||
| sourceVal + sizeVal < sourceVal || sourceVal + sizeVal < sizeVal || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need to check for wrapping against both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is copied from the parallel logic in MemoryCopy. I agree that it looks unoptimal. I think the TODO covers improving them. |
||
| destVal + sizeVal < destVal || destVal + sizeVal < sizeVal) { | ||
| trap("out of bounds segment access in table.copy"); | ||
| } | ||
|
|
||
| int64_t start = 0; | ||
| int64_t end = sizeVal; | ||
| int step = 1; | ||
| // Reverse direction if source is below dest | ||
| if (sourceVal < destVal) { | ||
| start = int64_t(sizeVal) - 1; | ||
| end = -1; | ||
| step = -1; | ||
| } | ||
| for (int64_t i = start; i != end; i += step) { | ||
| destInfo.interface->tableStore( | ||
| destInfo.name, | ||
| destVal + i, | ||
| sourceInfo.interface->tableLoad(sourceInfo.name, sourceVal + i)); | ||
| } | ||
| return {}; | ||
| } | ||
|
|
||
| Flow visitLocalGet(LocalGet* curr) { | ||
| NOTE_ENTER("LocalGet"); | ||
| auto index = curr->index; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2706,6 +2706,24 @@ Expression* SExpressionWasmBuilder::makeTableFill(Element& s) { | |
| return Builder(wasm).makeTableFill(tableName, dest, value, size); | ||
| } | ||
|
|
||
| Expression* SExpressionWasmBuilder::makeTableCopy(Element& s) { | ||
| auto destTableName = s[1]->str(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we support raw indices as well, or do we not support those in other places? The new wat parser will support them in either case (eventually). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we do in other places, as this is based on TableFill right above us. That actually might explain why some spec tests didn't pass... |
||
| auto* destTable = wasm.getTableOrNull(destTableName); | ||
| if (!destTable) { | ||
| throw SParseException("invalid dest table name in table.copy", s); | ||
| } | ||
| auto sourceTableName = s[2]->str(); | ||
| auto* sourceTable = wasm.getTableOrNull(sourceTableName); | ||
| if (!sourceTable) { | ||
| throw SParseException("invalid source table name in table.copy", s); | ||
| } | ||
| auto* dest = parseExpression(s[3]); | ||
| auto* source = parseExpression(s[4]); | ||
| auto* size = parseExpression(s[5]); | ||
| return Builder(wasm).makeTableCopy( | ||
| dest, source, size, destTableName, sourceTableName); | ||
| } | ||
|
|
||
| // try can be either in the form of try-catch or try-delegate. | ||
| // try-catch is written in the folded wast format as | ||
| // (try | ||
|
|
||
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.
It would be nice to add a test case for this in test/lit/passes/unsubtyping.wast.