[wasm] Allow import function to be any kind of callables.

src/compiler/wasm-compiler.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/wasm-compiler.h"
 
 #include <memory>
 
 #include "src/isolate-inl.h"
 
@@ skipping 2166 matching lines @@
 
   // Allocate the box for the {value}.
   vbox = BuildAllocateHeapNumberWithValue(value, if_box);
 
   Node* control = graph()->NewNode(common->Merge(2), if_smi, if_box);
   value = graph()->NewNode(common->Phi(MachineRepresentation::kTagged, 2), vsmi,
                            vbox, control);
   return value;
 }
 
-Node* WasmGraphBuilder::ToJS(Node* node, Node* context, wasm::LocalType type) {
+Node* WasmGraphBuilder::ToJS(Node* node, wasm::LocalType type) {
   switch (type) {
     case wasm::kAstI32:
       return BuildChangeInt32ToTagged(node);
     case wasm::kAstI64:
       // TODO(titzer): i64->JS has no good solution right now. Using lower 32
       // bits.
       if (jsgraph()->machine()->Is64()) {
         // On 32 bit platforms we do not have to do the truncation because the
         // node we get in as a parameter only contains the low word anyways.
         node = graph()->NewNode(jsgraph()->machine()->TruncateInt64ToInt32(),
@@ skipping 318 matching lines @@
     desc = wasm::ModuleEnv::GetI32WasmCallDescriptor(jsgraph()->zone(), desc);
   }
   Node* call = graph()->NewNode(jsgraph()->common()->Call(desc), count, args);
   Node* retval = call;
   if (jsgraph()->machine()->Is32() && sig->return_count() > 0 &&
       sig->GetReturn(0) == wasm::kAstI64) {
     // The return values comes as two values, we pick the low word.
     retval = graph()->NewNode(jsgraph()->common()->Projection(0), retval,
                               graph()->start());
   }
-  Node* jsval =
-      ToJS(retval, context,
-           sig->return_count() == 0 ? wasm::kAstStmt : sig->GetReturn());
+  Node* jsval = ToJS(
+      retval, sig->return_count() == 0 ? wasm::kAstStmt : sig->GetReturn());
   Node* ret =
       graph()->NewNode(jsgraph()->common()->Return(), jsval, call, start);
 
   MergeControlToEnd(jsgraph(), ret);
 }
 
-void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSFunction> function,
+void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
                                             wasm::FunctionSig* sig) {
-  int js_count = function->shared()->internal_formal_parameter_count();
+  DCHECK(target->IsCallable());
+
   int wasm_count = static_cast<int>(sig->parameter_count());
   int param_count;
   if (jsgraph()->machine()->Is64()) {
     param_count = wasm_count;
   } else {
     param_count = Int64Lowering::GetParameterCountAfterLowering(sig);
   }
 
   // Build the start and the parameter nodes.
   Isolate* isolate = jsgraph()->isolate();
   CallDescriptor* desc;
   Node* start = Start(param_count + 3);
   *effect_ = start;
   *control_ = start;
-  // JS context is the last parameter.
-  Node* context = HeapConstant(Handle<Context>(function->context(), isolate));
   Node** args = Buffer(wasm_count + 7);
 
   bool arg_count_before_args = false;
   bool add_new_target_undefined = false;
-
+  bool call_direct = false;
   int pos = 0;
-  if (js_count == wasm_count) {
-    // exact arity match, just call the function directly.
-    desc = Linkage::GetJSCallDescriptor(graph()->zone(), false, wasm_count + 1,
-                                        CallDescriptor::kNoFlags);
-    arg_count_before_args = false;
-    add_new_target_undefined = true;
-  } else {
+  Handle<JSFunction> function;

[Mircea Trofin, 2016/08/09 14:54:07]

For readability/maintainability, could you perhaps pull in the if statement all
the variables that depend on function being a valid handle, and assign them
there? We reduce the risk of someone attempting to use function in incorrect
cases; we also encapsulate the decisions about receiver's value and the heap
constant that is to be used if (call_direct) in that one place. 

In fact, you can do away with having the call_direct bool altogether.

[ahaas, 2016/08/09 18:00:09]

I refactored the code now and think that it's more readable than before.

+  if (target->IsJSFunction()) {
+    function = Handle<JSFunction>::cast(target);
+    if (function->shared()->internal_formal_parameter_count() == wasm_count) {
+      desc = Linkage::GetJSCallDescriptor(
+          graph()->zone(), false, wasm_count + 1, CallDescriptor::kNoFlags);
+      arg_count_before_args = false;
+      add_new_target_undefined = true;
+      call_direct = true;
+    }
+  }
+  if (!call_direct) {
     // Use the Call builtin.
     Callable callable = CodeFactory::Call(isolate);
     args[pos++] = jsgraph()->HeapConstant(callable.code());
     desc = Linkage::GetStubCallDescriptor(isolate, graph()->zone(),
                                           callable.descriptor(), wasm_count + 1,
                                           CallDescriptor::kNoFlags);
     arg_count_before_args = true;
   }
 
-  args[pos++] = jsgraph()->Constant(function);  // JS function.
+  args[pos++] = jsgraph()->Constant(target);  // target callable.
   if (arg_count_before_args) {
     args[pos++] = jsgraph()->Int32Constant(wasm_count);  // argument count
   }
   // Create the receiver constant (either undefined or the global proxy).
   Handle<Object> receiver(isolate->heap()->undefined_value(), isolate);
-  if (is_sloppy(function->shared()->language_mode())) {
-    receiver = Handle<Object>(function->context()->global_proxy(), isolate);
+  if (call_direct && is_sloppy(function->shared()->language_mode())) {
+    receiver = handle(function->context()->global_proxy(), isolate);
   }
   args[pos++] = jsgraph()->Constant(receiver);
-
   // Convert WASM numbers to JS values.
   int param_index = 0;
   for (int i = 0; i < wasm_count; ++i) {
     Node* param =
         graph()->NewNode(jsgraph()->common()->Parameter(param_index++), start);
-    args[pos++] = ToJS(param, context, sig->GetParam(i));
+    args[pos++] = ToJS(param, sig->GetParam(i));
     if (jsgraph()->machine()->Is32() && sig->GetParam(i) == wasm::kAstI64) {
       // On 32 bit platforms we have to skip the high word of int64 parameters.
       param_index++;
     }
   }
 
   if (add_new_target_undefined) {
     args[pos++] = jsgraph()->UndefinedConstant();  // new target
   }
 
   if (!arg_count_before_args) {
     args[pos++] = jsgraph()->Int32Constant(wasm_count);  // argument count
   }
-  args[pos++] = context;
+  if (call_direct) {
+    // We do a direct call to JS, we need the context of the function.
+    args[pos++] = HeapConstant(handle(function->context()));
+  } else {
+    args[pos++] = HeapConstant(isolate->native_context());
+  }
+
   args[pos++] = *effect_;
   args[pos++] = *control_;
 
   Node* call = graph()->NewNode(jsgraph()->common()->Call(desc), pos, args);
 
   // Convert the return value back.
   Node* ret;
   Node* val =
-      FromJS(call, context,
+      FromJS(call, HeapConstant(isolate->native_context()),
              sig->return_count() == 0 ? wasm::kAstStmt : sig->GetReturn());
   if (jsgraph()->machine()->Is32() && sig->return_count() > 0 &&
       sig->GetReturn() == wasm::kAstI64) {
     ret = graph()->NewNode(jsgraph()->common()->Return(), val,
                            graph()->NewNode(jsgraph()->machine()->Word32Sar(),
                                             val, jsgraph()->Int32Constant(31)),
                            call, start);
   } else {
     ret = graph()->NewNode(jsgraph()->common()->Return(), val, call, start);
   }
@@ skipping 419 matching lines @@
 
   if (isolate->logger()->is_logging_code_events() || isolate->is_profiling()) {
     RecordFunctionCompilation(
         CodeEventListener::FUNCTION_TAG, isolate, code, "js-to-wasm", index,
         wasm::WasmName("export"),
         module->module->GetName(func->name_offset, func->name_length));
   }
   return code;
 }
 
-Handle<Code> CompileWasmToJSWrapper(Isolate* isolate,
-                                    Handle<JSFunction> function,
+Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
                                     wasm::FunctionSig* sig, uint32_t index,
                                     Handle<String> import_module,
                                     MaybeHandle<String> import_function) {
   //----------------------------------------------------------------------------
   // Create the Graph
   //----------------------------------------------------------------------------
   Zone zone(isolate->allocator());
   Graph graph(&zone);
   CommonOperatorBuilder common(&zone);
   MachineOperatorBuilder machine(&zone);
   JSGraph jsgraph(isolate, &graph, &common, nullptr, nullptr, &machine);
 
   Node* control = nullptr;
   Node* effect = nullptr;
 
   WasmGraphBuilder builder(&zone, &jsgraph, sig);
   builder.set_control_ptr(&control);
   builder.set_effect_ptr(&effect);
-  builder.BuildWasmToJSWrapper(function, sig);
+  builder.BuildWasmToJSWrapper(target, sig);
 
   Handle<Code> code = Handle<Code>::null();
   {
     if (FLAG_trace_turbo_graph) {  // Simple textual RPO.
       OFStream os(stdout);
       os << "-- Graph after change lowering -- " << std::endl;
       os << AsRPO(graph);
     }
 
     // Schedule and compile to machine code.
@@ skipping 238 matching lines @@
                             function_->code_start_offset),
            compile_ms);
   }
 
   return code;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8