WIP: prototype ffi support

WIP: prototype ffi support

BUG=

[ofrobots, 2016-06-21 00:34:37]

Description was changed from

==========
WIP: prototype ffi support

BUG=
==========

to

==========
WIP: prototype ffi support

BUG=
==========

[ofrobots, 2016-06-21 00:39:16]

bmeurer@: this is WIP, but if you could take a peek to see if this is headed in
the right direction, that would be great. A fair amount of code is borrowed from
wasm-compiler.cc.

[Benedikt Meurer, 2016-06-21 03:26:01]

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

[Benedikt Meurer, 2016-06-21 03:26:02]

Nice prototype. Looks promising.

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.cc
File src/compiler/ffi-compiler.cc (right):

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:214: static bool CanCover(Node* value,
IrOpcode::Value opcode) {
You don't need this helper (see below).

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:253: if (CanCover(value, IrOpcode::kJSToNumber)) {
You don't need this magic. It will never trigger anyways.

https://codereview.chromium.org/2084663004/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2084663004/diff/1/src/objects.h#newcode4874
src/objects.h:4874: V(JS_TO_NATIVE_FUNCTION)
Not sure this necessary. Those functions could also use the BUILTIN kind,
similar to what we do for TurboFan builtins, i.e. MathSin and friends.

[ofrobots, 2016-06-23 01:02:51]

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.cc
File src/compiler/ffi-compiler.cc (right):

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:214: static bool CanCover(Node* value,
IrOpcode::Value opcode) {
On 2016/06/21 03:26:01, Benedikt Meurer wrote:
> You don't need this helper (see below).

Acknowledged.

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:253: if (CanCover(value, IrOpcode::kJSToNumber)) {
On 2016/06/21 03:26:01, Benedikt Meurer wrote:
> You don't need this magic. It will never trigger anyways.

Acknowledged.

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:462: ApiFunction api_func(func.start);
Any thoughts on the abuse of 'ApiFunction' here?

https://codereview.chromium.org/2084663004/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2084663004/diff/1/src/objects.h#newcode4874
src/objects.h:4874: V(JS_TO_NATIVE_FUNCTION)
On 2016/06/21 03:26:01, Benedikt Meurer wrote:
> Not sure this necessary. Those functions could also use the BUILTIN kind,
> similar to what we do for TurboFan builtins, i.e. MathSin and friends.

I think the same argument could be made for JS_TO_WASM_FUNCTION.

These functions would have 1:1 mapping to user provided function pointers. Also,
note this code in log.cc:

    case AbstractCode::BUILTIN:
      description =
          isolate_->builtins()->name(code_object->GetCode()->builtin_index());
      tag = CodeEventListener::BUILTIN_TAG;
      break;

I think all JS_TO_NATIVE_FUNCTIONs should have be considered unique (i.e.
shouldn't share a builtin_index). Are you proposing adding builtin indices for
each of these wrapper functions?

[Benedikt Meurer, 2016-06-23 03:34:07]

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.cc
File src/compiler/ffi-compiler.cc (right):

https://codereview.chromium.org/2084663004/diff/1/src/compiler/ffi-compiler.c...
src/compiler/ffi-compiler.cc:462: ApiFunction api_func(func.start);
I think it's OK for prototyping. Long-term you'll have to chat with Yang about
serializability and appropriate reloc infos for these guys.

https://codereview.chromium.org/2084663004/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2084663004/diff/1/src/objects.h#newcode4874
src/objects.h:4874: V(JS_TO_NATIVE_FUNCTION)
JS_TO_WASM is somewhat special and likely subject to change in some way.
But I you are right, I forgot about the implication of BUILTIN for logging. So,
yes, JS_TO_NATIVE_FUNCTION sounds good to me.