[wasm] Instantiate the interpreter on demand

[wasm] Instantiate the interpreter on demand

If a breakpoint is set on a wasm function, compile an interpreter entry
stub for it, and replace all calls to the original function by calls to
this interpreter entry.
Also, instantiate a wasm interpreter object on demand and set the
breakpoint there.

R=titzer@chromium.org
BUG=v8:5822
Review-Url: https://codereview.chromium.org/2625093004
Cr-Commit-Position: refs/heads/master@{#42309}
Committed: https://chromium.googlesource.com/v8/v8/+/eb04a25f1f61611588bf3ccf0bb564ddc2a872e3

[Clemens Hammacher, 2017-01-12 11:23:22]

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

[commit-bot: I haz the power, 2017-01-12 11:23:33]

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, 2017-01-12 11:35:04]

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

[commit-bot: I haz the power, 2017-01-12 11:35:05]

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

[Clemens Hammacher, 2017-01-12 11:58:20]

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

[commit-bot: I haz the power, 2017-01-12 11:58:24]

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, 2017-01-12 12:30:11]

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

[commit-bot: I haz the power, 2017-01-12 12:30:13]

Dry run: This issue passed the CQ dry run.

[titzer, 2017-01-12 15:32:05]

lgtm with comments

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

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:117: DCHECK_GE(1, sig->return_count());
I think you should check against kV8MaxWasmFunctionReturns here, and a add a
TODO about multi-value returns.

Come to think of it, you should also add this in the wasm-compiler.cc where it
compiles the stubs.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:186: for (int i = 0, e = GetNumFunctions(instance); i <
e; ++i)
Please add brackets.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:190: // Redirect in all exported functions.
"Redirect all calls in exported functions"? and move a line down.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:209: DCHECK_GT(interpreted_functions->length(),
func_index);
The FixedArray should do this bounds check for you.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:219: DCHECK_GT(code_table->length(), func_index);
Same here. The FixedArray will bounds check for you.

[Clemens Hammacher, 2017-01-12 19:19:32]

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

[commit-bot: I haz the power, 2017-01-12 19:19:37]

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

[Clemens Hammacher, 2017-01-12 19:19:54]

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

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:117: DCHECK_GE(1, sig->return_count());
On 2017/01/12 at 15:32:05, titzer wrote:
> I think you should check against kV8MaxWasmFunctionReturns here, and a add a
TODO about multi-value returns.
> 
> Come to think of it, you should also add this in the wasm-compiler.cc where it
compiles the stubs.

Done.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:186: for (int i = 0, e = GetNumFunctions(instance); i <
e; ++i)
On 2017/01/12 at 15:32:05, titzer wrote:
> Please add brackets.

Done.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:190: // Redirect in all exported functions.
On 2017/01/12 at 15:32:05, titzer wrote:
> "Redirect all calls in exported functions"? and move a line down.

Done, and removed the DCHECK which does not really serve a purpose here...

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:209: DCHECK_GT(interpreted_functions->length(),
func_index);
On 2017/01/12 at 15:32:05, titzer wrote:
> The FixedArray should do this bounds check for you.

Yeah, but it's only SLOW_DCHECK.
I removed it for now, but I feel more comfortable with these early checks.

https://codereview.chromium.org/2625093004/diff/20001/src/wasm/wasm-debug.cc#...
src/wasm/wasm-debug.cc:219: DCHECK_GT(code_table->length(), func_index);
On 2017/01/12 at 15:32:05, titzer wrote:
> Same here. The FixedArray will bounds check for you.

Done.

[commit-bot: I haz the power, 2017-01-12 19:56:46]

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

[commit-bot: I haz the power, 2017-01-12 19:56:47]

Dry run: This issue passed the CQ dry run.

[Clemens Hammacher, 2017-01-13 09:02:21]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2017-01-13 09:02:22]

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/2625093004/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2017-01-13 09:02:24]

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, 2017-01-13 09:04:01]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484298141889750,
"parent_rev": "865b5e579561146a583cf2f9cb7266a6498d12f7", "commit_rev":
"eb04a25f1f61611588bf3ccf0bb564ddc2a872e3"}

[commit-bot: I haz the power, 2017-01-13 09:04:07]

Description was changed from

==========
[wasm] Instantiate the interpreter on demand

If a breakpoint is set on a wasm function, compile an interpreter entry
stub for it, and replace all calls to the original function by calls to
this interpreter entry.
Also, instantiate a wasm interpreter object on demand and set the
breakpoint there.

R=titzer@chromium.org
BUG=v8:5822
==========

to

==========
[wasm] Instantiate the interpreter on demand

If a breakpoint is set on a wasm function, compile an interpreter entry
stub for it, and replace all calls to the original function by calls to
this interpreter entry.
Also, instantiate a wasm interpreter object on demand and set the
breakpoint there.

R=titzer@chromium.org
BUG=v8:5822

Review-Url: https://codereview.chromium.org/2625093004
Cr-Commit-Position: refs/heads/master@{#42309}
Committed:
https://chromium.googlesource.com/v8/v8/+/eb04a25f1f61611588bf3ccf0bb564ddc2a...
==========

[commit-bot: I haz the power, 2017-01-13 09:04:08]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/eb04a25f1f61611588bf3ccf0bb564ddc2a...