[wasm] Decouple function name and exported name in WasmFunctionBuilder

[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203
Committed: https://crrev.com/4f9976aa6838ca67754d570d0896e2a073355547
Cr-Commit-Position: refs/heads/master@{#40159}

[Clemens Hammacher, 2016-10-11 06:57:13]

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

[commit-bot: I haz the power, 2016-10-11 06:57:19]

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-11 07:13:48]

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

[commit-bot: I haz the power, 2016-10-11 07:13:49]

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

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

lgtm. Nit: it would be nice to add a small test here.

[titzer, 2016-10-11 08:08:38]

On 2016/10/11 07:16:51, ahaas wrote:
> lgtm. Nit: it would be nice to add a small test here.

lgtm modulo naming

[titzer, 2016-10-11 08:08:48]

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder.cc
File src/asmjs/asm-wasm-builder.cc (right):

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder....
src/asmjs/asm-wasm-builder.cc:567: function->SetExportedWithName(
How about ExportAs()?

(we should work incrementally to make the C++ WasmModuleBuilder more directly
reflect the JS version)

[Clemens Hammacher, 2016-10-11 08:21:55]

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder.cc
File src/asmjs/asm-wasm-builder.cc (right):

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder....
src/asmjs/asm-wasm-builder.cc:567: function->SetExportedWithName(
On 2016/10/11 08:08:48, titzer wrote:
> How about ExportAs()?
> 
> (we should work incrementally to make the C++ WasmModuleBuilder more directly
> reflect the JS version)

Is SetExportedAs better?
IMHO Export() and ExportAs() sound as if it did more than just setting a flag.

[titzer, 2016-10-11 08:36:03]

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder.cc
File src/asmjs/asm-wasm-builder.cc (right):

https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder....
src/asmjs/asm-wasm-builder.cc:567: function->SetExportedWithName(
On 2016/10/11 08:21:54, Clemens Hammacher wrote:
> On 2016/10/11 08:08:48, titzer wrote:
> > How about ExportAs()?
> > 
> > (we should work incrementally to make the C++ WasmModuleBuilder more
directly
> > reflect the JS version)
> 
> Is SetExportedAs better?
> IMHO Export() and ExportAs() sound as if it did more than just setting a flag.

Indeed, it does a memcpy of the underlying name and stores that inside, so it's
not quite a simple setter.

The JS WasmModuleBuilder is a bit nicer to use IMO, so I think we should migrate
the C++ names closer to it, rather than the other way around.

[Clemens Hammacher, 2016-10-11 08:36:15]

Description was changed from

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
==========

to

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203
==========

[Clemens Hammacher, 2016-10-11 08:42:06]

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

[commit-bot: I haz the power, 2016-10-11 08:42:10]

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-11 08:44:59]

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

[commit-bot: I haz the power, 2016-10-11 08:45:00]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  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...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14311)

[Clemens Hammacher, 2016-10-11 08:49:19]

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

[commit-bot: I haz the power, 2016-10-11 08:49:33]

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

[Clemens Hammacher, 2016-10-11 08:49:48]

On 2016/10/11 08:36:03, titzer wrote:
>
https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder.cc
> File src/asmjs/asm-wasm-builder.cc (right):
> 
>
https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder....
> src/asmjs/asm-wasm-builder.cc:567: function->SetExportedWithName(
> On 2016/10/11 08:21:54, Clemens Hammacher wrote:
> > On 2016/10/11 08:08:48, titzer wrote:
> > > How about ExportAs()?
> > > 
> > > (we should work incrementally to make the C++ WasmModuleBuilder more
> directly
> > > reflect the JS version)
> > 
> > Is SetExportedAs better?
> > IMHO Export() and ExportAs() sound as if it did more than just setting a
flag.
> 
> Indeed, it does a memcpy of the underlying name and stores that inside, so
it's
> not quite a simple setter.
> 
> The JS WasmModuleBuilder is a bit nicer to use IMO, so I think we should
migrate
> the C++ names closer to it, rather than the other way around.

Renamed to Export and ExportAs.

[commit-bot: I haz the power, 2016-10-11 08:56:42]

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

[commit-bot: I haz the power, 2016-10-11 08:56:43]

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, 2016-10-11 09:46:27]

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

[commit-bot: I haz the power, 2016-10-11 09:46:36]

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

[Clemens Hammacher, 2016-10-11 09:50:16]

On 2016/10/11 08:49:48, Clemens Hammacher wrote:
> On 2016/10/11 08:36:03, titzer wrote:
> >
>
https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder.cc
> > File src/asmjs/asm-wasm-builder.cc (right):
> > 
> >
>
https://codereview.chromium.org/2406133003/diff/1/src/asmjs/asm-wasm-builder....
> > src/asmjs/asm-wasm-builder.cc:567: function->SetExportedWithName(
> > On 2016/10/11 08:21:54, Clemens Hammacher wrote:
> > > On 2016/10/11 08:08:48, titzer wrote:
> > > > How about ExportAs()?
> > > > 
> > > > (we should work incrementally to make the C++ WasmModuleBuilder more
> > directly
> > > > reflect the JS version)
> > > 
> > > Is SetExportedAs better?
> > > IMHO Export() and ExportAs() sound as if it did more than just setting a
> flag.
> > 
> > Indeed, it does a memcpy of the underlying name and stores that inside, so
> it's
> > not quite a simple setter.
> > 
> > The JS WasmModuleBuilder is a bit nicer to use IMO, so I think we should
> migrate
> > the C++ names closer to it, rather than the other way around.
> 
> Renamed to Export and ExportAs.

... and changed the interface to accept Vector<const char> instead of const
char* and length. This avoids nasty size_t -> int conversions, or having to
choose between strlen and i::StrLength.

PTAL

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

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

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

Dry run: This issue passed the CQ dry run.

[titzer, 2016-10-11 10:31:08]

lgtm

[Clemens Hammacher, 2016-10-11 10:33:05]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2016-10-11 10:33:05]

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/2406133003/#ps60001
(title: "Change interface to Vector<const char>; avoids size_t/int confusion")

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

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-11 10:35:51]

Description was changed from

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203
==========

to

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203
==========

[commit-bot: I haz the power, 2016-10-11 10:35:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-11 10:36:09]

Description was changed from

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203
==========

to

==========
[wasm] Decouple function name and exported name in WasmFunctionBuilder

This is needed for the asm.js -> WASM pipeline. A single exported
function is exported as __single_function__, but we still want to see
the correct function name on the stack, so the underlying wasm function
has to carry the original name.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:4203

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

[commit-bot: I haz the power, 2016-10-11 10:36:10]

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