[wasm] Support for restricted table imports.

[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
Committed: https://crrev.com/b7aff1ff647595b7e529b6df0c719d513473703a
Cr-Commit-Position: refs/heads/master@{#40661}

[titzer, 2016-10-27 20:33:35]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesnt change dispatch tables
Missing: updates of tables (WebAssembly.Set) doesnt change dispatch tables
Missing: consistency across mutations of tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

[titzer, 2016-10-27 20:33:49]

The CQ bit was checked by titzer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 20:33:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[titzer, 2016-10-27 20:34:29]

PTAL. This is missing some core functionality, but might be worth landing in its
incomplete state nonetheless.

[commit-bot: I haz the power, 2016-10-27 20:39:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 20:39:02]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/16882)

[titzer, 2016-10-28 09:49:10]

The CQ bit was checked by titzer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 09:49:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 10:14:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 10:14:14]

Dry run: This issue passed the CQ dry run.

[titzer, 2016-10-28 13:01:18]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesnt change dispatch tables
Missing: updates of tables (WebAssembly.Set) doesnt change dispatch tables
Missing: consistency across mutations of tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

[titzer, 2016-10-28 14:15:32]

The CQ bit was checked by titzer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 14:15:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[titzer, 2016-10-28 14:15:54]

On 2016/10/27 20:34:29, titzer wrote:
> PTAL. This is missing some core functionality, but might be worth landing in
its
> incomplete state nonetheless.

PTAL, I've implemented the proper SET functionality and added tests.

[commit-bot: I haz the power, 2016-10-28 14:43:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 14:43:48]

Dry run: This issue passed the CQ dry run.

[titzer, 2016-10-28 16:58:07]

The CQ bit was checked by titzer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 16:58:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bradnelson, 2016-10-28 16:59:01]

lgtm w/questions

https://codereview.chromium.org/2454503005/diff/60001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:2155: EnsureFunctionTableNodes();
Don't care for Ensure, but not sure a good alternative.
I assume Domenic stuck this in here lazily to keep it localized.
Any reason this can't be done up front?

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:32: static const int kWasmTableDispatchTablesFieldIndex = 2;
Any reason not to move these into the enum?

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:861: static bool IsBrand(i::Handle<i::Object> value,
i::Handle<i::Symbol> symbol) {
HasBrand is a little better.

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:945: GetWasmFunctionForImportWrapper(isolate, function),
Seems like CompileImportWrapper below could get used / moved up here?

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:180: static const size_t kV8MaxTableSize = 16 * 1024 *
1024;
Reasonable limit, any particular motivation for this value?

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
File test/mjsunit/wasm/indirect-tables.js (right):

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:201: let table = new
WebAssembly.Table({element: "anyfunc", initial: kTableSize});
>80

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:203: let i2 = new WebAssembly.Instance(m2,
{base: i, table: table, js_div: js_div});
>80

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:342: // TODO(titzer): a test that all
instances using a table get updated
Also one for import table size doesn't match initial size?

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/wasm-...
File test/mjsunit/wasm/wasm-module-builder.js (right):

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:208: let o = {module: module, name:
name, kind: kExternalTable, initial: initial, maximum: maximum};
>80

[titzer, 2016-10-28 17:29:09]

https://codereview.chromium.org/2454503005/diff/60001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:2155: EnsureFunctionTableNodes();
On 2016/10/28 16:59:01, bradnelson wrote:
> Don't care for Ensure, but not sure a good alternative.
> I assume Domenic stuck this in here lazily to keep it localized.
> Any reason this can't be done up front?

Just to save nodes, since I assume most functions don't have direct calls. The
previous code was allocating new nodes every time, which isn't a huge deal. I
also made it easier to extend to multiple tables.

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:32: static const int kWasmTableDispatchTablesFieldIndex = 2;
On 2016/10/28 16:59:01, bradnelson wrote:
> Any reason not to move these into the enum?

Yeah, would be OK.

I'm planning on moving this all to a wasm-objects.h header and hiding all the
gunk behind a nicer typed interface, so no sense cleaning up too much now, IMHO.

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:861: static bool IsBrand(i::Handle<i::Object> value,
i::Handle<i::Symbol> symbol) {
On 2016/10/28 16:59:01, bradnelson wrote:
> HasBrand is a little better.

Done.

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:945: GetWasmFunctionForImportWrapper(isolate, function),
On 2016/10/28 16:59:01, bradnelson wrote:
> Seems like CompileImportWrapper below could get used / moved up here?

Done.

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2454503005/diff/60001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:180: static const size_t kV8MaxTableSize = 16 * 1024 *
1024;
On 2016/10/28 16:59:01, bradnelson wrote:
> Reasonable limit, any particular motivation for this value?

Just spitballing a reasonable limit on table size; since each entry is 16 bytes
on 64-bit, that's about 128MB.

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
File test/mjsunit/wasm/indirect-tables.js (right):

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:201: let table = new
WebAssembly.Table({element: "anyfunc", initial: kTableSize});
On 2016/10/28 16:59:01, bradnelson wrote:
> >80

Done.

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:203: let i2 = new WebAssembly.Instance(m2,
{base: i, table: table, js_div: js_div});
On 2016/10/28 16:59:01, bradnelson wrote:
> >80

Done.

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/indir...
test/mjsunit/wasm/indirect-tables.js:342: // TODO(titzer): a test that all
instances using a table get updated
On 2016/10/28 16:59:01, bradnelson wrote:
> Also one for import table size doesn't match initial size?

Done.

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/wasm-...
File test/mjsunit/wasm/wasm-module-builder.js (right):

https://codereview.chromium.org/2454503005/diff/60001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:208: let o = {module: module, name:
name, kind: kExternalTable, initial: initial, maximum: maximum};
On 2016/10/28 16:59:01, bradnelson wrote:
> >80

Done.

[bradnelson, 2016-10-28 17:31:24]

lgtm

[titzer, 2016-10-28 17:35:36]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2016-10-28 17:35:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 18:03:28]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

[commit-bot: I haz the power, 2016-10-28 18:03:29]

Committed patchset #7 (id:120001)

[Michael Achenbach, 2016-10-28 18:58:23]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/2456193006/ by machenbach@chromium.org.

The reason for reverting is: GC stress failures:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20....

[titzer, 2016-10-29 20:07:24]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

[titzer, 2016-10-29 20:09:20]

The CQ bit was checked by titzer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-29 20:09:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[titzer, 2016-10-29 21:04:03]

The CQ bit was unchecked by titzer@chromium.org

[titzer, 2016-10-29 21:04:09]

The CQ bit was checked by titzer@chromium.org

[titzer, 2016-10-29 21:04:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
bradnelson@chromium.org
Link to the patchset: https://codereview.chromium.org/2454503005/#ps140001
(title: "Fix GC stress issue; tables weren't being reset correctly")

[commit-bot: I haz the power, 2016-10-29 21:04:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-29 21:07:06]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

[commit-bot: I haz the power, 2016-10-29 21:07:07]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-11-17 22:17:04]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507

Committed: https://crrev.com/404e215458df2676f21017cec00a7ae51fb163c1
Cr-Commit-Position: refs/heads/master@{#40652}
==========

[commit-bot: I haz the power, 2016-11-17 22:17:05]

Patchset 8 (id:??) landed as
https://crrev.com/404e215458df2676f21017cec00a7ae51fb163c1
Cr-Commit-Position: refs/heads/master@{#40652}

[commit-bot: I haz the power, 2016-11-17 22:17:33]

Description was changed from

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507

Committed: https://crrev.com/404e215458df2676f21017cec00a7ae51fb163c1
Cr-Commit-Position: refs/heads/master@{#40652}
==========

to

==========
[wasm] Support for restricted table imports.

This CL implements basic table import functionality.

Missing: growing of tables (WebAssembly.Grow) doesn't change dispatch tables
Missing: allowing larger table imports than minimum size

R=rossberg@chromium.org,bradnelson@chromium.org
BUG=v8:5507

Committed: https://crrev.com/b7aff1ff647595b7e529b6df0c719d513473703a
Cr-Commit-Position: refs/heads/master@{#40661}
==========

[commit-bot: I haz the power, 2016-11-17 22:17:35]

Patchset 8 (id:??) landed as
https://crrev.com/b7aff1ff647595b7e529b6df0c719d513473703a
Cr-Commit-Position: refs/heads/master@{#40661}