[wasm] Identify wasm functions with index into the function tables.

[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag wasm functions with the index into the function table to identify functions. Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Original-Commit-Position: refs/heads/master@{#43296}
Committed: https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b394b296
Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43338}
Committed: https://chromium.googlesource.com/v8/v8/+/684323b45c913e8a3dd8f80fc675bb45bc947746

[gdeepti, 2017-02-14 20:04:02]

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

[commit-bot: I haz the power, 2017-02-14 20:04:08]

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-02-14 20:08:58]

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

[commit-bot: I haz the power, 2017-02-14 20:08:58]

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...)

[gdeepti, 2017-02-14 20:13:27]

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

[commit-bot: I haz the power, 2017-02-14 20:13:38]

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-02-14 20:34:20]

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

[commit-bot: I haz the power, 2017-02-14 20:34:21]

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

[Mircea Trofin, 2017-02-14 20:36:53]

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.h (right):

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:52: wasm::WasmName function_name);
You could pass this as const wasm::WasmName&, too.

I'd rename the parameter from "function_name" to "reportable_function_name" or
something like that. The reader may be surprised you pass in the function name
here, when there's enough context to fetch it from the decoded module already.

Alternatively - let WasmCompilation unit call that API that manufactures a name
if one isn't available.

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:64: wasm::WasmName function_name) {
See previous comment - move the API for computing the name in wasm compilation
unit.

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

https://codereview.chromium.org/2690113012/diff/40001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:265: Vector<char> buffer = Vector<char>::New(128);
who disposes the buffer?

If you move this API to be a member of WasmCompilationUnit, then you can keep
track of the Vector there, and Dispose it at destruction time. You'd probably
want to remember if you own it or if the WasmModule owns it (case in which you
don't dispose it)

[gdeepti, 2017-02-14 23:09:07]

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

[commit-bot: I haz the power, 2017-02-14 23:09:18]

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-02-14 23:31:47]

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

[commit-bot: I haz the power, 2017-02-14 23:31:48]

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

[gdeepti, 2017-02-15 00:13:07]

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

[commit-bot: I haz the power, 2017-02-15 00:13:13]

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-02-15 00:41:22]

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

[commit-bot: I haz the power, 2017-02-15 00:41:23]

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

[gdeepti, 2017-02-17 17:34:08]

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

[commit-bot: I haz the power, 2017-02-17 17:34:21]

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

[gdeepti, 2017-02-17 17:52:37]

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.h (right):

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:52: wasm::WasmName function_name);
On 2017/02/14 20:36:53, Mircea Trofin wrote:
> You could pass this as const wasm::WasmName&, too.
> 
> I'd rename the parameter from "function_name" to "reportable_function_name" or
> something like that. The reader may be surprised you pass in the function name
> here, when there's enough context to fetch it from the decoded module already.
> 
> Alternatively - let WasmCompilation unit call that API that manufactures a
name
> if one isn't available.

Moved function to WasmCompilationUnit that is called only when a name is not
available.

https://codereview.chromium.org/2690113012/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:64: wasm::WasmName function_name) {
On 2017/02/14 20:36:53, Mircea Trofin wrote:
> See previous comment - move the API for computing the name in wasm compilation
> unit.

Done.

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

https://codereview.chromium.org/2690113012/diff/40001/src/wasm/wasm-module.cc...
src/wasm/wasm-module.cc:265: Vector<char> buffer = Vector<char>::New(128);
On 2017/02/14 20:36:53, Mircea Trofin wrote:
> who disposes the buffer?
> 
> If you move this API to be a member of WasmCompilationUnit, then you can keep
> track of the Vector there, and Dispose it at destruction time. You'd probably
> want to remember if you own it or if the WasmModule owns it (case in which you
> don't dispose it)

Added char buffer scoped to WasmCompilationUnit.

[commit-bot: I haz the power, 2017-02-17 17:58:06]

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

[commit-bot: I haz the power, 2017-02-17 17:58:07]

Dry run: This issue passed the CQ dry run.

[Mircea Trofin, 2017-02-17 18:47:45]

lgtm with 1 nit

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.h (right):

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:77: char function_name_[128];
why 128? From what I see, we generate a string of the form wasm#<number>, and
number is at most 2^32, which has, in decimal, 10 digits.  So that's what, 15+1
so 16 (but please check my math).

We should also put a comment explaining how we came with the number.

[gdeepti, 2017-02-18 01:05:55]

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

[commit-bot: I haz the power, 2017-02-18 01:05:58]

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-02-18 01:49:49]

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

[commit-bot: I haz the power, 2017-02-18 01:49:50]

Dry run: This issue passed the CQ dry run.

[gdeepti, 2017-02-18 01:57:35]

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.h (right):

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:77: char function_name_[128];
On 2017/02/17 18:47:45, Mircea Trofin wrote:
> why 128? From what I see, we generate a string of the form wasm#<number>, and
> number is at most 2^32, which has, in decimal, 10 digits.  So that's what,
15+1
> so 16 (but please check my math).
> 
> We should also put a comment explaining how we came with the number.

Done.

[gdeepti, 2017-02-18 01:57:45]

The CQ bit was checked by gdeepti@chromium.org

[gdeepti, 2017-02-18 01:57:49]

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.h (right):

https://codereview.chromium.org/2690113012/diff/90001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.h:77: char function_name_[128];
On 2017/02/17 18:47:45, Mircea Trofin wrote:
> why 128? From what I see, we generate a string of the form wasm#<number>, and
> number is at most 2^32, which has, in decimal, 10 digits.  So that's what,
15+1
> so 16 (but please check my math).
> 
> We should also put a comment explaining how we came with the number.

Done.

[gdeepti, 2017-02-18 01:57:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mtrofin@chromium.org
Link to the patchset: https://codereview.chromium.org/2690113012/#ps110001
(title: "Review")

[commit-bot: I haz the power, 2017-02-18 01:58:01]

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-02-18 01:59:31]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1487383065856990,
"parent_rev": "d9bc0ffb16e633d52d7bcfd547a6125f0e4dfb87", "commit_rev":
"5fc3ac29e4d942ccb4c45f6cdcee75d0b394b296"}

[commit-bot: I haz the power, 2017-02-18 01:59:46]

Description was changed from

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org
==========

to

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43296}
Committed:
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...
==========

[commit-bot: I haz the power, 2017-02-18 01:59:47]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...

[Michael Hablich, 2017-02-20 07:30:30]

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

The reason for reverting is: Introduces a new test failure/flake:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20debug/builds....

[gdeepti, 2017-02-21 08:39:12]

Description was changed from

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43296}
Committed:
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...
==========

to

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43296}
Committed:
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...
==========

[gdeepti, 2017-02-21 08:39:59]

On 2017/02/20 07:30:30, Michael Hablich wrote:
> A revert of this CL (patchset #7 id:110001) has been created in
> https://codereview.chromium.org/2708593002/ by mailto:hablich@chromium.org.
> 
> The reason for reverting is: Introduces a new test failure/flake:
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20debug/builds....

Relanding as failing test has been fixed.

[gdeepti, 2017-02-21 08:40:08]

The CQ bit was checked by gdeepti@chromium.org

[commit-bot: I haz the power, 2017-02-21 08:40:15]

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-02-21 09:30:25]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1487666408679760,
"parent_rev": "c6ce410fbfcf866f4579171ad4215258d1fa9716", "commit_rev":
"684323b45c913e8a3dd8f80fc675bb45bc947746"}

[commit-bot: I haz the power, 2017-02-21 09:30:30]

Description was changed from

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43296}
Committed:
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...
==========

to

==========
[wasm] Identify wasm functions with index into the function tables.

Currently, the default name for wasm functions in generated code is 'wasm', tag
wasm functions with the index into the function table to identify functions.
Snippets of sample output with --print-code below.

Before:
--- Code ---
kind = WASM_FUNCTION
name = wasm
compiler = turbofan

After:
--- Code ---
kind = WASM_FUNCTION
name = wasm#200
compiler = turbofan

R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2690113012
Cr-Original-Commit-Position: refs/heads/master@{#43296}
Committed:
https://chromium.googlesource.com/v8/v8/+/5fc3ac29e4d942ccb4c45f6cdcee75d0b39...
Review-Url: https://codereview.chromium.org/2690113012
Cr-Commit-Position: refs/heads/master@{#43338}
Committed:
https://chromium.googlesource.com/v8/v8/+/684323b45c913e8a3dd8f80fc675bb45bc9...
==========

[commit-bot: I haz the power, 2017-02-21 09:30:31]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/v8/v8/+/684323b45c913e8a3dd8f80fc675bb45bc9...