[wasm] Exit loop once wasm code in JS_TO_WASM is found

src/wasm/wasm-module.cc

Index: src/wasm/wasm-module.cc
diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc
index 75a556971d521f7b515201b551598f77fa4c9364..a07469719e7bdb554bac3cba91538d946438a9b2 100644
--- a/src/wasm/wasm-module.cc
+++ b/src/wasm/wasm-module.cc
@@ -860,24 +860,29 @@ static WasmFunction* GetWasmFunctionForImportWrapper(Isolate* isolate,
   return nullptr;
 }
 
-static Handle<Code> UnwrapImportWrapper(Handle<Object> target) {
-  Handle<JSFunction> func = Handle<JSFunction>::cast(target);
+Code* UnwrapImportWrapper(Object* target) {

[titzer, 2017/02/28 15:07:31]

I think it'd be better if you left this as using handles.

[titzer, 2017/02/28 15:07:31]

any reason for dropping the static qualifier?

[Clemens Hammacher, 2017/02/28 15:14:52]

No, this is a mistake. This CL (https://codereview.chromium.org/2714373003) will
wrap it in an anonymous namespace, but without this, we should re-add the
static.
Hm, any reason for this? I know that the performance impact is minimal, but it
exists (handle(obj) also needs to gather the isolate first by
obj->GetIsolate()).
Handles generally make code less readable IMO.

[Clemens Hammacher, 2017/03/01 15:31:15]

Done.

+  JSFunction* func = JSFunction::cast(target);
   Handle<Code> export_wrapper_code = handle(func->code());
-  int found = 0;
   int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
   Handle<Code> code;
-  for (RelocIterator it(*export_wrapper_code, mask); !it.done(); it.next()) {
-    RelocInfo* rinfo = it.rinfo();
-    Address target_address = rinfo->target_address();
-    Code* target = Code::GetCodeFromTargetAddress(target_address);
-    if (target->kind() == Code::WASM_FUNCTION ||
-        target->kind() == Code::WASM_TO_JS_FUNCTION) {
-      ++found;
-      code = handle(target);
-    }
-  }
-  DCHECK_EQ(1, found);
-  return code;
+  for (RelocIterator it(*export_wrapper_code, mask);; it.next()) {
+    DCHECK(!it.done());
+    Code* target = Code::GetCodeFromTargetAddress(it.rinfo()->target_address());
+    if (target->kind() != Code::WASM_FUNCTION &&
+        target->kind() != Code::WASM_TO_JS_FUNCTION)

[ahaas, 2017/02/28 18:40:35]

What is the reason why code->builtin_index() != Builtins::kIllegal is not
included here?

[Clemens Hammacher, 2017/03/01 15:31:14]

The import wrapper we are unwrapping here belongs to another module, which must
be fully compiled at that stage, so kIllegal cannot occur here.
If it ever would, the DCHECK(!it.done()) would fail.

+      continue;
+// There should only be this one call to wasm code.
+#ifdef DEBUG
+    for (it.next(); !it.done(); it.next()) {
+      Code* code = Code::GetCodeFromTargetAddress(it.rinfo()->target_address());
+      DCHECK(code->kind() != Code::WASM_FUNCTION &&
+             code->kind() != Code::WASM_TO_JS_FUNCTION);
+    }
+#endif
+    return target;
+  }
+  UNREACHABLE();
+  return nullptr;
 }
 
 static Handle<Code> CompileImportWrapper(Isolate* isolate, int index,
@@ -891,7 +896,7 @@ static Handle<Code> CompileImportWrapper(Isolate* isolate, int index,
     if (sig->Equals(other_func->sig)) {
       // Signature matched. Unwrap the JS->WASM wrapper and return the raw
       // WASM function code.
-      return UnwrapImportWrapper(target);
+      return handle(UnwrapImportWrapper(*target), isolate);
     } else {
       return Handle<Code>::null();
     }
@@ -941,7 +946,7 @@ void wasm::UpdateDispatchTables(Isolate* isolate,
     UpdateDispatchTablesInternal(
         isolate, dispatch_tables, index,
         GetWasmFunctionForImportWrapper(isolate, function),
-        UnwrapImportWrapper(function));
+        handle(UnwrapImportWrapper(*function), isolate));
   }
 }
 
@@ -1583,7 +1588,7 @@ class InstantiationHelper {
             }
             int sig_index = table.map.FindOrInsert(function->sig);
             table_instance.signature_table->set(i, Smi::FromInt(sig_index));
-            table_instance.function_table->set(i, *UnwrapImportWrapper(val));
+            table_instance.function_table->set(i, UnwrapImportWrapper(*val));
           }
 
           num_imported_tables++;