[wasm] Remove indirection for accessing local types

Local type information was stored in the {WasmFullDecoder}, and a
pointer to that vector was handed to {WasmDecoder}. Since
{WasmFullDecoder} inherits from {WasmDecoder}, we can just move the
vector to the {WasmDecoder} class, and save an indirection and an
unnecessary nullptr check.

Drive-by: Rename {GetLocalType} to {local_type}, since it's a simple
accessor.
Drive-by 2: Move fields of {WasmDecoder} to the end of the class, as
mandated in the style guide.
Drive-by 3: Rename some locals in the 'let' decoding to make the meaning
more clear.

R=thibaudm@chromium.org

Bug: v8:10576
Change-Id: I6ab9831f0c1955e47562e84c5fbf15807439b024
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2264360Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68541}

src/wasm/baseline/liftoff-compiler.cc

@@ -460,7 +460,7 @@ class LiftoffCompiler {
     int num_locals = decoder->num_locals();
     __ set_num_locals(num_locals);
     for (int i = 0; i < num_locals; ++i) {
-      ValueType type = decoder->GetLocalType(i);
+      ValueType type = decoder->local_type(i);
       __ set_local_type(i, type);
     }
   }
@@ -520,7 +520,7 @@ class LiftoffCompiler {
     // because other types cannot be initialized to constants.
     for (uint32_t param_idx = num_params; param_idx < __ num_locals();
          ++param_idx) {
-      ValueType type = decoder->GetLocalType(param_idx);
+      ValueType type = decoder->local_type(param_idx);
       if (type != kWasmI32 && type != kWasmI64) return true;
     }
     return false;
@@ -586,7 +586,7 @@ class LiftoffCompiler {
     if (SpillLocalsInitially(decoder, num_params)) {
       for (uint32_t param_idx = num_params; param_idx < __ num_locals();
            ++param_idx) {
-        ValueType type = decoder->GetLocalType(param_idx);
+        ValueType type = decoder->local_type(param_idx);
         __ PushStack(type);
       }
       int spill_size = __ TopSpillOffset() - params_size;
@@ -594,7 +594,7 @@ class LiftoffCompiler {
     } else {
       for (uint32_t param_idx = num_params; param_idx < __ num_locals();
            ++param_idx) {
-        ValueType type = decoder->GetLocalType(param_idx);
+        ValueType type = decoder->local_type(param_idx);
         __ PushConstant(type, int32_t{0});
       }
     }

src/wasm/function-body-decoder.cc

@@ -22,23 +22,20 @@ namespace wasm {
 bool DecodeLocalDecls(const WasmFeatures& enabled, BodyLocalDecls* decls,
                       const byte* start, const byte* end) {
   WasmFeatures no_features = WasmFeatures::None();
-  WasmDecoder<Decoder::kValidate> decoder(nullptr, enabled, &no_features,
+  Zone* zone = decls->type_list.get_allocator().zone();
+  WasmDecoder<Decoder::kValidate> decoder(zone, nullptr, enabled, &no_features,
                                           nullptr, start, end, 0);
-  // The decoded functions need to be inserted into &decls->type_list,
-  // so we pass a pointer to it to local_types_ which will be updated
-  // in DecodeLocals.
-  decoder.local_types_ = &decls->type_list;
   uint32_t length;
-  if (decoder.DecodeLocals(
-          decoder.pc(), &length,
-          static_cast<uint32_t>(decoder.local_types_->size()))) {
-    DCHECK(decoder.ok());
-    decls->encoded_size = length;
-    return true;
-  } else {
+  if (!decoder.DecodeLocals(decoder.pc(), &length, 0)) {
     decls->encoded_size = 0;
     return false;
   }
+  DCHECK(decoder.ok());
+  decls->encoded_size = length;
+  // Copy the decoded locals types into {decls->type_list}.
+  DCHECK(decls->type_list.empty());
+  decls->type_list = std::move(decoder.local_types_);
+  return true;
 }
 
 BytecodeIterator::BytecodeIterator(const byte* start, const byte* end,
@@ -65,8 +62,11 @@ DecodeResult VerifyWasmCode(AccountingAllocator* allocator,
 
 unsigned OpcodeLength(const byte* pc, const byte* end) {
   WasmFeatures no_features = WasmFeatures::None();
-  WasmDecoder<Decoder::kNoValidate> decoder(nullptr, no_features, &no_features,
-                                            nullptr, pc, end, 0);
+  Zone* no_zone = nullptr;
+  WasmModule* no_module = nullptr;
+  FunctionSig* no_sig = nullptr;
+  WasmDecoder<Decoder::kNoValidate> decoder(no_zone, no_module, no_features,
+                                            &no_features, no_sig, pc, end, 0);
   return WasmDecoder<Decoder::kNoValidate>::OpcodeLength(&decoder, pc);
 }
 
@@ -74,8 +74,10 @@ std::pair<uint32_t, uint32_t> StackEffect(const WasmModule* module,
                                           const FunctionSig* sig,
                                           const byte* pc, const byte* end) {
   WasmFeatures unused_detected_features = WasmFeatures::None();
+  Zone* no_zone = nullptr;
   WasmDecoder<Decoder::kNoValidate> decoder(
-      module, WasmFeatures::All(), &unused_detected_features, sig, pc, end);
+      no_zone, module, WasmFeatures::All(), &unused_detected_features, sig, pc,
+      end);
   return decoder.StackEffect(pc);
 }
 
@@ -122,7 +124,7 @@ bool PrintRawWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
                       std::ostream& os, std::vector<int>* line_numbers) {
   Zone zone(allocator, ZONE_NAME);
   WasmFeatures unused_detected_features = WasmFeatures::None();
-  WasmDecoder<Decoder::kNoValidate> decoder(module, WasmFeatures::All(),
+  WasmDecoder<Decoder::kNoValidate> decoder(&zone, module, WasmFeatures::All(),
                                             &unused_detected_features, body.sig,
                                             body.start, body.end);
   int line_nr = 0;
@@ -302,8 +304,8 @@ bool PrintRawWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
 BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
                                            const byte* start, const byte* end) {
   WasmFeatures no_features = WasmFeatures::None();
-  WasmDecoder<Decoder::kValidate> decoder(nullptr, no_features, &no_features,
-                                          nullptr, start, end, 0);
+  WasmDecoder<Decoder::kValidate> decoder(zone, nullptr, no_features,
+                                          &no_features, nullptr, start, end, 0);
   return WasmDecoder<Decoder::kValidate>::AnalyzeLoopAssignment(
       &decoder, start, static_cast<uint32_t>(num_locals), zone);
 }

src/wasm/graph-builder-interface.cc

@@ -133,9 +133,9 @@ class WasmGraphBuildingInterface {
       ssa_env->locals[index] = builder_->Param(index + 1);
     }
     while (index < num_locals) {
-      ValueType type = decoder->GetLocalType(index);
+      ValueType type = decoder->local_type(index);
       TFNode* node = DefaultValue(type);
-      while (index < num_locals && decoder->GetLocalType(index) == type) {
+      while (index < num_locals && decoder->local_type(index) == type) {
         // Do a whole run of like-typed locals at a time.
         ssa_env->locals[index++] = node;
       }
@@ -900,7 +900,7 @@ class WasmGraphBuildingInterface {
           TFNode* b = ssa_env_->locals[i];
           if (a != b) {
             TFNode* inputs[] = {a, b, merge};
-            to->locals[i] = builder_->Phi(decoder->GetLocalType(i), 2, inputs);
+            to->locals[i] = builder_->Phi(decoder->local_type(i), 2, inputs);
           }
         }
         // Start a new merge from the instance cache.
@@ -918,7 +918,7 @@ class WasmGraphBuildingInterface {
         // Merge locals.
         for (int i = decoder->num_locals() - 1; i >= 0; i--) {
           to->locals[i] = builder_->CreateOrMergeIntoPhi(
-              decoder->GetLocalType(i).machine_representation(), merge,
+              decoder->local_type(i).machine_representation(), merge,
               to->locals[i], ssa_env_->locals[i]);
         }
         // Merge the instance caches.
@@ -941,16 +941,15 @@ class WasmGraphBuildingInterface {
     builder_->TerminateLoop(effect(), control());
     // The '+ 1' here is to be able to set the instance cache as assigned.
     BitVector* assigned = WasmDecoder<validate>::AnalyzeLoopAssignment(
-        decoder, decoder->pc(), decoder->total_locals() + 1, decoder->zone());
+        decoder, decoder->pc(), decoder->num_locals() + 1, decoder->zone());
     if (decoder->failed()) return;
     if (assigned != nullptr) {
       // Only introduce phis for variables assigned in this loop.
-      int instance_cache_index = decoder->total_locals();
+      int instance_cache_index = decoder->num_locals();
       for (int i = decoder->num_locals() - 1; i >= 0; i--) {
         if (!assigned->Contains(i)) continue;
         TFNode* inputs[] = {ssa_env_->locals[i], control()};
-        ssa_env_->locals[i] =
-            builder_->Phi(decoder->GetLocalType(i), 1, inputs);
+        ssa_env_->locals[i] = builder_->Phi(decoder->local_type(i), 1, inputs);
       }
       // Introduce phis for instance cache pointers if necessary.
       if (assigned->Contains(instance_cache_index)) {
@@ -966,7 +965,7 @@ class WasmGraphBuildingInterface {
     // Conservatively introduce phis for all local variables.
     for (int i = decoder->num_locals() - 1; i >= 0; i--) {
       TFNode* inputs[] = {ssa_env_->locals[i], control()};
-      ssa_env_->locals[i] = builder_->Phi(decoder->GetLocalType(i), 1, inputs);
+      ssa_env_->locals[i] = builder_->Phi(decoder->local_type(i), 1, inputs);
     }
 
     // Conservatively introduce phis for instance cache.

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -3500,9 +3500,9 @@ class WasmOpcodeLengthTest : public TestWithZone {
   void ExpectFailure(Bytes... bytes) {
     const byte code[] = {bytes..., 0, 0, 0, 0, 0, 0, 0, 0};
     WasmFeatures no_features = WasmFeatures::None();
-    WasmDecoder<Decoder::kValidate> decoder(nullptr, no_features, &no_features,
-                                            nullptr, code, code + sizeof(code),
-                                            0);
+    WasmDecoder<Decoder::kValidate> decoder(this->zone(), nullptr, no_features,
+                                            &no_features, nullptr, code,
+                                            code + sizeof(code), 0);
     WasmDecoder<Decoder::kValidate>::OpcodeLength(&decoder, code);
     EXPECT_EQ(decoder.failed(), true);
   }