Bug 178690

Summary:

WebAssembly: topEntryFrame on Wasm::Instance

Product:

WebKit

Reporter:

JF Bastien <jfbastien>

Component:

WebAssembly

Assignee:

JF Bastien <jfbastien>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, fpizlo, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

178699

Bug Blocks:

177472

Attachments:

Description	Flags
patch	saam: review+
patch	none

Description JF Bastien 2017-10-23 16:37:09 PDT

topEntryFrame is usually on VM, but for a no-VM WebAssembly we need to hold topEntryFrame elsewhere, and generated code cannot hard-code where topEntryFrame live. Do this at creation time of Wasm::Instance, and then generated code will just load from wherever Wasm::Instance was told topEntryFrame is. In a JavaScript embedding this is still from VM, so all of the unwinding machinery stays the same.

Comment 1 JF Bastien 2017-10-23 16:46:47 PDT

Created attachment 324611 [details]
patch

Comment 2 Saam Barati 2017-10-23 16:49:35 PDT

Comment on attachment 324611 [details]
patch

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

> Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.h:137
> +    EntryFrame** m_topEntryFramePointer { nullptr };

Why have this in both JSWebAssemblyInstance and WasmInstance?

Comment 3 JF Bastien 2017-10-23 16:51:48 PDT

(In reply to Saam Barati from comment #2)
> Comment on attachment 324611 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324611&action=review
> 
> > Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.h:137
> > +    EntryFrame** m_topEntryFramePointer { nullptr };
> 
> Why have this in both JSWebAssemblyInstance and WasmInstance?

JSWebAssemblyInstance copies a bunch of things from Wasm::Instance for now, because the goal I'm driving towards is having Wasm::Context be Wasm::Instance (not JSWebAssemblyInstance), and I want the swap to be as trivial as possible (i.e. change one line to do that swap, then delete all the copies from JSWebAssemblyInstance). If I do it right everything else will be the same.

Comment 4 JF Bastien 2017-10-23 16:54:31 PDT

Created attachment 324613 [details]
patch

Comment 5 WebKit Commit Bot 2017-10-23 17:29:43 PDT

Comment on attachment 324613 [details]
patch

Clearing flags on attachment: 324613

Committed r223866: <https://trac.webkit.org/changeset/223866>

Comment 6 WebKit Commit Bot 2017-10-23 17:29:45 PDT

All reviewed patches have been landed.  Closing bug.

Comment 7 Keith Miller 2017-10-23 19:01:52 PDT

I think this broke the windows build? https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/5466/steps/compile-webkit/logs/stdio

Comment 8 WebKit Commit Bot 2017-10-23 19:31:27 PDT

Re-opened since this is blocked by bug 178699

Comment 9 Keith Miller 2017-10-23 20:20:37 PDT

Committed r223875: <https://trac.webkit.org/changeset/223875>

Comment 10 JF Bastien 2017-10-23 21:58:33 PDT

(In reply to Keith Miller from comment #7)
> I think this broke the windows build?
> https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/
> 5466/steps/compile-webkit/logs/stdio

lolwut?

Comment 11 Radar WebKit Bug Importer 2017-11-15 12:58:18 PST

<rdar://problem/35568509>