Revert "[wasm-gc] Implement call_ref on WasmJSFunction"

This reverts commit 6227c95e.

Reason for revert: Breaks Mac64 GC stress - https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8866365194967638384/+/steps/Check_-_d8/0/logs/call-ref/0

Original change's description:
> [wasm-gc] Implement call_ref on WasmJSFunction
>
> Changes:
> - Introduce turbofan builtin WasmAllocatePair.
> - Implement call_ref for WasmJSFunction in wasm-compiler.cc.
> - Remove WasmJSFunction trap.
> - Improve and extend call-ref.js test.
>
> Bug: v8:9495
> Change-Id: I8b4d1ab70cbbe9ae37887a6241d409eec638fd28
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2463226
> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70535}

TBR=clemensb@chromium.org,manoskouk@chromium.org

Change-Id: Ifad2cd8185df5e8d6766cefbcd3f28234a157dfb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:9495
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2475735Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70539}

src/builtins/base.tq

@@ -335,6 +335,7 @@ extern enum MessageTemplate {
   kWasmTrapNullDereference,
   kWasmTrapIllegalCast,
   kWasmTrapArrayOutOfBounds,
+  kWasmTrapWasmJSFunction,
   kWeakRefsRegisterTargetAndHoldingsMustNotBeSame,
   kWeakRefsRegisterTargetMustBeObject,
   kWeakRefsUnregisterTokenMustBeObject,

src/builtins/builtins-definitions.h

@@ -811,7 +811,6 @@ namespace internal {
   TFS(WasmAllocateArrayWithRtt, kMap, kLength, kElementSize)                   \
   TFC(WasmI32AtomicWait32, WasmI32AtomicWait32)                                \
   TFC(WasmI64AtomicWait32, WasmI64AtomicWait32)                                \
-  TFS(WasmAllocatePair, kValue1, kValue2)                                      \
                                                                                \
   /* WeakMap */                                                                \
   TFJ(WeakMapConstructor, kDontAdaptArgumentsSentinel)                         \

src/builtins/builtins-wasm-gen.cc

@@ -127,27 +127,5 @@ TF_BUILTIN(WasmAllocateArrayWithRtt, WasmBuiltinsAssembler) {
   Return(result);
 }
 
-TF_BUILTIN(WasmAllocatePair, WasmBuiltinsAssembler) {
-  TNode<WasmInstanceObject> instance = LoadInstanceFromFrame();
-  TNode<HeapObject> value1 = Parameter<HeapObject>(Descriptor::kValue1);
-  TNode<HeapObject> value2 = Parameter<HeapObject>(Descriptor::kValue2);
-
-  TNode<IntPtrT> roots = LoadObjectField<IntPtrT>(
-      instance, WasmInstanceObject::kIsolateRootOffset);
-  TNode<Map> map = CAST(Load(
-      MachineType::AnyTagged(), roots,
-      IntPtrConstant(IsolateData::root_slot_offset(RootIndex::kTuple2Map))));
-
-  TNode<IntPtrT> instance_size =
-      TimesTaggedSize(LoadMapInstanceSizeInWords(map));
-  TNode<Tuple2> result = UncheckedCast<Tuple2>(Allocate(instance_size));
-
-  StoreMap(result, map);
-  StoreObjectField(result, Tuple2::kValue1Offset, value1);
-  StoreObjectField(result, Tuple2::kValue2Offset, value2);
-
-  Return(result);
-}
-
 }  // namespace internal
 }  // namespace v8

src/builtins/wasm.tq

@@ -420,4 +420,8 @@ builtin ThrowWasmTrapIllegalCast(): JSAny {
 builtin ThrowWasmTrapArrayOutOfBounds(): JSAny {
   tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapArrayOutOfBounds));
 }
+
+builtin ThrowWasmTrapWasmJSFunction(): JSAny {
+  tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapWasmJSFunction));
+}
 }

src/common/globals.h

@@ -1612,6 +1612,7 @@ enum class LoadSensitivity {
   V(TrapRethrowNull)               \
   V(TrapNullDereference)           \
   V(TrapIllegalCast)               \
+  V(TrapWasmJSFunction)            \
   V(TrapArrayOutOfBounds)
 
 enum KeyedAccessLoadMode {

src/common/message-template.h

@@ -564,6 +564,7 @@ namespace internal {
   T(WasmTrapNullDereference, "dereferencing a null pointer")                   \
   T(WasmTrapIllegalCast, "illegal cast")                                       \
   T(WasmTrapArrayOutOfBounds, "array element access out of bounds")            \
+  T(WasmTrapWasmJSFunction, "cannot call WebAssembly.Function with call_ref")  \
   T(WasmExceptionError, "wasm exception")                                      \
   /* Asm.js validation related */                                              \
   T(AsmJsInvalid, "Invalid asm.js: %")                                         \

src/compiler/wasm-compiler.cc

@@ -3076,30 +3076,13 @@ Node* WasmGraphBuilder::BuildCallRef(uint32_t sig_index, Vector<Node*> args,
   }
 
   {
-    // Call to a WasmJSFunction. The call target is
-    // function_data->wasm_to_js_wrapper_code()->instruction_start().
-    // The instance_node is the pair
-    // (current WasmInstanceObject, function_data->callable()).
+    // Call to a WasmJSFunction.
+    // The call target is the wasm-to-js wrapper code.
     gasm_->Bind(&js_label);
-
-    Node* wrapper_code =
-        gasm_->Load(MachineType::TaggedPointer(), function_data,
-                    wasm::ObjectAccess::ToTagged(
-                        WasmJSFunctionData::kWasmToJsWrapperCodeOffset));
-    Node* call_target = gasm_->IntAdd(
-        wrapper_code,
-        gasm_->IntPtrConstant(wasm::ObjectAccess::ToTagged(Code::kHeaderSize)));
-
-    Node* callable = gasm_->Load(
-        MachineType::TaggedPointer(), function_data,
-        wasm::ObjectAccess::ToTagged(WasmJSFunctionData::kCallableOffset));
-    // TODO(manoskouk): Find an elegant way to avoid allocating this pair for
-    // every call.
-    Node* function_instance_node = CALL_BUILTIN(
-        WasmAllocatePair, instance_node_.get(), callable,
-        LOAD_INSTANCE_FIELD(NativeContext, MachineType::TaggedPointer()));
-
-    gasm_->Goto(&end_label, call_target, function_instance_node);
+    // TODO(9495): Implement when the interaction with the type reflection
+    // proposal is clear.
+    TrapIfTrue(wasm::kTrapWasmJSFunction, gasm_->Int32Constant(1), position);
+    gasm_->Goto(&end_label, args[0], RefNull() /* Dummy value */);
   }
 
   gasm_->Bind(&end_label);

src/wasm/wasm-code-manager.h

@@ -57,7 +57,6 @@ struct WasmModule;
   V(WasmFloat64ToNumber)                 \
   V(WasmTaggedToFloat64)                 \
   V(WasmAllocateJSArray)                 \
-  V(WasmAllocatePair)                    \
   V(WasmAtomicNotify)                    \
   V(WasmI32AtomicWait32)                 \
   V(WasmI32AtomicWait64)                 \

src/wasm/wasm-objects.cc

@@ -2021,17 +2021,7 @@ Handle<WasmJSFunction> WasmJSFunction::New(Isolate* isolate,
       SharedFunctionInfo shared = Handle<JSFunction>::cast(callable)->shared();
       expected_arity = shared.internal_formal_parameter_count();
       if (expected_arity != parameter_count) {
-        // TODO(manoskouk): call_ref on WasmJSFunction with arity mismatch is
-        // disabled due to a segfault in the wasm-to-js wrapper for mismatching
-        // arity.
-        //#ifdef V8_REVERSE_JSARGS
-        //        kind = CK::kJSFunctionArityMismatch;
-        //#else
-        // kind = shared.is_safe_to_skip_arguments_adaptor()
-        //           ? CK::kJSFunctionArityMismatchSkipAdaptor
-        //           : CK::kJSFunctionArityMismatch;
-        //#endif
-        kind = CK::kRuntimeTypeError;
+        kind = CK::kJSFunctionArityMismatch;
       }
     }
     // TODO(wasm): Think about caching and sharing the wasm-to-JS wrappers per

test/mjsunit/wasm/call-ref.js

@@ -22,8 +22,8 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
 
     var sig_index = builder.addType(kSig_i_ii);
 
-    var imported_js_api_function_index =
-      builder.addImport("imports", "js_api_mul", sig_index);
+    var imported_type_reflection_function_index =
+      builder.addImport("imports", "mul", sig_index);
 
     var imported_js_function_index =
       builder.addImport("imports", "js_add", sig_index);
@@ -31,6 +31,10 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
     var imported_wasm_function_index =
       builder.addImport("imports", "wasm_add", sig_index);
 
+    builder.addExport("unused", imported_wasm_function_index);
+    builder.addExport("reexported_js_function", imported_js_function_index);
+    builder.addExport("reexported_webassembly_function",
+                      imported_type_reflection_function_index);
 
     var locally_defined_function =
       builder.addFunction("sub", sig_index)
@@ -53,35 +57,34 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
                 kExprRefFunc, imported_js_function_index, kExprCallRef])
       .exportFunc();
 
-    builder.addFunction("test_wasm_import", kSig_i_v)
-      .addBody([kExprI32Const, 15, kExprI32Const, 42,
-                kExprRefFunc, imported_wasm_function_index, kExprCallRef])
-      .exportFunc();
+      builder.addFunction("test_wasm_import", kSig_i_v)
+        .addBody([kExprI32Const, 15, kExprI32Const, 42,
+                  kExprRefFunc, imported_wasm_function_index, kExprCallRef])
+        .exportFunc();
 
-    builder.addFunction("test_js_api_import", kSig_i_v)
+    /* Future use
+    builder.addFunction("test_webassembly_import", kSig_i_v)
       .addBody([kExprI32Const, 3, kExprI32Const, 7,
-                kExprRefFunc, imported_js_api_function_index,
+                kExprRefFunc, imported_type_reflection_function_index,
                 kExprCallRef])
       .exportFunc();
-
-    builder.addExport("reexported_js_function", imported_js_function_index);
-
-    // Just to make these functions eligible for call_ref.
-    builder.addDeclarativeElementSegment([imported_wasm_function_index,
-                                          imported_js_api_function_index]);
+    */
 
     return builder.instantiate({imports: {
       js_add: function(a, b) { return a + b; },
       wasm_add: exporting_instance.exports.addition,
-      js_api_mul: new WebAssembly.Function(
-          {parameters:['i32', 'i32'], results: ['i32']},
-          function(a, b) { return a * b; })
+      mul: new WebAssembly.Function({parameters:['i32', 'i32'],
+                                     results: ['i32']},
+                                    function(a, b) { return a * b; })
     }});
   })();
 
-  // Check that the modules exist.
-  assertTrue(!!exporting_instance);
-  assertTrue(!!instance);
+  // Check the modules exist.
+  assertFalse(instance === undefined);
+  assertFalse(instance === null);
+  assertFalse(instance === 0);
+  assertEquals("object", typeof instance.exports);
+  assertEquals("function", typeof instance.exports.main);
 
   print("--locally defined func--");
   assertEquals(13, instance.exports.test_local());
@@ -100,20 +103,9 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
   assertEquals(19, instance.exports.main(
     exporting_instance.exports.addition, 12, 7));
 
-  print("--imported WebAssembly.Function--")
-  assertEquals(21, instance.exports.test_js_api_import());
-  print("--not imported WebAssembly.Function--")
-  assertEquals(-5, instance.exports.main(
-    new WebAssembly.Function(
-      {parameters:['i32', 'i32'], results: ['i32']},
-      function(a, b) { return a - b; }),
-    10, 15));
-  // TODO(manoskouk): call_ref on WasmJSFunction with arity mismatch is disabled
-  // due to a segfault in the wasm-to-js wrapper for mismatching arity.
-  // print("--not imported WebAssembly.Function, arity mismatch--")
-  // assertEquals(100, instance.exports.main(
-  //   new WebAssembly.Function(
-  //     {parameters:['i32', 'i32'], results: ['i32']},
-  //     function(a) { return a * a; }),
-  //   10, 15));
+  // TODO(7748): Make these work once we know how we interact
+  //             with the 'type reflection' proposal.
+  //print("--imported WebAssembly.Function--")
+  //assertEquals(21, instance.exports.test_webassembly_import());
+  //print(" --not imported WebAssembly.Function--")
 })();