[wasm] Several unrelated cleanups

src/wasm/wasm-module.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 <memory>
 
 #include "src/assembler-inl.h"
 #include "src/base/adapters.h"
 #include "src/base/atomic-utils.h"
 #include "src/code-stubs.h"
@@ skipping 810 matching lines @@
     os << "?";
   }
   return os;
 }
 
 WasmInstanceObject* wasm::GetOwningWasmInstance(Code* code) {
   DisallowHeapAllocation no_gc;
   DCHECK(code->kind() == Code::WASM_FUNCTION ||
          code->kind() == Code::WASM_INTERPRETER_ENTRY);
   FixedArray* deopt_data = code->deoptimization_data();
-  DCHECK_NOT_NULL(deopt_data);
   DCHECK_EQ(code->kind() == Code::WASM_INTERPRETER_ENTRY ? 1 : 2,
             deopt_data->length());
   Object* weak_link = deopt_data->get(0);
   DCHECK(weak_link->IsWeakCell());
   WeakCell* cell = WeakCell::cast(weak_link);
   if (cell->cleared()) return nullptr;
   return WasmInstanceObject::cast(cell->value());
 }
 
 int wasm::GetFunctionCodeOffset(Handle<WasmCompiledModule> compiled_module,
                                 int func_index) {
   return GetFunctionOffsetAndLength(compiled_module, func_index).first;
 }
 
 WasmModule::WasmModule(Zone* owned)
     : owned_zone(owned), pending_tasks(new base::Semaphore(0)) {}
 
-static WasmFunction* GetWasmFunctionForImportWrapper(Isolate* isolate,
-                                                     Handle<Object> target) {
+namespace {
+
+WasmFunction* GetWasmFunctionForImportWrapper(Isolate* isolate,
+                                              Handle<Object> target) {
   if (target->IsJSFunction()) {
     Handle<JSFunction> func = Handle<JSFunction>::cast(target);
     if (func->code()->kind() == Code::JS_TO_WASM_FUNCTION) {
       auto exported = Handle<WasmExportedFunction>::cast(func);
       Handle<WasmInstanceObject> other_instance(exported->instance(), isolate);
       int func_index = exported->function_index();
       return &other_instance->module()->functions[func_index];
     }
   }
   return nullptr;
@@ skipping 17 matching lines @@
       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,
-                                         FunctionSig* sig,
-                                         Handle<JSReceiver> target,
-                                         Handle<String> module_name,
-                                         MaybeHandle<String> import_name,
-                                         ModuleOrigin origin) {
+Handle<Code> CompileImportWrapper(Isolate* isolate, int index, FunctionSig* sig,
+                                  Handle<JSReceiver> target,
+                                  Handle<String> module_name,
+                                  MaybeHandle<String> import_name,
+                                  ModuleOrigin origin) {
   WasmFunction* other_func = GetWasmFunctionForImportWrapper(isolate, target);
   if (other_func) {
     if (sig->Equals(other_func->sig)) {
       // Signature matched. Unwrap the JS->WASM wrapper and return the raw
       // WASM function code.
       return handle(UnwrapImportWrapper(*target), isolate);
     } else {
       return Handle<Code>::null();
     }
   } else {
     // Signature mismatch. Compile a new wrapper for the new signature.
     return compiler::CompileWasmToJSWrapper(isolate, target, sig, index,
                                             module_name, import_name, origin);
   }
 }
 
-static void UpdateDispatchTablesInternal(Isolate* isolate,
-                                         Handle<FixedArray> dispatch_tables,
-                                         int index, WasmFunction* function,
-                                         Handle<Code> code) {
+void UpdateDispatchTablesInternal(Isolate* isolate, FixedArray* dispatch_tables,

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

Please don't unhandlify this.

[Clemens Hammacher, 2017/03/01 16:41:12]

Un-done ;)

+                                  int index, WasmFunction* function,
+                                  Code* code) {
+  DisallowHeapAllocation no_gc;
   DCHECK_EQ(0, dispatch_tables->length() % 4);
   for (int i = 0; i < dispatch_tables->length(); i += 4) {
     int table_index = Smi::cast(dispatch_tables->get(i + 1))->value();
-    Handle<FixedArray> function_table(
-        FixedArray::cast(dispatch_tables->get(i + 2)), isolate);
-    Handle<FixedArray> signature_table(
-        FixedArray::cast(dispatch_tables->get(i + 3)), isolate);
+    FixedArray* function_table = FixedArray::cast(dispatch_tables->get(i + 2));
+    FixedArray* signature_table = FixedArray::cast(dispatch_tables->get(i + 3));
     if (function) {
       // TODO(titzer): the signature might need to be copied to avoid
       // a dangling pointer in the signature map.
-      Handle<WasmInstanceObject> instance(
-          WasmInstanceObject::cast(dispatch_tables->get(i)), isolate);
-      int sig_index = static_cast<int>(
-          instance->module()->function_tables[table_index].map.FindOrInsert(
-              function->sig));
-      signature_table->set(index, Smi::FromInt(sig_index));
-      function_table->set(index, *code);
+      auto instance = WasmInstanceObject::cast(dispatch_tables->get(i));
+      auto func_table = instance->module()->function_tables[table_index];
+      uint32_t sig_index = func_table.map.FindOrInsert(function->sig);
+      signature_table->set(index, Smi::FromInt(static_cast<int>(sig_index)));
+      function_table->set(index, code);
     } else {
-      Code* code = nullptr;
       signature_table->set(index, Smi::FromInt(-1));
-      function_table->set(index, code);
+      function_table->set(index, Smi::kZero);
     }
   }
 }
 
+}  // namespace
+
 void wasm::UpdateDispatchTables(Isolate* isolate,
                                 Handle<FixedArray> dispatch_tables, int index,
                                 Handle<JSFunction> function) {
   if (function.is_null()) {
-    UpdateDispatchTablesInternal(isolate, dispatch_tables, index, nullptr,
-                                 Handle<Code>::null());
+    UpdateDispatchTablesInternal(isolate, *dispatch_tables, index, nullptr,
+                                 nullptr);
   } else {
     UpdateDispatchTablesInternal(
-        isolate, dispatch_tables, index,
+        isolate, *dispatch_tables, index,
         GetWasmFunctionForImportWrapper(isolate, function),
-        handle(UnwrapImportWrapper(*function), isolate));
+        UnwrapImportWrapper(*function));
   }
 }
 
 // A helper class to simplify instantiating a module from a compiled module.
 // It closes over the {Isolate}, the {ErrorThrower}, the {WasmCompiledModule},
 // etc.
 class InstantiationHelper {
  public:
   InstantiationHelper(Isolate* isolate, ErrorThrower* thrower,
                       Handle<WasmModuleObject> module_object,
@@ skipping 58 matching lines @@
         // the owner + original state used for cloning and patching
         // won't be mutated by possible finalizer runs.
         DCHECK(!owner.is_null());
         TRACE("Cloning from %d\n", original->instance_id());
         compiled_module_ = WasmCompiledModule::Clone(isolate_, original);
         // Avoid creating too many handles in the outer scope.
         HandleScope scope(isolate_);
 
         // Clone the code for WASM functions and exports.
         for (int i = 0; i < code_table->length(); ++i) {
-          Handle<Code> orig_code =
-              code_table->GetValueChecked<Code>(isolate_, i);
+          Handle<Code> orig_code(Code::cast(code_table->get(i)), isolate_);

[ahaas, 2017/02/28 18:11:39]

With this change you get rid of the CHECK in GetValueChecked. Is it guaranteed
that code_table will never return undefined?

[Clemens Hammacher, 2017/03/01 16:41:11]

Yes, this is guaranteed here. And Code::cast still does a SLOW_DCHECK for
IsCode().

           switch (orig_code->kind()) {
             case Code::WASM_TO_JS_FUNCTION:
               // Imports will be overwritten with newly compiled wrappers.
               break;
             case Code::JS_TO_WASM_FUNCTION:
             case Code::WASM_FUNCTION: {
               Handle<Code> code = factory->CopyCode(orig_code);
               code_table->set(i, *code);
               break;
             }
@@ skipping 141 matching lines @@
       code_specialization.RelocateMemoryReferences(old_mem_start, old_mem_size,
                                                    mem_start, mem_size);
       compiled_module_->set_memory(memory_);
     }
 
     //--------------------------------------------------------------------------
     // Set up the runtime support for the new instance.
     //--------------------------------------------------------------------------
     Handle<WeakCell> weak_link = factory->NewWeakCell(instance);
 
-    for (int i = num_imported_functions + FLAG_skip_compiling_wasm_funcs;
-         i < code_table->length(); ++i) {
-      Handle<Code> code = code_table->GetValueChecked<Code>(isolate_, i);
+    for (int i = num_imported_functions + FLAG_skip_compiling_wasm_funcs,
+             num_functions = code_table->length();
+         i < num_functions; ++i) {
+      Handle<Code> code = handle(Code::cast(code_table->get(i)), isolate_);
       if (code->kind() == Code::WASM_FUNCTION) {
         Handle<FixedArray> deopt_data = factory->NewFixedArray(2, TENURED);
         deopt_data->set(0, *weak_link);
-        deopt_data->set(1, Smi::FromInt(static_cast<int>(i)));
-        deopt_data->set_length(2);
+        deopt_data->set(1, Smi::FromInt(i));
         code->set_deoptimization_data(*deopt_data);
       }
     }
 
     //--------------------------------------------------------------------------
     // Set up the exports object for the new instance.
     //--------------------------------------------------------------------------
     ProcessExports(code_table, instance, compiled_module_);
 
     //--------------------------------------------------------------------------
@@ skipping 88 matching lines @@
     //--------------------------------------------------------------------------
     WasmSharedModuleData::SetBreakpointsOnNewInstance(
         compiled_module_->shared(), instance);
 
     //--------------------------------------------------------------------------
     // Run the start function if one was specified.
     //--------------------------------------------------------------------------
     if (module_->start_function_index >= 0) {
       HandleScope scope(isolate_);
       int start_index = module_->start_function_index;
-      Handle<Code> startup_code =
-          code_table->GetValueChecked<Code>(isolate_, start_index);
+      Handle<Code> startup_code(Code::cast(code_table->get(start_index)),
+                                isolate_);
       FunctionSig* sig = module_->functions[start_index].sig;
       Handle<Code> wrapper_code =
           js_to_wasm_cache_.CloneOrCompileJSToWasmWrapper(
               isolate_, module_, startup_code, start_index);
       Handle<WasmExportedFunction> startup_fct = WasmExportedFunction::New(
           isolate_, instance, MaybeHandle<String>(), start_index,
           static_cast<int>(sig->parameter_count()), wrapper_code);
       RecordStats(isolate_, *startup_code);
       // Call the JS function.
       Handle<Object> undefined = factory->undefined_value();
@@ skipping 677 matching lines @@
             Handle<WasmInstanceObject>::null(), index,
             Handle<FixedArray>::null(), Handle<FixedArray>::null());
       }
 
       // TODO(titzer): this does redundant work if there are multiple tables,
       // since initializations are not sorted by table index.
       for (auto table_init : module_->table_inits) {
         uint32_t base = EvalUint32InitExpr(table_init.offset);
         DCHECK(in_bounds(base, static_cast<uint32_t>(table_init.entries.size()),
                          table_instance.function_table->length()));
-        for (int i = 0; i < static_cast<int>(table_init.entries.size()); ++i) {
+        for (int i = 0, e = static_cast<int>(table_init.entries.size()); i < e;
+             ++i) {
           uint32_t func_index = table_init.entries[i];
           WasmFunction* function = &module_->functions[func_index];
           int table_index = static_cast<int>(i + base);
           int32_t sig_index = table.map.Find(function->sig);
           DCHECK_GE(sig_index, 0);
           table_instance.signature_table->set(table_index,
                                               Smi::FromInt(sig_index));
           table_instance.function_table->set(table_index,
                                              code_table->get(func_index));
 
@@ skipping 26 matching lines @@
               Handle<WasmExportedFunction> js_function =
                   WasmExportedFunction::New(
                       isolate_, instance, func_name, func_index,
                       static_cast<int>(function->sig->parameter_count()),
                       wrapper_code);
               js_wrappers_[func_index] = js_function;
             }
             table_instance.js_wrappers->set(table_index,
                                             *js_wrappers_[func_index]);
 
-            UpdateDispatchTablesInternal(isolate_, all_dispatch_tables,
-                                         table_index, function, wasm_code);
+            UpdateDispatchTablesInternal(isolate_, *all_dispatch_tables,
+                                         table_index, function, *wasm_code);
           }
         }
       }
 
       // TODO(titzer): we add the new dispatch table at the end to avoid
       // redundant work and also because the new instance is not yet fully
       // initialized.
       if (!table_instance.table_object.is_null()) {
         // Add the new dispatch table to the WebAssembly.Table object.
         all_dispatch_tables = WasmTableObject::AddDispatchTable(
@@ skipping 571 matching lines @@
 }
 
 MaybeHandle<WasmInstanceObject> wasm::SyncInstantiate(
     Isolate* isolate, ErrorThrower* thrower,
     Handle<WasmModuleObject> module_object, MaybeHandle<JSReceiver> imports,
     MaybeHandle<JSArrayBuffer> memory) {
   InstantiationHelper helper(isolate, thrower, module_object, imports, memory);
   return helper.Build();
 }
 
+namespace {
+
 void RejectPromise(Isolate* isolate, ErrorThrower* thrower,
                    Handle<JSPromise> promise) {
   v8::Local<v8::Promise::Resolver> resolver =
       v8::Utils::PromiseToLocal(promise).As<v8::Promise::Resolver>();
   Handle<Context> context(isolate->context(), isolate);
   resolver->Reject(v8::Utils::ToLocal(context),
                    v8::Utils::ToLocal(thrower->Reify()));
 }
 
 void ResolvePromise(Isolate* isolate, Handle<JSPromise> promise,
                     Handle<Object> result) {
   v8::Local<v8::Promise::Resolver> resolver =
       v8::Utils::PromiseToLocal(promise).As<v8::Promise::Resolver>();
   Handle<Context> context(isolate->context(), isolate);
   resolver->Resolve(v8::Utils::ToLocal(context), v8::Utils::ToLocal(result));
 }
 
+}  // namespace
+
 void wasm::AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
                         const ModuleWireBytes& bytes) {
   ErrorThrower thrower(isolate, nullptr);
   MaybeHandle<WasmModuleObject> module_object =
       SyncCompile(isolate, &thrower, bytes);
   if (thrower.error()) {
     RejectPromise(isolate, &thrower, promise);
     return;
   }
   ResolvePromise(isolate, promise, module_object.ToHandleChecked());
@@ skipping 42 matching lines @@
   Handle<String> module_property_name =
       isolate->factory()->InternalizeUtf8String("module");
   Handle<String> instance_property_name =
       isolate->factory()->InternalizeUtf8String("instance");
   JSObject::AddProperty(ret, module_property_name, module, NONE);
   JSObject::AddProperty(ret, instance_property_name,
                         instance_object.ToHandleChecked(), NONE);
 
   ResolvePromise(isolate, promise, ret);
 }