[wasm] Implement Table#set and Table#grow

[wasm] Implement Table#set and Table#grow

R=ahaas@chromium.org
BUG=

Committed: https://crrev.com/4a90e7220c7efcc0c08335e3436e1745b1b562ce
Cr-Commit-Position: refs/heads/master@{#40357}

[ahaas, 2016-10-13 14:25:09]

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js
File test/mjsunit/wasm/table.js (right):

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:161: for (let i = 0; i < 10; ++i)
table.set(String(i), f);
Could you do the last two for loops in a separate "for (let f of [null, wasm,
host]) {" loop? Otherwise the values you add to the table are already contained
in the table.

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:163: 
Could you also add a test where you check that the value is actually set at the
correct index?

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:170: for (let key of [10, -1]) {
According to the spec a RangeError should be thrown if key is negative, see
https://github.com/WebAssembly/design/blob/master/JS.md#tononwrappinguint32

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:171: table.set(key, f);  // ignored
Should we adjust the spec to specify what should happen if the key is out of
bounds?

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:184: }
Do you check somewhere already that table.set returns undefined?

[rossberg, 2016-10-13 14:43:46]

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js
File test/mjsunit/wasm/table.js (right):

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:161: for (let i = 0; i < 10; ++i)
table.set(String(i), f);
On 2016/10/13 14:25:09, ahaas wrote:
> Could you do the last two for loops in a separate "for (let f of [null, wasm,
> host]) {" loop? Otherwise the values you add to the table are already
contained
> in the table.

Done.

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:163: 
On 2016/10/13 14:25:09, ahaas wrote:
> Could you also add a test where you check that the value is actually set at
the
> correct index?

Done.

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:170: for (let key of [10, -1]) {
On 2016/10/13 14:25:09, ahaas wrote:
> According to the spec a RangeError should be thrown if key is negative, see
> https://github.com/WebAssembly/design/blob/master/JS.md#tononwrappinguint32

Done.

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:171: table.set(key, f);  // ignored
On 2016/10/13 14:25:09, ahaas wrote:
> Should we adjust the spec to specify what should happen if the key is out of
> bounds? 

Yes, probably. I changed the implementation to throw consistently. Will make a
design PR.

https://codereview.chromium.org/2414053002/diff/1/test/mjsunit/wasm/table.js#...
test/mjsunit/wasm/table.js:184: }
On 2016/10/13 14:25:09, ahaas wrote:
> Do you check somewhere already that table.set returns undefined?

No, done.

[ahaas, 2016-10-17 09:36:35]

lgtm

[rossberg, 2016-10-17 09:53:25]

The CQ bit was checked by rossberg@chromium.org

[commit-bot: I haz the power, 2016-10-17 09:53:37]

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-17 09:55:06]

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

[commit-bot: I haz the power, 2016-10-17 09:55:07]

Try jobs failed on following builders:
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/14650)

[rossberg, 2016-10-17 10:44:26]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-10-17 10:44:26]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org
Link to the patchset: https://codereview.chromium.org/2414053002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-10-17 10:44:30]

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-17 10:48:18]

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

[commit-bot: I haz the power, 2016-10-17 10:48:18]

Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)

[rossberg, 2016-10-17 11:15:33]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-10-17 11:15:33]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org
Link to the patchset: https://codereview.chromium.org/2414053002/#ps60001
(title: "Avoid lossy int conversion")

[commit-bot: I haz the power, 2016-10-17 11:15:44]

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-17 11:39:11]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-17 11:39:40]

Description was changed from

==========
[wasm] Implement Table#set and Table#grow

R=ahaas@chromium.org
BUG=
==========

to

==========
[wasm] Implement Table#set and Table#grow

R=ahaas@chromium.org
BUG=

Committed: https://crrev.com/4a90e7220c7efcc0c08335e3436e1745b1b562ce
Cr-Commit-Position: refs/heads/master@{#40357}
==========

[commit-bot: I haz the power, 2016-10-17 11:39:41]

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