Implement table.copy #6078
Implement table.copy #6078
Conversation
| @@ -446,6 +446,10 @@ struct Unsubtyping | |||
| void visitTableFill(TableFill* curr) { | |||
| noteSubtype(curr->value->type, getModule()->getTable(curr->table)->type); | |||
| } | |||
| void visitTableCopy(TableCopy* curr) { | |||
There was a problem hiding this comment.
It would be nice to add a test case for this in test/lit/passes/unsubtyping.wast.
| 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.
I don't think you need to check for wrapping against both sourceVal and sizeVal. Even the most wrapping possible cannot wrap around to greater than the minimum of the two, so just one of them will serve to detect the wrapping.
There was a problem hiding this comment.
This is copied from the parallel logic in MemoryCopy. I agree that it looks unoptimal. I think the TODO covers improving them.
| @@ -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.
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.
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...
test/spec/table_copy.wast
Outdated
There was a problem hiding this comment.
I assume this is copied from upstream? I saw the TODO at the end, but did anything here require modifications? Are the failures mentioned in the TODO due to the fact that we re-evaluate table initializers whenever the table is accessed (IIRC)?
There was a problem hiding this comment.
No modifications. Just the TODO at the end, for tests that don't pass yet. I didn't investigate those in detail, but at least one factor was a failure to parse, which might be what you mentioned above.
With this, the cool Scheme game in https://davexunit.itch.io/strigoform can be optimized by wasm-opt. We reduce code size by 20% and improve the quality pretty substantially (e.g. we remove most of the casts).
Helps #5951