[wasm][refactor] Improvements to AnalyzeLoopAssignment

Changes:
- Move enhancement of locals_count by 1 inside AnalyzeLoopAssignment.
- Update documentation of AnalyzeLoopAssignment.
- Factor out invocation to OpcodeLength();
- Use uint32_t for locals count consistently in related testing
  functions.

Change-Id: I5bb5a324c4f4ed1aafc37849f3762d7a9630da51
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2549966Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71312}

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

@@ -1185,36 +1185,36 @@ class WasmDecoder : public Decoder {
     wasm::DecodeError<validate>(this, std::forward<Args>(args)...);
   }
 
+  // Returns a BitVector of length {locals_count + 1} representing the set of
+  // variables that are assigned in the loop starting at {pc}. The additional
+  // position at the end of the vector represents possible assignments to
+  // the instance cache.
   static BitVector* AnalyzeLoopAssignment(WasmDecoder* decoder, const byte* pc,
                                           uint32_t locals_count, Zone* zone) {
     if (pc >= decoder->end()) return nullptr;
     if (*pc != kExprLoop) return nullptr;
-
-    // The number of locals_count is augmented by 2 so that 'locals_count - 2'
-    // can be used to track mem_size, and 'locals_count - 1' to track mem_start.
-    BitVector* assigned = zone->New<BitVector>(locals_count, zone);
+    // The number of locals_count is augmented by 1 so that the 'locals_count'
+    // index can be used to track the instance cache.
+    BitVector* assigned = zone->New<BitVector>(locals_count + 1, zone);
     int depth = 0;
     // Iteratively process all AST nodes nested inside the loop.
     while (pc < decoder->end() && VALIDATE(decoder->ok())) {
       WasmOpcode opcode = static_cast<WasmOpcode>(*pc);
-      uint32_t length = 1;
       switch (opcode) {
         case kExprLoop:
         case kExprIf:
         case kExprBlock:
         case kExprTry:
-          length = OpcodeLength(decoder, pc);
+        case kExprLet:
           depth++;
           break;
-        case kExprLocalSet:  // fallthru
+        case kExprLocalSet:
         case kExprLocalTee: {
           LocalIndexImmediate<validate> imm(decoder, pc + 1);
-          if (assigned->length() > 0 &&
-              imm.index < static_cast<uint32_t>(assigned->length())) {
+          if (imm.index < locals_count) {
             // Unverified code might have an out-of-bounds index.
             assigned->Add(imm.index);
           }
-          length = 1 + imm.length;
           break;
         }
         case kExprMemoryGrow:
@@ -1222,20 +1222,19 @@ class WasmDecoder : public Decoder {
         case kExprCallIndirect:
         case kExprReturnCall:
         case kExprReturnCallIndirect:
-          // Add instance cache nodes to the assigned set.
-          // TODO(titzer): make this more clear.
-          assigned->Add(locals_count - 1);
-          length = OpcodeLength(decoder, pc);
+        case kExprCallRef:
+        case kExprReturnCallRef:
+          // Add instance cache to the assigned set.
+          assigned->Add(locals_count);
           break;
         case kExprEnd:
           depth--;
           break;
         default:
-          length = OpcodeLength(decoder, pc);
           break;
       }
       if (depth <= 0) break;
-      pc += length;
+      pc += OpcodeLength(decoder, pc);
     }
     return VALIDATE(decoder->ok()) ? assigned : nullptr;
   }

src/wasm/function-body-decoder.cc

@@ -301,13 +301,13 @@ bool PrintRawWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
   return decoder.ok();
 }
 
-BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
+BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, uint32_t num_locals,
                                            const byte* start, const byte* end) {
   WasmFeatures no_features = WasmFeatures::None();
   WasmDecoder<Decoder::kFullValidation> decoder(
       zone, nullptr, no_features, &no_features, nullptr, start, end, 0);
   return WasmDecoder<Decoder::kFullValidation>::AnalyzeLoopAssignment(
-      &decoder, start, static_cast<uint32_t>(num_locals), zone);
+      &decoder, start, num_locals, zone);
 }
 
 }  // namespace wasm

src/wasm/function-body-decoder.h

@@ -69,10 +69,8 @@ V8_EXPORT_PRIVATE bool DecodeLocalDecls(const WasmFeatures& enabled,
                                         BodyLocalDecls* decls,
                                         const byte* start, const byte* end);
 
-V8_EXPORT_PRIVATE BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone,
-                                                             size_t num_locals,
-                                                             const byte* start,
-                                                             const byte* end);
+V8_EXPORT_PRIVATE BitVector* AnalyzeLoopAssignmentForTesting(
+    Zone* zone, uint32_t num_locals, const byte* start, const byte* end);
 
 // Computes the length of the opcode at the given address.
 V8_EXPORT_PRIVATE unsigned OpcodeLength(const byte* pc, const byte* end);

src/wasm/graph-builder-interface.cc

@@ -1074,9 +1074,8 @@ class WasmGraphBuildingInterface {
     TFNode* effect_inputs[] = {effect(), control()};
     builder_->SetEffect(builder_->EffectPhi(1, effect_inputs));
     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->num_locals() + 1, decoder->zone());
+        decoder, decoder->pc(), decoder->num_locals(), decoder->zone());
     if (decoder->failed()) return;
     DCHECK_NOT_NULL(assigned);