[wasm] Fix OpcodeLength() calculation.

R=bradnelson@chromium.org,binji@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1787733002

Cr-Commit-Position: refs/heads/master@{#34730}

src/wasm/ast-decoder.cc

@@ -334,12 +334,18 @@ class WasmDecoder : public Decoder {
         BranchTableOperand operand(this, pc);
         return 1 + operand.length;
       }
+      case kExprI32Const: {
+        ImmI32Operand operand(this, pc);
+        return 1 + operand.length;
+      }
+      case kExprI64Const: {
+        ImmI64Operand operand(this, pc);
+        return 1 + operand.length;
+      }
       case kExprI8Const:
         return 2;
-      case kExprI32Const:
       case kExprF32Const:
         return 5;
-      case kExprI64Const:
       case kExprF64Const:
         return 9;
 
@@ -653,7 +659,7 @@ class SR_WasmDecoder : public WasmDecoder {
             PushBlock(break_env);
             SsaEnv* cont_env = Steal(break_env);
             // The continue environment is the inner environment.
-            PrepareForLoop(cont_env);
+            PrepareForLoop(pc_, cont_env);
             SetEnv("loop:start", Split(cont_env));
             if (ssa_env_->go()) ssa_env_->state = SsaEnv::kReached;
             PushBlock(cont_env);
@@ -1452,28 +1458,18 @@ class SR_WasmDecoder : public WasmDecoder {
     return tnode;
   }
 
-  void BuildInfiniteLoop() {
-    if (ssa_env_->go()) {
-      PrepareForLoop(ssa_env_);
-      SsaEnv* cont_env = ssa_env_;
-      ssa_env_ = Split(ssa_env_);
-      ssa_env_->state = SsaEnv::kReached;
-      Goto(ssa_env_, cont_env);
-    }
-  }
-
-  void PrepareForLoop(SsaEnv* env) {
-    if (env->go()) {
-      env->state = SsaEnv::kMerged;
-      if (builder_) {
-        env->control = builder_->Loop(env->control);
-        env->effect = builder_->EffectPhi(1, &env->effect, env->control);
-        builder_->Terminate(env->effect, env->control);
-        for (int i = EnvironmentCount() - 1; i >= 0; i--) {
-          env->locals[i] = builder_->Phi(local_type_vec_[i], 1, &env->locals[i],
-                                         env->control);
-        }
-      }
+  void PrepareForLoop(const byte* pc, SsaEnv* env) {
+    if (!env->go()) return;
+    env->state = SsaEnv::kMerged;
+    if (!builder_) return;
+
+    env->control = builder_->Loop(env->control);
+    env->effect = builder_->EffectPhi(1, &env->effect, env->control);
+    builder_->Terminate(env->effect, env->control);
+    // Conservatively introduce phis for all local variables.
+    for (int i = EnvironmentCount() - 1; i >= 0; i--) {
+      env->locals[i] =
+          builder_->Phi(local_type_vec_[i], 1, &env->locals[i], env->control);
     }
   }
 
@@ -1592,6 +1588,9 @@ class SR_WasmDecoder : public WasmDecoder {
         length = OpcodeLength(pc);
       }
 
+      TRACE("loop-assign module+%-6d %s func+%d: 0x%02x %s (len=%d)\n",
+            baserel(pc), indentation(), startrel(pc), opcode,
+            WasmOpcodes::OpcodeName(opcode), length);
       pc += length;
       arity_stack.push_back(arity);
       while (arity_stack.back() == 0) {

src/wasm/ast-decoder.h

@@ -46,8 +46,7 @@ struct ImmI32Operand {
   int32_t value;
   int length;
   inline ImmI32Operand(Decoder* decoder, const byte* pc) {
-    value =
-        bit_cast<int32_t>(decoder->checked_read_i32v(pc, 1, &length, "immi32"));
+    value = decoder->checked_read_i32v(pc, 1, &length, "immi32");
   }
 };
 
@@ -55,8 +54,7 @@ struct ImmI64Operand {
   int64_t value;
   int length;
   inline ImmI64Operand(Decoder* decoder, const byte* pc) {
-    value =
-        bit_cast<int64_t>(decoder->checked_read_i64v(pc, 1, &length, "immi64"));
+    value = decoder->checked_read_i64v(pc, 1, &length, "immi64");
   }
 };
 

src/wasm/wasm-macro-gen.h

@@ -110,13 +110,11 @@ namespace wasm {
 inline void CheckI32v(int32_t value, int length) {
   DCHECK(length >= 1 && length <= 5);
   DCHECK(length == 5 || I32V_IN_RANGE(value, length));
-  DCHECK(length == 1 || !I32V_IN_RANGE(value, length - 1));
 }
 
 inline void CheckI64v(int64_t value, int length) {
   DCHECK(length >= 1 && length <= 10);
   DCHECK(length == 10 || I64V_IN_RANGE(value, length));
-  DCHECK(length == 1 || !I64V_IN_RANGE(value, length - 1));
 }
 
 // A helper for encoding local declarations prepended to the body of a

test/unittests/wasm/ast-decoder-unittest.cc

@@ -1725,6 +1725,12 @@ class WasmOpcodeLengthTest : public TestWithZone {
     EXPECT_EQ(expected, OpcodeLength(code, code + sizeof(code))); \
   }
 
+#define EXPECT_LENGTH_N(expected, ...)                            \
+  {                                                               \
+    static const byte code[] = {__VA_ARGS__};                     \
+    EXPECT_EQ(expected, OpcodeLength(code, code + sizeof(code))); \
+  }
+
 TEST_F(WasmOpcodeLengthTest, Statements) {
   EXPECT_LENGTH(1, kExprNop);
   EXPECT_LENGTH(2, kExprBlock);
@@ -1739,9 +1745,7 @@ TEST_F(WasmOpcodeLengthTest, Statements) {
 
 TEST_F(WasmOpcodeLengthTest, MiscExpressions) {
   EXPECT_LENGTH(2, kExprI8Const);
-  EXPECT_LENGTH(5, kExprI32Const);
   EXPECT_LENGTH(5, kExprF32Const);
-  EXPECT_LENGTH(9, kExprI64Const);
   EXPECT_LENGTH(9, kExprF64Const);
   EXPECT_LENGTH(2, kExprGetLocal);
   EXPECT_LENGTH(2, kExprSetLocal);
@@ -1758,21 +1762,33 @@ TEST_F(WasmOpcodeLengthTest, MiscExpressions) {
   EXPECT_LENGTH(2, kExprBrIf);
 }
 
+TEST_F(WasmOpcodeLengthTest, I32Const) {
+  EXPECT_LENGTH_N(2, kExprI32Const, U32V_1(1));
+  EXPECT_LENGTH_N(3, kExprI32Const, U32V_2(999));
+  EXPECT_LENGTH_N(4, kExprI32Const, U32V_3(9999));
+  EXPECT_LENGTH_N(5, kExprI32Const, U32V_4(999999));
+  EXPECT_LENGTH_N(6, kExprI32Const, U32V_5(99999999));
+}
 
-TEST_F(WasmOpcodeLengthTest, VariableLength) {
-  byte size2[] = {kExprLoadGlobal, 1};
-  byte size3[] = {kExprLoadGlobal, 1 | 0x80, 2};
-  byte size4[] = {kExprLoadGlobal, 1 | 0x80, 2 | 0x80, 3};
-  byte size5[] = {kExprLoadGlobal, 1 | 0x80, 2 | 0x80, 3 | 0x80, 4};
-  byte size6[] = {kExprLoadGlobal, 1 | 0x80, 2 | 0x80, 3 | 0x80, 4 | 0x80, 5};
-
-  EXPECT_EQ(2, OpcodeLength(size2, size2 + sizeof(size2)));
-  EXPECT_EQ(3, OpcodeLength(size3, size3 + sizeof(size3)));
-  EXPECT_EQ(4, OpcodeLength(size4, size4 + sizeof(size4)));
-  EXPECT_EQ(5, OpcodeLength(size5, size5 + sizeof(size5)));
-  EXPECT_EQ(6, OpcodeLength(size6, size6 + sizeof(size6)));
+TEST_F(WasmOpcodeLengthTest, I64Const) {
+  EXPECT_LENGTH_N(2, kExprI64Const, U32V_1(1));
+  EXPECT_LENGTH_N(3, kExprI64Const, U32V_2(99));
+  EXPECT_LENGTH_N(4, kExprI64Const, U32V_3(9999));
+  EXPECT_LENGTH_N(5, kExprI64Const, U32V_4(99999));
+  EXPECT_LENGTH_N(6, kExprI64Const, U32V_5(9999999));
+  EXPECT_LENGTH_N(7, WASM_I64V_6(777777));
+  EXPECT_LENGTH_N(8, WASM_I64V_7(7777777));
+  EXPECT_LENGTH_N(9, WASM_I64V_8(77777777));
+  EXPECT_LENGTH_N(10, WASM_I64V_9(777777777));
 }
 
+TEST_F(WasmOpcodeLengthTest, VariableLength) {
+  EXPECT_LENGTH_N(2, kExprLoadGlobal, U32V_1(1));
+  EXPECT_LENGTH_N(3, kExprLoadGlobal, U32V_2(33));
+  EXPECT_LENGTH_N(4, kExprLoadGlobal, U32V_3(44));
+  EXPECT_LENGTH_N(5, kExprLoadGlobal, U32V_4(66));
+  EXPECT_LENGTH_N(6, kExprLoadGlobal, U32V_5(77));
+}
 
 TEST_F(WasmOpcodeLengthTest, LoadsAndStores) {
   EXPECT_LENGTH(3, kExprI32LoadMem8S);

test/unittests/wasm/wasm-macro-gen-unittest.cc

@@ -27,12 +27,16 @@ TEST_F(WasmMacroGenTest, Constants) {
   EXPECT_SIZE(2, WASM_I8(254));
 
   EXPECT_SIZE(2, WASM_I32V_1(1));
+  EXPECT_SIZE(3, WASM_I32V_2(200));
   EXPECT_SIZE(4, WASM_I32V_3(10000));
   EXPECT_SIZE(5, WASM_I32V_4(-9828934));
+  EXPECT_SIZE(6, WASM_I32V_5(-1119828934));
 
   EXPECT_SIZE(2, WASM_I64V_1(1));
+  EXPECT_SIZE(3, WASM_I64V_2(300));
   EXPECT_SIZE(4, WASM_I64V_3(10000));
   EXPECT_SIZE(5, WASM_I64V_4(-9828934));
+  EXPECT_SIZE(6, WASM_I64V_5(-1119828934));
   EXPECT_SIZE(10, WASM_I64V_9(0x123456789abcdef0ULL));
 
   EXPECT_SIZE(5, WASM_F32(1.0f));