[Webkit-unassigned] [Bug 219943] [WASM-References] Add support for memory.copy, memory.init and data.drop

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #416402|review?                     |review+
              Flags|                            |

--- Comment #8 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 416402
  --> https://bugs.webkit.org/attachment.cgi?id=416402
Patch

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

r=me with comments.

> Source/JavaScriptCore/ChangeLog:25
> +

Nice. It is also nice if we have DataCount section information (like, what it is and why it is needed).

> Source/JavaScriptCore/wasm/WasmFormat.cpp:37
> +Segment* Segment::create(Optional<I32InitExpr> offset, uint32_t sizeInBytes, Kind kind)

Oh, while this is not the changed part in this patch, I think we should return `Segment::Ptr` from this function instead of calling `Segment::adoptPtr()` later.
Then, Segment's memory is managed well by unique_ptr.

> Source/JavaScriptCore/wasm/WasmFormat.cpp:52
>      return segment;

Let's return `return Segment::adoptPtr(segment)`.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:759
> +            Segment* segment = Segment::create(*initExpr, dataByteLength, Segment::Kind::Active);

Let's receive this as `auto segment = Segment::create(*initExpr, dataByteLength, Segment::Kind::Active);`.
And let's keep Segment* pointer by doing like, `Segment* segmentPtr = segment.get();`.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:761
> +            m_info->data.uncheckedAppend(Segment::adoptPtr(segment));

Let's pass it with `WTFMove(segment)`.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:765
> +                segment->byte(dataByte) = byte;

Use segmentPtr to access since segment is already moved.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:768
> +        }

We can insert `ASSERT(Options::useWebAssemblyReferences())`.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:775
> +            Segment* segment = Segment::create(WTF::nullopt, dataByteLength, Segment::Kind::Passive);

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:777
> +            m_info->data.uncheckedAppend(Segment::adoptPtr(segment));

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:781
> +                segment->byte(dataByte) = byte;

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:788
> +            WASM_PARSER_FAIL_IF(!Options::useWebAssemblyReferences(), "references are not enabled");

Let's remove this line since if `!Options::useWebAssemblyReferences()`, then execution never reaches here since we have `if (!Options::useWebAssemblyReferences() || !dataFlag) {`.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:800
> +            Segment* segment = Segment::create(*initExpr, dataByteLength, Segment::Kind::Active);

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:802
> +            m_info->data.uncheckedAppend(Segment::adoptPtr(segment));

Ditto.

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:806
> +                segment->byte(dataByte) = byte;

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/20201217/c806dcfb/attachment.htm>