[Webkit-unassigned] [Bug 219427] [WASM-References] Add support for table.copy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=219427

--- Comment #4 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 415201
  --> https://bugs.webkit.org/attachment.cgi?id=415201
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415201&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Add support for table.copy from reference types proposal:

might be worth describing what it does

> Source/JavaScriptCore/wasm/WasmFunctionParser.h:670
> +            TypedExpression lenght;

lenght => length

> Source/JavaScriptCore/wasm/WasmInstance.cpp:134
> +            for (uint32_t idx = len; idx > 0; --idx)
> +                fn(dstTable, srcTable, dstOffset + (idx - 1), srcOffset + (idx - 1));

I prefer to write this loop as:

```
for (uint32_t idx = len; idx--; )
```
Then inside the body of the loop, you can just refer to idx instead of (idx - 1)

>> Source/JavaScriptCore/wasm/WasmOperations.cpp:729
>> +}
> 
> I think you can use Checked<uint32_t> for this.

just make sure to not use CrashOnOverflow and use RecordOverflow instead

> Source/JavaScriptCore/wasm/WasmOperations.cpp:739
> +    if (dstTable->isExternrefTable() != srcTable->isExternrefTable())
> +        return false;

the error message you throw in the JIT code seems different than this type of error

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:424
> +            // We don't have the table.init instruction to use passive element segments,
> +            // but we need to parse them for the table.copy spec test, so we just skip them.

This seems very fishy. We're parsing something we don't support just for a test?

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:501
> +            // We don't have the table.init instruction to use passive element segments,
> +            // but we need to parse them for the table.copy spec test, so we just skip them.

ditto

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201202/6ba904fd/attachment.htm>