Revert "[wasm] Unify deoptimization data"

This reverts commit 236298ac.

Reason for revert: suspected cause of failures on GC stress bots:

https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/16341
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/16269

Original change's description:
> [wasm] Unify deoptimization data
> 
> Add methods to add deoptimization data and use them from all the places
> where we currently add them manually. Also add them to wasm-to-wasm
> wrappers compiled on table set, which was missing before, leading to
> the referenced bug.
> 
> Drive-by: Disable non-applicable MaybeHandle constructors to allow
> overloading functions with different Handle types.
> 
> R=​ahaas@chromium.org
> 
> Bug: chromium:779292
> Change-Id: Ib9132d9faeb1092c46e22dd8196d201ce5c0942f
> Reviewed-on: https://chromium-review.googlesource.com/774838
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49452}

TBR=ahaas@chromium.org,clemensh@chromium.org

Change-Id: I02fb49d2ece8e04ac5fb26f618bfe6fb2f133d06
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:779292
Reviewed-on: https://chromium-review.googlesource.com/777079Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49455}

src/compiler/wasm-compiler.cc

@@ -4277,8 +4277,6 @@ Handle<Code> CompileJSToWasmWrapper(Isolate* isolate, wasm::WasmModule* module,
   return code;
 }
 
-namespace {
-
 void ValidateImportWrapperReferencesImmovables(Handle<Code> wrapper) {
 #if !DEBUG
   return;
@@ -4317,35 +4315,6 @@ void ValidateImportWrapperReferencesImmovables(Handle<Code> wrapper) {
   }
 }
 
-}  // namespace
-
-void AttachWasmFunctionInfo(Isolate* isolate, Handle<Code> code,
-                            MaybeHandle<WeakCell> weak_instance,
-                            int func_index) {
-  DCHECK(weak_instance.is_null() ||
-         weak_instance.ToHandleChecked()->value()->IsWasmInstanceObject());
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
-  Handle<FixedArray> deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
-  if (!weak_instance.is_null()) {
-    // TODO(wasm): Introduce constants for the indexes in wasm deopt data.
-    deopt_data->set(0, *weak_instance.ToHandleChecked());
-  }
-  deopt_data->set(1, Smi::FromInt(func_index));
-  code->set_deoptimization_data(*deopt_data);
-}
-
-void AttachWasmFunctionInfo(Isolate* isolate, Handle<Code> code,
-                            MaybeHandle<WasmInstanceObject> instance,
-                            int func_index) {
-  MaybeHandle<WeakCell> weak_instance;
-  if (!instance.is_null()) {
-    weak_instance = isolate->factory()->NewWeakCell(instance.ToHandleChecked());
-  }
-  AttachWasmFunctionInfo(isolate, code, weak_instance, func_index);
-}
-
 Handle<Code> CompileWasmToJSWrapper(
     Isolate* isolate, Handle<JSReceiver> target, wasm::FunctionSig* sig,
     uint32_t index, wasm::ModuleOrigin origin,

src/compiler/wasm-compiler.h

@@ -183,21 +183,6 @@ Handle<Code> CompileWasmInterpreterEntry(Isolate* isolate, uint32_t func_index,
                                          wasm::FunctionSig* sig,
                                          Handle<WasmInstanceObject> instance);
 
-// Attach function information in the form of deoptimization data to the given
-// code object. This information will be used for generating stack traces,
-// calling imported functions in the interpreter, knowing which function to
-// compile in a lazy compile stub, and more. The deopt data will be a newly
-// allocated FixedArray of length 2, where the first element is a WeakCell
-// containing the WasmInstanceObject, and the second element is the function
-// index.
-// If calling this method repeatedly for the same instance, pass a WeakCell
-// directly in order to avoid creating many cells pointing to the same instance.
-void AttachWasmFunctionInfo(Isolate*, Handle<Code>,
-                            MaybeHandle<WeakCell> weak_instance,
-                            int func_index);
-void AttachWasmFunctionInfo(Isolate*, Handle<Code>,
-                            MaybeHandle<WasmInstanceObject>, int func_index);
-
 enum CWasmEntryParameters {
   kCodeObject,
   kArgumentsBuffer,

src/wasm/module-compiler.cc

@@ -218,8 +218,6 @@ class ModuleCompiler {
   Handle<Code> centry_stub_;
 };
 
-namespace {
-
 class JSToWasmWrapperCache {
  public:
   void SetContextAddress(Address context_address) {
@@ -393,7 +391,7 @@ class InstanceBuilder {
 };
 
 // TODO(titzer): move to wasm-objects.cc
-void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
+static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   DisallowHeapAllocation no_gc;
   JSObject** p = reinterpret_cast<JSObject**>(data.GetParameter());
   WasmInstanceObject* owner = reinterpret_cast<WasmInstanceObject*>(*p);
@@ -467,8 +465,6 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   TRACE("}\n");
 }
 
-}  // namespace
-
 bool SyncValidate(Isolate* isolate, const ModuleWireBytes& bytes) {
   if (bytes.start() == nullptr || bytes.length() == 0) return false;
   ModuleResult result = SyncDecodeWasmModule(isolate, bytes.start(),
@@ -753,7 +749,15 @@ void LazyCompilationOrchestrator::CompileFunction(
   CHECK(!thrower.error());
   Handle<Code> code = maybe_code.ToHandleChecked();
 
-  compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+
+  Handle<FixedArray> deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
+  Handle<WeakCell> weak_instance = isolate->factory()->NewWeakCell(instance);
+  // TODO(wasm): Introduce constants for the indexes in wasm deopt data.
+  deopt_data->set(0, *weak_instance);
+  deopt_data->set(1, Smi::FromInt(func_index));
+  code->set_deoptimization_data(*deopt_data);
 
   DCHECK_EQ(Builtins::kWasmCompileLazy,
             Code::cast(compiled_module->code_table()->get(func_index))
@@ -1259,11 +1263,21 @@ Handle<Code> EnsureExportedLazyDeoptData(Isolate* isolate,
   //   #1: func_index
   // might be extended later for table exports (see
   // EnsureTableExportLazyDeoptData).
-  DCHECK_EQ(0, code->deoptimization_data()->length() % 2);
-  if (code->deoptimization_data()->length() == 0) {
+  Handle<FixedArray> deopt_data(code->deoptimization_data());
+  DCHECK_EQ(0, deopt_data->length() % 2);
+  if (deopt_data->length() == 0) {
     code = isolate->factory()->CopyCode(code);
     code_table->set(func_index, *code);
-    compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
+    deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
+    // TODO(6792): No longer needed once WebAssembly code is off heap.
+    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+    code->set_deoptimization_data(*deopt_data);
+    if (!instance.is_null()) {
+      Handle<WeakCell> weak_instance =
+          isolate->factory()->NewWeakCell(instance);
+      deopt_data->set(0, *weak_instance);
+    }
+    deopt_data->set(1, Smi::FromInt(func_index));
   }
   DCHECK_IMPLIES(!instance.is_null(),
                  WeakCell::cast(code->deoptimization_data()->get(0))->value() ==
@@ -1347,11 +1361,18 @@ Handle<Code> MakeWasmToWasmWrapper(
   Handle<Code> wrapper_code = compiler::CompileWasmToWasmWrapper(
       isolate, wasm_code, *sig, imported_function->function_index(),
       new_wasm_context_address);
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   // Set the deoptimization data for the WasmToWasm wrapper. This is
   // needed by the interpreter to find the imported instance for
   // a cross-instance call.
-  compiler::AttachWasmFunctionInfo(isolate, wrapper_code, imported_instance,
-                                   imported_function->function_index());
+  Factory* factory = isolate->factory();
+  Handle<WeakCell> weak_link = factory->NewWeakCell(imported_instance);
+  Handle<FixedArray> deopt_data = factory->NewFixedArray(2, TENURED);
+  deopt_data->set(0, *weak_link);
+  auto function_index = Smi::FromInt(imported_function->function_index());
+  deopt_data->set(1, function_index);
+  wrapper_code->set_deoptimization_data(*deopt_data);
   return wrapper_code;
 }
 
@@ -1663,8 +1684,15 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
             // unique copy to attach updated deoptimization data.
             if (orig_code->deoptimization_data()->length() > 0) {
               Handle<Code> code = factory->CopyCode(orig_code);
-              compiler::AttachWasmFunctionInfo(isolate_, code,
-                                               Handle<WasmInstanceObject>(), i);
+              Handle<FixedArray> deopt_data =
+                  factory->NewFixedArray(2, TENURED);
+              deopt_data->set(1, Smi::FromInt(i));
+              // TODO(6792): No longer needed once WebAssembly code is off heap.
+              {
+                CodeSpaceMemoryModificationScope modification_scope(
+                    isolate_->heap());
+                code->set_deoptimization_data(*deopt_data);
+              }
               code_table->set(i, *code);
             }
             break;
@@ -1842,7 +1870,12 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
        i < num_functions; ++i) {
     Handle<Code> code = handle(Code::cast(code_table->get(i)), isolate_);
     if (code->kind() == Code::WASM_FUNCTION) {
-      compiler::AttachWasmFunctionInfo(isolate_, code, weak_link, i);
+      Handle<FixedArray> deopt_data = factory->NewFixedArray(2, TENURED);
+      deopt_data->set(0, *weak_link);
+      deopt_data->set(1, Smi::FromInt(i));
+      // TODO(6792): No longer needed once WebAssembly code is off heap.
+      CodeSpaceMemoryModificationScope modification_scope(isolate_->heap());
+      code->set_deoptimization_data(*deopt_data);
       continue;
     }
     DCHECK_EQ(Builtins::kWasmCompileLazy, code->builtin_index());

src/wasm/wasm-objects.cc

@@ -298,14 +298,11 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
     value = function;
     // TODO(titzer): Make JSToWasm wrappers just call the WASM to WASM wrapper,
     // and then we can just reuse the WASM to WASM wrapper.
-    Handle<WasmInstanceObject> instance(exported_function->instance(), isolate);
-    int func_index = exported_function->function_index();
-    Address new_context_address =
-        reinterpret_cast<Address>(instance->wasm_context()->get());
+    Address new_context_address = reinterpret_cast<Address>(
+        exported_function->instance()->wasm_context()->get());
     code = compiler::CompileWasmToWasmWrapper(
         isolate, exported_function->GetWasmCode(), wasm_function->sig,
-        func_index, new_context_address);
-    compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
+        exported_function->function_index(), new_context_address);
   }
 
   UpdateDispatchTables(isolate, dispatch_tables, index, wasm_function, code);

test/mjsunit/wasm/interpreter.js

@@ -449,21 +449,3 @@ function checkStack(stack, expected_lines) {
   const instance = builder2.instantiate({imp: {func: exp}});
   assertEquals(23, instance.exports.main());
 })();
-
-(function testTableCall() {
-  print(arguments.callee.name);
-  const builder1 = new WasmModuleBuilder();
-  builder1.addFunction('func', kSig_v_v).addBody([]).exportFunc();
-  const instance1 = builder1.instantiate();
-  const table = new WebAssembly.Table({element: 'anyfunc', initial: 2});
-
-  const builder2 = new WasmModuleBuilder()
-  builder2.addImportedTable('m', 'table');
-  const sig = builder2.addType(kSig_v_v);
-  builder2.addFunction('call_func', kSig_v_v)
-      .addBody([kExprI32Const, 0, kExprCallIndirect, sig, kTableZero])
-      .exportFunc();
-  const instance2 = builder2.instantiate({m: {table: table}});
-  table.set(0, instance1.exports.func);
-  instance2.exports.call_func();
-})();