Revert "[wasm] Various small cleanups/fixes"

This reverts commit f1c2a208.

Reason for revert: Breaks some tests on no-sse configuration, please see https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux/45243/overview

Original change's description:
> [wasm] Various small cleanups/fixes
>
> Changes:
> - Fix a bug in objects-printer where array elements were not treated as
>   tagged pointers.
> - Fix a few TODOs, mainly in the wasm interpreter.
> - Improve documentation, small refactorings.
>
> Change-Id: I1d70ad454b3a0693b9b784b17395434d81d01b61
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3383136
> Reviewed-by: Nikolaos Papaspyrou <nikolaos@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78656}

Change-Id: Ic698177259bb14b4c251a4212c79cc0d945b07f8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3398109
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Owners-Override: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78657}

src/compiler/wasm-compiler.cc

@@ -3065,7 +3065,7 @@ Node* WasmGraphBuilder::BuildImportCall(const wasm::FunctionSig* sig,
       func_index_intptr, gasm_->IntPtrConstant(kSystemPointerSize));
   Node* imported_targets =
       LOAD_INSTANCE_FIELD(ImportedFunctionTargets, MachineType::Pointer());
-  Node* target_node = gasm_->LoadImmutable(
+  Node* target_node = gasm_->LoadImmutableFromObject(
       MachineType::Pointer(), imported_targets, func_index_times_pointersize);
   args[0] = target_node;
 

src/diagnostics/objects-printer.cc

@@ -1805,16 +1805,9 @@ void WasmStruct::WasmStructPrint(std::ostream& os) {
       case wasm::kRef:
       case wasm::kOptRef:
       case wasm::kRtt:
-      case wasm::kRttWithDepth: {
-        Tagged_t raw = base::ReadUnalignedValue<Tagged_t>(field_address);
-#if V8_COMPRESS_POINTERS
-        Address obj = DecompressTaggedPointer(address(), raw);
-#else
-        Address obj = raw;
-#endif
-        os << Brief(Object(obj));
+      case wasm::kRttWithDepth:
+        os << Brief(base::ReadUnalignedValue<Object>(field_address));
         break;
-      }
       case wasm::kS128:
         os << "UNIMPLEMENTED";  // TODO(7748): Implement.
         break;
@@ -1851,24 +1844,17 @@ void WasmArray::WasmArrayPrint(std::ostream& os) {
                               true);
       break;
     case wasm::kI8:
-      PrintTypedArrayElements(os, reinterpret_cast<int8_t*>(data_ptr), len,
-                              true);
-      break;
     case wasm::kI16:
-      PrintTypedArrayElements(os, reinterpret_cast<int16_t*>(data_ptr), len,
-                              true);
-      break;
     case wasm::kS128:
     case wasm::kRef:
     case wasm::kOptRef:
     case wasm::kRtt:
     case wasm::kRttWithDepth:
+    case wasm::kBottom:
+    case wasm::kVoid:
       os << "\n   Printing elements of this type is unimplemented, sorry";
       // TODO(7748): Implement.
       break;
-    case wasm::kBottom:
-    case wasm::kVoid:
-      UNREACHABLE();
   }
   os << "\n";
 }

src/wasm/function-body-decoder-impl.h

@@ -1273,8 +1273,6 @@ class WasmDecoder : public Decoder {
   }
 
   bool Validate(const byte* pc, GlobalIndexImmediate<validate>& imm) {
-    // We compare with the current size of the globals vector. This is important
-    // if we are decoding a constant expression in the global section.
     if (!VALIDATE(imm.index < module_->globals.size())) {
       DecodeError(pc, "Invalid global index: %u", imm.index);
       return false;
@@ -2508,16 +2506,9 @@ class WasmFullDecoder : public WasmDecoder<validate, decoding_mode> {
             Append("T");
             break;
           case kControlIfElse:
-            Append("E");
-            break;
           case kControlTryCatch:
-            Append("C");
-            break;
           case kControlTryCatchAll:
-            Append("A");
-            break;
-          case kControlLet:
-            Append("D");
+          case kControlLet:  // TODO(7748): Implement
             break;
         }
         if (c.start_merge.arity) Append("%u-", c.start_merge.arity);

src/wasm/init-expr-interface.h

@@ -63,7 +63,7 @@ class InitExprInterface {
 #define UNREACHABLE_INTERFACE_FUNCTION(name, ...) \
   V8_INLINE void name(FullDecoder* decoder, ##__VA_ARGS__) { UNREACHABLE(); }
   INTERFACE_NON_CONSTANT_FUNCTIONS(UNREACHABLE_INTERFACE_FUNCTION)
-#undef UNREACHABLE_INTERFACE_FUNCTION
+#undef EMPTY_INTERFACE_FUNCTION
 
 #define DECLARE_INTERFACE_FUNCTION(name, ...) \
   void name(FullDecoder* decoder, ##__VA_ARGS__);

src/wasm/module-decoder.cc

@@ -859,8 +859,6 @@ class ModuleDecoderImpl : public Decoder {
   void DecodeGlobalSection() {
     uint32_t globals_count = consume_count("globals count", kV8MaxWasmGlobals);
     uint32_t imported_globals = static_cast<uint32_t>(module_->globals.size());
-    // It is important to not resize the globals vector from the beginning,
-    // because we use its current size when decoding the initializer.
     module_->globals.reserve(imported_globals + globals_count);
     for (uint32_t i = 0; ok() && i < globals_count; ++i) {
       TRACE("DecodeGlobal[%d] module+%d\n", i, static_cast<int>(pc_ - start_));
@@ -1930,8 +1928,10 @@ class ModuleDecoderImpl : public Decoder {
     ValueType* fields = zone->NewArray<ValueType>(field_count);
     bool* mutabilities = zone->NewArray<bool>(field_count);
     for (uint32_t i = 0; ok() && i < field_count; ++i) {
-      fields[i] = consume_storage_type();
-      mutabilities[i] = consume_mutability();
+      ValueType field = consume_storage_type();
+      fields[i] = field;
+      bool mutability = consume_mutability();
+      mutabilities[i] = mutability;
     }
     if (failed()) return nullptr;
     uint32_t* offsets = zone->NewArray<uint32_t>(field_count);
@@ -1939,10 +1939,10 @@ class ModuleDecoderImpl : public Decoder {
   }
 
   const ArrayType* consume_array(Zone* zone) {
-    ValueType element_type = consume_storage_type();
-    bool mutability = consume_mutability();
+    ValueType field = consume_storage_type();
     if (failed()) return nullptr;
-    return zone->New<ArrayType>(element_type, mutability);
+    bool mutability = consume_mutability();
+    return zone->New<ArrayType>(field, mutability);
   }
 
   // Consume the attribute field of an exception.

src/wasm/module-instantiate.cc

@@ -2077,12 +2077,16 @@ void InstanceBuilder::LoadTableSegments(Handle<WasmInstanceObject> instance) {
                    instance->tables().get(elem_segment.table_index)),
                isolate_),
         table_index, segment_index, dst, src, count);
-    // Set the active segments to being already dropped, since table.init on
-    // a dropped passive segment and an active segment have the same behavior.
+    // Set the active segments to being already dropped, since memory.init on
+    // a dropped passive segment and an active segment have the same
+    // behavior.
     instance->dropped_elem_segments()[segment_index] = 1;
     if (!success) {
       thrower_->RuntimeError("table initializer is out of bounds");
-      return;
+      // Break out instead of returning; we don't want to continue to
+      // initialize any further element segments, but still need to add
+      // dispatch tables below.
+      break;
     }
   }
 }

src/wasm/value-type.h

@@ -69,9 +69,13 @@ class HeapType {
     kArray,                   // shorthand: g
     kAny,                     // shorthand: a
     // This value is used to represent failures in the parsing of heap types and
-    // does not correspond to a wasm heap type. It has to be last in this list.
+    // does not correspond to a wasm heap type.
     kBottom
   };
+  // Internal use only; defined in the public section to make it easy to
+  // check that they are defined correctly:
+  static constexpr Representation kFirstSentinel = kFunc;
+  static constexpr Representation kLastSentinel = kAny;
 
   static constexpr HeapType from_code(uint8_t code) {
     switch (code) {
@@ -177,14 +181,8 @@ class HeapType {
 
  private:
   friend class ValueType;
-
-  constexpr bool is_valid() const { return representation_ <= kLastSentinel; }
-
-  static constexpr Representation kFirstSentinel =
-      static_cast<Representation>(kV8MaxWasmTypes);
-  static constexpr Representation kLastSentinel =
-      static_cast<Representation>(kBottom - 1);
   Representation representation_;
+  constexpr bool is_valid() const { return representation_ <= kLastSentinel; }
 };
 
 enum Nullability : bool { kNonNullable, kNullable };

src/wasm/wasm-opcodes.h

@@ -661,10 +661,6 @@ bool V8_EXPORT_PRIVATE IsJSCompatibleSignature(const FunctionSig* sig,
   V(I64AtomicCompareExchange16U, 0xfe4d, l_ill) \
   V(I64AtomicCompareExchange32U, 0xfe4e, l_ill)
 
-#define FOREACH_ATOMIC_0_OPERAND_OPCODE(V)                      \
-  /* AtomicFence does not target a particular linear memory. */ \
-  V(AtomicFence, 0xfe03, v_v)
-
 #define FOREACH_GC_OPCODE(V)                                         \
   V(StructNewWithRtt, 0xfb01, _)                                     \
   V(StructNewDefaultWithRtt, 0xfb02, _)                              \
@@ -717,6 +713,10 @@ bool V8_EXPORT_PRIVATE IsJSCompatibleSignature(const FunctionSig* sig,
   V(BrOnNonI31, 0xfb65, _)                                           \
   V(BrOnNonArray, 0xfb67, _) /* not standardized - V8 experimental */
 
+#define FOREACH_ATOMIC_0_OPERAND_OPCODE(V)                      \
+  /* AtomicFence does not target a particular linear memory. */ \
+  V(AtomicFence, 0xfe03, v_v)
+
 // All opcodes.
 #define FOREACH_OPCODE(V)            \
   FOREACH_CONTROL_OPCODE(V)          \

test/cctest/wasm/test-gc.cc

@@ -240,82 +240,67 @@ ValueType optref(uint32_t type_index) {
 WASM_COMPILED_EXEC_TEST(WasmBasicStruct) {
   WasmGCTester tester(execution_tier);
 
-  const byte kStructIndex =
+  const byte type_index =
       tester.DefineStruct({F(kWasmI32, true), F(kWasmI32, true)});
-  const byte kEmptyStructIndex = tester.DefineStruct({});
-  const byte kComplexStructIndex = tester.DefineStruct(
-      {F(kWasmI32, false), F(optref(kStructIndex), false), F(kWasmS128, false),
-       F(ValueType::Rtt(kStructIndex), false)});
-  auto sig_n_v = FixedSizeSignature<ValueType>::Returns(optref(kStructIndex));
-  auto sig_r_v = FixedSizeSignature<ValueType>::Returns(ref(kEmptyStructIndex));
-  auto sig_r_v_2 =
-      FixedSizeSignature<ValueType>::Returns(ref(kComplexStructIndex));
+  const byte empty_struct_index = tester.DefineStruct({});
+  ValueType kRefType = ref(type_index);
+  ValueType kEmptyStructType = ref(empty_struct_index);
+  ValueType kOptRefType = optref(type_index);
+  FunctionSig sig_q_v(1, 0, &kRefType);
+  FunctionSig sig_qe_v(1, 0, &kEmptyStructType);
 
   // Test struct.new and struct.get.
   const byte kGet1 = tester.DefineFunction(
       tester.sigs.i_v(), {},
       {WASM_STRUCT_GET(
-           kStructIndex, 0,
-           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(kStructIndex))),
+           type_index, 0,
+           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(type_index))),
        kExprEnd});
 
   // Test struct.new and struct.get.
   const byte kGet2 = tester.DefineFunction(
       tester.sigs.i_v(), {},
       {WASM_STRUCT_GET(
-           kStructIndex, 1,
-           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(kStructIndex))),
+           type_index, 1,
+           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(type_index))),
        kExprEnd});
 
   // Test struct.new, returning struct reference.
   const byte kGetStruct = tester.DefineFunction(
-      &sig_n_v, {},
-      {WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
-                                WASM_RTT_CANON(kStructIndex)),
+      &sig_q_v, {},
+      {WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
+                                WASM_RTT_CANON(type_index)),
        kExprEnd});
 
   const byte kGetStructNominal = tester.DefineFunction(
-      &sig_n_v, {},
-      {WASM_STRUCT_NEW_DEFAULT(kStructIndex), WASM_DROP,
-       WASM_STRUCT_NEW(kStructIndex, WASM_I32V(42), WASM_I32V(64)), kExprEnd});
+      &sig_q_v, {},
+      {WASM_STRUCT_NEW_DEFAULT(type_index), WASM_DROP,
+       WASM_STRUCT_NEW(type_index, WASM_I32V(42), WASM_I32V(64)), kExprEnd});
 
   // Test struct.new, returning reference to an empty struct.
   const byte kGetEmptyStruct = tester.DefineFunction(
-      &sig_r_v, {},
-      {WASM_STRUCT_NEW_WITH_RTT(kEmptyStructIndex,
-                                WASM_RTT_CANON(kEmptyStructIndex)),
+      &sig_qe_v, {},
+      {WASM_STRUCT_NEW_WITH_RTT(empty_struct_index,
+                                WASM_RTT_CANON(empty_struct_index)),
        kExprEnd});
 
   // Test struct.set, struct refs types in locals.
   const byte j_local_index = 0;
   const byte j_field_index = 0;
   const byte kSet = tester.DefineFunction(
-      tester.sigs.i_v(), {optref(kStructIndex)},
+      tester.sigs.i_v(), {kOptRefType},
       {WASM_LOCAL_SET(
            j_local_index,
-           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(kStructIndex))),
-       WASM_STRUCT_SET(kStructIndex, j_field_index,
-                       WASM_LOCAL_GET(j_local_index), WASM_I32V(-99)),
-       WASM_STRUCT_GET(kStructIndex, j_field_index,
+           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(type_index))),
+       WASM_STRUCT_SET(type_index, j_field_index, WASM_LOCAL_GET(j_local_index),
+                       WASM_I32V(-99)),
+       WASM_STRUCT_GET(type_index, j_field_index,
                        WASM_LOCAL_GET(j_local_index)),
        kExprEnd});
 
-  const byte kSimdConstant[16] = {0, 1, 2,  3,  4,  5,  6,  7,
-                                  8, 9, 10, 11, 12, 13, 14, 15};
-
-  const byte kComplexStructProducer = tester.DefineFunction(
-      &sig_r_v_2, {},
-      {WASM_STRUCT_NEW_WITH_RTT(kComplexStructIndex, WASM_I32V(42),
-                                WASM_STRUCT_NEW_DEFAULT_WITH_RTT(
-                                    kStructIndex, WASM_RTT_CANON(kStructIndex)),
-                                WASM_SIMD_CONSTANT(kSimdConstant),
-                                WASM_RTT_CANON(kStructIndex),
-                                WASM_RTT_CANON(kComplexStructIndex)),
-       kExprEnd});
-
   tester.CompileModule();
 
   tester.CheckResult(kGet1, 42);
@@ -328,9 +313,6 @@ WASM_COMPILED_EXEC_TEST(WasmBasicStruct) {
             .ToHandleChecked()
             ->IsWasmStruct());
   tester.CheckResult(kSet, -99);
-  CHECK(tester.GetResultObject(kComplexStructProducer)
-            .ToHandleChecked()
-            ->IsWasmStruct());
 }
 
 // Test struct.get, ref.as_non_null and ref-typed globals.

test/common/wasm/wasm-interpreter.cc

@@ -1440,7 +1440,7 @@ class WasmInterpreterInternals {
           val = WasmValue(isolate_->factory()->null_value(), p);
           break;
         }
-        case kRef:
+        case kRef:  // TODO(7748): Implement.
         case kRtt:
         case kRttWithDepth:
         case kVoid:
@@ -3164,11 +3164,29 @@ class WasmInterpreterInternals {
           break;
         }
         case kRef:
-        case kOptRef:
-        case kRtt:
-        case kRttWithDepth:
-          encoded_values->set(encoded_index++, *value.to_ref());
+        case kOptRef: {
+          switch (sig->GetParam(i).heap_representation()) {
+            case HeapType::kExtern:
+            case HeapType::kFunc:
+            case HeapType::kEq:
+            case HeapType::kData:
+            case HeapType::kArray:
+            case HeapType::kI31:
+            case HeapType::kAny: {
+              Handle<Object> ref = value.to_ref();
+              encoded_values->set(encoded_index++, *ref);
+              break;
+            }
+            case HeapType::kBottom:
+              UNREACHABLE();
+            default:
+              // TODO(7748): Implement these.
+              UNIMPLEMENTED();
+          }
           break;
+        }
+        case kRtt:  // TODO(7748): Implement.
+        case kRttWithDepth:
         case kI8:
         case kI16:
         case kVoid:
@@ -3252,13 +3270,28 @@ class WasmInterpreterInternals {
           break;
         }
         case kRef:
-        case kOptRef:
-        case kRtt:
-        case kRttWithDepth: {
-          Handle<Object> ref(encoded_values->get(encoded_index++), isolate_);
-          value = WasmValue(ref, sig->GetParam(i));
+        case kOptRef: {
+          switch (sig->GetParam(i).heap_representation()) {
+            case HeapType::kExtern:
+            case HeapType::kFunc:
+            case HeapType::kEq:
+            case HeapType::kData:
+            case HeapType::kArray:
+            case HeapType::kI31:
+            case HeapType::kAny: {
+              Handle<Object> ref(encoded_values->get(encoded_index++),
+                                 isolate_);
+              value = WasmValue(ref, sig->GetParam(i));
+              break;
+            }
+            default:
+              // TODO(7748): Implement these.
+              UNIMPLEMENTED();
+          }
           break;
         }
+        case kRtt:  // TODO(7748): Implement.
+        case kRttWithDepth:
         case kI8:
         case kI16:
         case kVoid:
@@ -3629,9 +3662,7 @@ class WasmInterpreterInternals {
             FOREACH_WASMVALUE_CTYPES(CASE_TYPE)
 #undef CASE_TYPE
             case kRef:
-            case kOptRef:
-            case kRtt:
-            case kRttWithDepth: {
+            case kOptRef: {
               // TODO(7748): Type checks or DCHECKs for ref types?
               HandleScope handle_scope(isolate_);  // Avoid leaking handles.
               Handle<FixedArray> global_buffer;    // The buffer of the global.
@@ -3643,6 +3674,8 @@ class WasmInterpreterInternals {
               global_buffer->set(global_index, *ref);
               break;
             }
+            case kRtt:  // TODO(7748): Implement.
+            case kRttWithDepth:
             case kI8:
             case kI16:
             case kVoid:
@@ -4040,18 +4073,25 @@ class WasmInterpreterInternals {
         case kVoid:
           PrintF("void");
           break;
-        case kOptRef:
-          if (val.to_ref()->IsNull()) {
-            PrintF("ref:null");
-            break;
-          }
-          V8_FALLTHROUGH;
         case kRef:
-          PrintF("ref:0x%" V8PRIxPTR, val.to_ref()->ptr());
+        case kOptRef: {
+          if (val.type().is_reference_to(HeapType::kExtern)) {
+            Handle<Object> ref = val.to_ref();
+            if (ref->IsNull()) {
+              PrintF("ref:null");
+            } else {
+              PrintF("ref:0x%" V8PRIxPTR, ref->ptr());
+            }
+          } else {
+            // TODO(7748): Implement this properly.
+            PrintF("ref/ref null");
+          }
           break;
+        }
         case kRtt:
         case kRttWithDepth:
-          PrintF("rtt:0x%" V8PRIxPTR, val.to_ref()->ptr());
+          // TODO(7748): Implement properly.
+          PrintF("rtt");
           break;
         case kI8:
         case kI16: