[wasm] Store the function_index in the js-to-wasm wrapper instead of the export index

[wasm] Store the function_index directly in the js-to-wasm wrapper.

If a WebAssembly function is exported, its js-to-wasm wrapper has a
field which contains a reference to the WebAssembly function.
Originally this reference was an index into the export table, which
then contains an index into the function table, which then contains
the metadata of the WebAssembly function.

With this CL we use the index into the function table directly as
the reference to the WebAssembly function.

TEST=mjsunit/wasm/test-import-export-wrapper
R=rossberg@chromium.org, mtrofin@chromium.org
CC=titzer@chromium.org

Committed: https://crrev.com/7f58be6b3852dad599ddfeb9c3554900693e0db6
Cr-Commit-Position: refs/heads/master@{#40729}

[ahaas, 2016-11-03 12:07:42]

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

[commit-bot: I haz the power, 2016-11-03 12:07:54]

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-11-03 12:21:41]

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

[commit-bot: I haz the power, 2016-11-03 12:21:42]

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

[ahaas, 2016-11-03 12:41:54]

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

[commit-bot: I haz the power, 2016-11-03 12:42:05]

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

[ahaas, 2016-11-03 12:46:43]

Description was changed from

==========
[wasm] Use the export table indirection in GetWasmFunctionForImportWrapper

If a WebAssembly function is exported, its js-to-wasm wrapper has a
field which contains the index of the WebAssembly function in the export
table. At this index the export table contains the function index of the
exported function. The function index can then be used get the meta data
of the exported function from the function table.

The indirection over the export table was missing in
GetWasmFunctionForImportWrapper. This CL adds this indirection over the
export table, and also renames some variables from func_index to
export_index to make the indirection more obvious.

TEST=mjsunit/wasm/test-import-export-wrapper
R=rossberg@chromium.org, mtrofin@chromium.org
CC=titzer@chromium.org
==========

to

==========
[wasm] Store the function_index directly in the js-to-wasm wrapper.

If a WebAssembly function is exported, its js-to-wasm wrapper has a
field which contains a reference to the WebAssembly function.
Originally this reference was an index into the export table, which
then contains an index into the function table, which then contains
the metadata of the WebAssembly function.

With this CL we use the index into the function table directly as
the reference to the WebAssembly function.

TEST=mjsunit/wasm/test-import-export-wrapper
R=rossberg@chromium.org, mtrofin@chromium.org
CC=titzer@chromium.org
==========

[ahaas, 2016-11-03 12:47:42]

On 2016/11/03 at 12:07:40, ahaas wrote:
> 

PTAL. I changed the CL so that it now stores the function_index in the
js-to-wasm wrapper instead of the export index.

[titzer, 2016-11-03 12:56:44]

titzer@chromium.org changed reviewers:
+ titzer@chromium.org

[titzer, 2016-11-03 12:56:45]

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

https://codereview.chromium.org/2472103002/diff/20001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:1630: int func_index = 0;
The renaming of this variable that you had done before (to export_index) was
correct, so let's keep that too.

Otherwise LGTM

[ahaas, 2016-11-03 13:02:11]

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

https://codereview.chromium.org/2472103002/diff/20001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:1630: int func_index = 0;
On 2016/11/03 at 12:56:45, titzer wrote:
> The renaming of this variable that you had done before (to export_index) was
correct, so let's keep that too.
> 
> Otherwise LGTM

Done.

[ahaas, 2016-11-03 13:02:18]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-11-03 13:02:19]

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

[commit-bot: I haz the power, 2016-11-03 13:02:27]

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-11-03 14:28:27]

Committed patchset #3 (id:40001)

[Mircea Trofin, 2016-11-03 14:38:29]

lgtm, mini-nits

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
File test/mjsunit/wasm/test-import-export-wrapper.js (right):

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
test/mjsunit/wasm/test-import-export-wrapper.js:72: // So that wrappers will be
removed. If the wrappers do are not removed, then
remove "do" (If the wrappers...)

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
test/mjsunit/wasm/test-import-export-wrapper.js:73: // the test crashes because
of the int64 parameter, which is not allowed in the
while at it - the test fails, right? (OK, maybe it crashes as a form of failure,
but that's what we want in that case, right?)

[ahaas, 2016-11-07 08:10:51]

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
File test/mjsunit/wasm/test-import-export-wrapper.js (right):

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
test/mjsunit/wasm/test-import-export-wrapper.js:72: // So that wrappers will be
removed. If the wrappers do are not removed, then
On 2016/11/03 at 14:38:29, Mircea Trofin wrote:
> remove "do" (If the wrappers...)

I fixed it in https://codereview.chromium.org/2481133002

https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
test/mjsunit/wasm/test-import-export-wrapper.js:73: // the test crashes because
of the int64 parameter, which is not allowed in the
On 2016/11/03 at 14:38:29, Mircea Trofin wrote:
> while at it - the test fails, right? (OK, maybe it crashes as a form of
failure, but that's what we want in that case, right?)

I do not understand your question. This test would fail without the changes to
wasm-module.cc, but with the changes it does not fail. Without changes the test
would fail with a runtime error.

[Mircea Trofin, 2016-11-07 15:41:40]

On 2016/11/07 08:10:51, ahaas wrote:
>
https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
> File test/mjsunit/wasm/test-import-export-wrapper.js (right):
> 
>
https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
> test/mjsunit/wasm/test-import-export-wrapper.js:72: // So that wrappers will
be
> removed. If the wrappers do are not removed, then
> On 2016/11/03 at 14:38:29, Mircea Trofin wrote:
> > remove "do" (If the wrappers...)
> 
> I fixed it in https://codereview.chromium.org/2481133002
> 
>
https://codereview.chromium.org/2472103002/diff/40001/test/mjsunit/wasm/test-...
> test/mjsunit/wasm/test-import-export-wrapper.js:73: // the test crashes
because
> of the int64 parameter, which is not allowed in the
> On 2016/11/03 at 14:38:29, Mircea Trofin wrote:
> > while at it - the test fails, right? (OK, maybe it crashes as a form of
> failure, but that's what we want in that case, right?)
> 
> I do not understand your question. This test would fail without the changes to
> wasm-module.cc, but with the changes it does not fail. Without changes the
test
> would fail with a runtime error.

Oh - the text says "the test crashes because of the int64 parameter". My
question was if "the test fails" is what you meant to say
instead. (just a nit, really)

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

Description was changed from

==========
[wasm] Store the function_index directly in the js-to-wasm wrapper.

If a WebAssembly function is exported, its js-to-wasm wrapper has a
field which contains a reference to the WebAssembly function.
Originally this reference was an index into the export table, which
then contains an index into the function table, which then contains
the metadata of the WebAssembly function.

With this CL we use the index into the function table directly as
the reference to the WebAssembly function.

TEST=mjsunit/wasm/test-import-export-wrapper
R=rossberg@chromium.org, mtrofin@chromium.org
CC=titzer@chromium.org
==========

to

==========
[wasm] Store the function_index directly in the js-to-wasm wrapper.

If a WebAssembly function is exported, its js-to-wasm wrapper has a
field which contains a reference to the WebAssembly function.
Originally this reference was an index into the export table, which
then contains an index into the function table, which then contains
the metadata of the WebAssembly function.

With this CL we use the index into the function table directly as
the reference to the WebAssembly function.

TEST=mjsunit/wasm/test-import-export-wrapper
R=rossberg@chromium.org, mtrofin@chromium.org
CC=titzer@chromium.org

Committed: https://crrev.com/7f58be6b3852dad599ddfeb9c3554900693e0db6
Cr-Commit-Position: refs/heads/master@{#40729}
==========

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

Patchset 3 (id:??) landed as
https://crrev.com/7f58be6b3852dad599ddfeb9c3554900693e0db6
Cr-Commit-Position: refs/heads/master@{#40729}