[wasm] Encode immediates to Load and Store as varint.

R=binji@chromium.org
BUG=

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

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

src/wasm/asm-wasm-builder.cc

@@ -799,9 +799,9 @@ class AsmWasmBuilderImpl : public AstVisitor {
     } else {
       UNREACHABLE();
     }
-    current_function_builder_->EmitWithU8(
-        WasmOpcodes::LoadStoreOpcodeOf(mtype, is_set_op_),
-        WasmOpcodes::LoadStoreAccessOf(false));
+    // TODO(titzer): use special asm-compatibility opcodes?
+    current_function_builder_->EmitWithU8U8(
+        WasmOpcodes::LoadStoreOpcodeOf(mtype, is_set_op_), 0, 0);
     is_set_op_ = false;
     if (size == 1) {
       // Allow more general expression in byte arrays than the spec

src/wasm/ast-decoder.h

@@ -165,19 +165,17 @@ struct BranchTableOperand {
 };
 
 struct MemoryAccessOperand {
-  bool aligned;
+  uint32_t alignment;
   uint32_t offset;
   int length;
   inline MemoryAccessOperand(Decoder* decoder, const byte* pc) {
-    byte bitfield = decoder->checked_read_u8(pc, 1, "memory access byte");
-    aligned = MemoryAccess::AlignmentField::decode(bitfield);
-    if (MemoryAccess::OffsetField::decode(bitfield)) {
-      offset = decoder->checked_read_u32v(pc, 2, &length, "memory offset");
-      length++;
-    } else {
-      offset = 0;
-      length = 1;
-    }
+    int alignment_length;
+    alignment =
+        decoder->checked_read_u32v(pc, 1, &alignment_length, "alignment");
+    int offset_length;
+    offset = decoder->checked_read_u32v(pc, 1 + alignment_length,
+                                        &offset_length, "offset");
+    length = alignment_length + offset_length;
   }
 };
 

src/wasm/encoder.cc

@@ -131,6 +131,13 @@ void WasmFunctionBuilder::EmitWithU8(WasmOpcode opcode, const byte immediate) {
   body_.push_back(immediate);
 }
 
+void WasmFunctionBuilder::EmitWithU8U8(WasmOpcode opcode, const byte imm1,
+                                       const byte imm2) {
+  body_.push_back(static_cast<byte>(opcode));
+  body_.push_back(imm1);
+  body_.push_back(imm2);
+}
+
 void WasmFunctionBuilder::EmitWithVarInt(WasmOpcode opcode,
                                          uint32_t immediate) {
   body_.push_back(static_cast<byte>(opcode));

src/wasm/encoder.h

@@ -55,6 +55,7 @@ class WasmFunctionBuilder : public ZoneObject {
                 const uint32_t* local_indices, uint32_t indices_size);
   void Emit(WasmOpcode opcode);
   void EmitWithU8(WasmOpcode opcode, const byte immediate);
+  void EmitWithU8U8(WasmOpcode opcode, const byte imm1, const byte imm2);
   void EmitWithVarInt(WasmOpcode opcode, uint32_t immediate);
   uint32_t EmitEditableVarIntImmediate();
   void EditVarIntImmediate(uint32_t offset, const uint32_t immediate);

src/wasm/wasm-macro-gen.h

@@ -19,6 +19,9 @@
 #define FUNC_INDEX(v) U32V_1(v)
 #define NAME_OFFSET(v) U32_LE(v)
 
+#define ZERO_ALIGNMENT 0
+#define ZERO_OFFSET 0
+
 #define BR_TARGET(v) U32_LE(v)
 
 #define MASK_7 ((1 << 7) - 1)
@@ -325,21 +328,19 @@ class LocalDeclEncoder {
 #define WASM_LOAD_MEM(type, index)                                      \
   static_cast<byte>(                                                    \
       v8::internal::wasm::WasmOpcodes::LoadStoreOpcodeOf(type, false)), \
-      v8::internal::wasm::WasmOpcodes::LoadStoreAccessOf(false), index
+      ZERO_ALIGNMENT, ZERO_OFFSET, index
 #define WASM_STORE_MEM(type, index, val)                               \
   static_cast<byte>(                                                   \
       v8::internal::wasm::WasmOpcodes::LoadStoreOpcodeOf(type, true)), \
-      v8::internal::wasm::WasmOpcodes::LoadStoreAccessOf(false), index, val
+      ZERO_ALIGNMENT, ZERO_OFFSET, index, val
 #define WASM_LOAD_MEM_OFFSET(type, offset, index)                       \
   static_cast<byte>(                                                    \
       v8::internal::wasm::WasmOpcodes::LoadStoreOpcodeOf(type, false)), \
-      v8::internal::wasm::WasmOpcodes::LoadStoreAccessOf(true),         \
-      static_cast<byte>(offset), index
+      ZERO_ALIGNMENT, U32V_1(offset), index
 #define WASM_STORE_MEM_OFFSET(type, offset, index, val)                \
   static_cast<byte>(                                                   \
       v8::internal::wasm::WasmOpcodes::LoadStoreOpcodeOf(type, true)), \
-      v8::internal::wasm::WasmOpcodes::LoadStoreAccessOf(true),        \
-      static_cast<byte>(offset), index, val
+      ZERO_ALIGNMENT, U32V_1(offset), index, val
 #define WASM_CALL_FUNCTION(index, ...) \
   kExprCallFunction, static_cast<byte>(index), __VA_ARGS__
 #define WASM_CALL_IMPORT(index, ...) \

src/wasm/wasm-opcodes.h

@@ -46,25 +46,6 @@ const LocalType kAstF64 = MachineRepresentation::kFloat64;
 // We use kTagged here because kNone is already used by kAstStmt.
 const LocalType kAstEnd = MachineRepresentation::kTagged;
 
-// Functionality related to encoding memory accesses.
-struct MemoryAccess {
-  // Atomicity annotations for access to the memory and globals.
-  enum Atomicity {
-    kNone = 0,        // non-atomic
-    kSequential = 1,  // sequential consistency
-    kAcquire = 2,     // acquire semantics
-    kRelease = 3      // release semantics
-  };
-
-  // Alignment annotations for memory accesses.
-  enum Alignment { kAligned = 0, kUnaligned = 1 };
-
-  // Bitfields for the various annotations for memory accesses.
-  typedef BitField<Alignment, 7, 1> AlignmentField;
-  typedef BitField<Atomicity, 5, 2> AtomicityField;
-  typedef BitField<bool, 4, 1> OffsetField;
-};
-
 typedef Signature<LocalType> FunctionSig;
 std::ostream& operator<<(std::ostream& os, const FunctionSig& function);
 
@@ -448,10 +429,6 @@ class WasmOpcodes {
     }
   }
 
-  static byte LoadStoreAccessOf(bool with_offset) {
-    return MemoryAccess::OffsetField::encode(with_offset);
-  }
-
   static char ShortNameOf(LocalType type) {
     switch (type) {
       case kAstI32:

test/cctest/wasm/test-run-wasm-module.cc

@@ -148,8 +148,7 @@ TEST(Run_WasmModule_CheckMemoryIsZero) {
               WASM_LOAD_MEM(MachineType::Int32(), WASM_GET_LOCAL(localIndex)),
               WASM_BRV(2, WASM_I8(-1)), WASM_INC_LOCAL_BY(localIndex, 4))),
       WASM_I8(11))};
-  uint32_t local_indices[] = {7, 18, 24, 27};
-  f->EmitCode(code, sizeof(code), local_indices, sizeof(local_indices) / 4);
+  f->EmitCode(code, sizeof(code), nullptr, 0);
   WasmModuleWriter* writer = builder->Build(&zone);
   TestModule(writer->WriteTo(&zone), 11);
 }
@@ -174,8 +173,7 @@ TEST(Run_WasmModule_CallMain_recursive) {
                                                 WASM_INC_LOCAL(localIndex)),
                               WASM_BRV(1, WASM_CALL_FUNCTION0(0))),
                    WASM_BRV(0, WASM_I8(55))))};
-  uint32_t local_indices[] = {3, 11, 21, 24};
-  f->EmitCode(code, sizeof(code), local_indices, sizeof(local_indices) / 4);
+  f->EmitCode(code, sizeof(code), nullptr, 0);
   WasmModuleWriter* writer = builder->Build(&zone);
   TestModule(writer->WriteTo(&zone), 55);
 }

test/cctest/wasm/test-run-wasm.cc

@@ -1777,9 +1777,9 @@ TEST(Run_Wasm_CheckMachIntsZero) {
   WasmRunner<uint32_t> r(&module, MachineType::Int32());
 
   BUILD(r, kExprBlock, 2, kExprLoop, 1, kExprIf, kExprGetLocal, 0, kExprBr, 0,
-        kExprIfElse, kExprI32LoadMem, 0, kExprGetLocal, 0, kExprBr, 2,
-        kExprI8Const, 255, kExprSetLocal, 0, kExprI32Sub, kExprGetLocal, 0,
-        kExprI8Const, 4, kExprI8Const, 0);
+        kExprIfElse, kExprI32LoadMem, ZERO_ALIGNMENT, ZERO_OFFSET,
+        kExprGetLocal, 0, kExprBr, 2, kExprI8Const, 255, kExprSetLocal, 0,
+        kExprI32Sub, kExprGetLocal, 0, kExprI8Const, 4, kExprI8Const, 0);
 
   module.BlankMemory();
   CHECK_EQ(0, r.Call((kNumElems - 1) * 4));
@@ -2558,7 +2558,7 @@ static void Run_WasmMixedCall_N(int start) {
     std::vector<byte> code;
     ADD_CODE(code,
              static_cast<byte>(WasmOpcodes::LoadStoreOpcodeOf(result, true)),
-             WasmOpcodes::LoadStoreAccessOf(false));
+             ZERO_ALIGNMENT, ZERO_OFFSET);
     ADD_CODE(code, WASM_ZERO);
     ADD_CODE(code, kExprCallFunction, static_cast<byte>(index));
 
@@ -2782,8 +2782,12 @@ TEST(Run_Wasm_LoadStoreI64_sx) {
     byte* memory = module.AddMemoryElems<byte>(16);
     WasmRunner<int64_t> r(&module);
 
-    byte code[] = {kExprI64StoreMem, 0, kExprI8Const, 8,
-                   loads[m],         0, kExprI8Const, 0};
+    byte code[] = {kExprI64StoreMem, ZERO_ALIGNMENT,
+                   ZERO_OFFSET,          // --
+                   kExprI8Const,     8,  // --
+                   loads[m],         ZERO_ALIGNMENT,
+                   ZERO_OFFSET,           // --
+                   kExprI8Const,     0};  // --
 
     r.Build(code, code + arraysize(code));
 

test/mjsunit/wasm/module-memory.js

@@ -21,7 +21,7 @@ function genModule(memory) {
             kExprGetLocal,0,
             kExprBr, 0,
               kExprIfElse,
-                kExprI32LoadMem,0,kExprGetLocal,0,
+                kExprI32LoadMem,0,0,kExprGetLocal,0,
                 kExprBr,2, kExprI8Const, 255,
                 kExprSetLocal,0,
                   kExprI32Sub,kExprGetLocal,0,kExprI8Const,4,
@@ -122,7 +122,7 @@ function testOOBThrows() {
   builder.addMemory(1, 1, true);
   builder.addFunction("geti", [kAstI32, kAstI32, kAstI32])
     .addBody([
-      kExprI32StoreMem, 0, kExprGetLocal, 0, kExprI32LoadMem, 0, kExprGetLocal, 1
+      kExprI32StoreMem, 0, 0, kExprGetLocal, 0, kExprI32LoadMem, 0, 0, kExprGetLocal, 1
     ])
     .exportFunc();
 

test/mjsunit/wasm/start-function.js

@@ -79,7 +79,7 @@ assertFails([kAstI32, kAstI32, kAstF32, kAstF64], [kExprGetLocal, 0]);
   builder.addMemory(12, 12, true);
 
   var func = builder.addFunction("", [kAstStmt])
-    .addBody([kExprI32StoreMem, 0, kExprI8Const, 0, kExprI8Const, 77]);
+    .addBody([kExprI32StoreMem, 0, 0, kExprI8Const, 0, kExprI8Const, 77]);
 
   builder.addStart(func.index);
 

test/mjsunit/wasm/test-wasm-module-builder.js

@@ -103,7 +103,7 @@ var debug = false;
     var module = new WasmModuleBuilder();
     module.addMemory(1, 1, false);
     module.addFunction("load", [kAstI32, kAstI32])
-        .addBody([kExprI32LoadMem, 0, kExprGetLocal, 0])
+        .addBody([kExprI32LoadMem, 0, 0, kExprGetLocal, 0])
         .exportAs("load");
     module.addDataSegment(0, [9, 9, 9, 9], true);
 

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

@@ -893,49 +893,29 @@ TEST_F(AstDecoderTest, GrowMemory) {
 
 TEST_F(AstDecoderTest, LoadMemOffset) {
   for (int offset = 0; offset < 128; offset += 7) {
-    byte code[] = {kExprI32LoadMem, WasmOpcodes::LoadStoreAccessOf(true),
-                   static_cast<byte>(offset), kExprI8Const, 0};
+    byte code[] = {kExprI32LoadMem, ZERO_ALIGNMENT, static_cast<byte>(offset),
+                   kExprI8Const, 0};
     EXPECT_VERIFIES(sigs.i_i(), code);
   }
 }
 
 TEST_F(AstDecoderTest, StoreMemOffset) {
   for (int offset = 0; offset < 128; offset += 7) {
-    byte code[] = {kExprI32StoreMem,
-                   WasmOpcodes::LoadStoreAccessOf(true),
-                   static_cast<byte>(offset),
-                   kExprI8Const,
-                   0,
-                   kExprI8Const,
-                   0};
+    byte code[] = {
+        kExprI32StoreMem, 0, static_cast<byte>(offset), kExprI8Const, 0,
+        kExprI8Const,     0};
     EXPECT_VERIFIES(sigs.i_i(), code);
   }
 }
 
 TEST_F(AstDecoderTest, LoadMemOffset_varint) {
-  byte code1[] = {kExprI32LoadMem, WasmOpcodes::LoadStoreAccessOf(true), 0,
-                  kExprI8Const, 0};
-  byte code2[] = {kExprI32LoadMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
-                  0x80,
-                  1,
-                  kExprI8Const,
-                  0};
-  byte code3[] = {kExprI32LoadMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
-                  0x81,
-                  0x82,
-                  5,
-                  kExprI8Const,
-                  0};
-  byte code4[] = {kExprI32LoadMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
-                  0x83,
-                  0x84,
-                  0x85,
-                  7,
-                  kExprI8Const,
+  byte code1[] = {kExprI32LoadMem, ZERO_ALIGNMENT, ZERO_OFFSET, kExprI8Const,
                   0};
+  byte code2[] = {kExprI32LoadMem, ZERO_ALIGNMENT, 0x80, 1, kExprI8Const, 0};
+  byte code3[] = {
+      kExprI32LoadMem, ZERO_ALIGNMENT, 0x81, 0x82, 5, kExprI8Const, 0};
+  byte code4[] = {
+      kExprI32LoadMem, ZERO_ALIGNMENT, 0x83, 0x84, 0x85, 7, kExprI8Const, 0};
 
   EXPECT_VERIFIES(sigs.i_i(), code1);
   EXPECT_VERIFIES(sigs.i_i(), code2);
@@ -944,15 +924,10 @@ TEST_F(AstDecoderTest, LoadMemOffset_varint) {
 }
 
 TEST_F(AstDecoderTest, StoreMemOffset_varint) {
-  byte code1[] = {kExprI32StoreMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
-                  0,
-                  kExprI8Const,
-                  0,
-                  kExprI8Const,
-                  0};
+  byte code1[] = {
+      kExprI32StoreMem, ZERO_ALIGNMENT, 0, kExprI8Const, 0, kExprI8Const, 0};
   byte code2[] = {kExprI32StoreMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
+                  ZERO_ALIGNMENT,
                   0x80,
                   1,
                   kExprI8Const,
@@ -960,7 +935,7 @@ TEST_F(AstDecoderTest, StoreMemOffset_varint) {
                   kExprI8Const,
                   0};
   byte code3[] = {kExprI32StoreMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
+                  ZERO_ALIGNMENT,
                   0x81,
                   0x82,
                   5,
@@ -969,7 +944,7 @@ TEST_F(AstDecoderTest, StoreMemOffset_varint) {
                   kExprI8Const,
                   0};
   byte code4[] = {kExprI32StoreMem,
-                  WasmOpcodes::LoadStoreAccessOf(true),
+                  ZERO_ALIGNMENT,
                   0x83,
                   0x84,
                   0x85,
@@ -992,7 +967,7 @@ TEST_F(AstDecoderTest, AllLoadMemCombinations) {
       MachineType mem_type = machineTypes[j];
       byte code[] = {
           static_cast<byte>(WasmOpcodes::LoadStoreOpcodeOf(mem_type, false)),
-          WasmOpcodes::LoadStoreAccessOf(false), kExprI8Const, 0};
+          ZERO_ALIGNMENT, ZERO_OFFSET, kExprI8Const, 0};
       FunctionSig sig(1, 0, &local_type);
       if (local_type == WasmOpcodes::LocalTypeFor(mem_type)) {
         EXPECT_VERIFIES(&sig, code);
@@ -1010,7 +985,8 @@ TEST_F(AstDecoderTest, AllStoreMemCombinations) {
       MachineType mem_type = machineTypes[j];
       byte code[] = {
           static_cast<byte>(WasmOpcodes::LoadStoreOpcodeOf(mem_type, true)),
-          WasmOpcodes::LoadStoreAccessOf(false),
+          ZERO_ALIGNMENT,
+          ZERO_OFFSET,
           kExprI8Const,
           0,
           kExprGetLocal,
@@ -1799,30 +1775,30 @@ TEST_F(WasmOpcodeLengthTest, VariableLength) {
 
 
 TEST_F(WasmOpcodeLengthTest, LoadsAndStores) {
-  EXPECT_LENGTH(2, kExprI32LoadMem8S);
-  EXPECT_LENGTH(2, kExprI32LoadMem8U);
-  EXPECT_LENGTH(2, kExprI32LoadMem16S);
-  EXPECT_LENGTH(2, kExprI32LoadMem16U);
-  EXPECT_LENGTH(2, kExprI32LoadMem);
-  EXPECT_LENGTH(2, kExprI64LoadMem8S);
-  EXPECT_LENGTH(2, kExprI64LoadMem8U);
-  EXPECT_LENGTH(2, kExprI64LoadMem16S);
-  EXPECT_LENGTH(2, kExprI64LoadMem16U);
-  EXPECT_LENGTH(2, kExprI64LoadMem32S);
-  EXPECT_LENGTH(2, kExprI64LoadMem32U);
-  EXPECT_LENGTH(2, kExprI64LoadMem);
-  EXPECT_LENGTH(2, kExprF32LoadMem);
-  EXPECT_LENGTH(2, kExprF64LoadMem);
-
-  EXPECT_LENGTH(2, kExprI32StoreMem8);
-  EXPECT_LENGTH(2, kExprI32StoreMem16);
-  EXPECT_LENGTH(2, kExprI32StoreMem);
-  EXPECT_LENGTH(2, kExprI64StoreMem8);
-  EXPECT_LENGTH(2, kExprI64StoreMem16);
-  EXPECT_LENGTH(2, kExprI64StoreMem32);
-  EXPECT_LENGTH(2, kExprI64StoreMem);
-  EXPECT_LENGTH(2, kExprF32StoreMem);
-  EXPECT_LENGTH(2, kExprF64StoreMem);
+  EXPECT_LENGTH(3, kExprI32LoadMem8S);
+  EXPECT_LENGTH(3, kExprI32LoadMem8U);
+  EXPECT_LENGTH(3, kExprI32LoadMem16S);
+  EXPECT_LENGTH(3, kExprI32LoadMem16U);
+  EXPECT_LENGTH(3, kExprI32LoadMem);
+  EXPECT_LENGTH(3, kExprI64LoadMem8S);
+  EXPECT_LENGTH(3, kExprI64LoadMem8U);
+  EXPECT_LENGTH(3, kExprI64LoadMem16S);
+  EXPECT_LENGTH(3, kExprI64LoadMem16U);
+  EXPECT_LENGTH(3, kExprI64LoadMem32S);
+  EXPECT_LENGTH(3, kExprI64LoadMem32U);
+  EXPECT_LENGTH(3, kExprI64LoadMem);
+  EXPECT_LENGTH(3, kExprF32LoadMem);
+  EXPECT_LENGTH(3, kExprF64LoadMem);
+
+  EXPECT_LENGTH(3, kExprI32StoreMem8);
+  EXPECT_LENGTH(3, kExprI32StoreMem16);
+  EXPECT_LENGTH(3, kExprI32StoreMem);
+  EXPECT_LENGTH(3, kExprI64StoreMem8);
+  EXPECT_LENGTH(3, kExprI64StoreMem16);
+  EXPECT_LENGTH(3, kExprI64StoreMem32);
+  EXPECT_LENGTH(3, kExprI64StoreMem);
+  EXPECT_LENGTH(3, kExprF32StoreMem);
+  EXPECT_LENGTH(3, kExprF64StoreMem);
 }
 
 

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

@@ -52,7 +52,7 @@ TEST_F(WasmMacroGenTest, Statements) {
 
   EXPECT_SIZE(4, WASM_STORE_GLOBAL(0, WASM_ZERO));
 
-  EXPECT_SIZE(6, WASM_STORE_MEM(MachineType::Int32(), WASM_ZERO, WASM_ZERO));
+  EXPECT_SIZE(7, WASM_STORE_MEM(MachineType::Int32(), WASM_ZERO, WASM_ZERO));
 
   EXPECT_SIZE(4, WASM_IF(WASM_ZERO, WASM_NOP));
 
@@ -104,9 +104,9 @@ TEST_F(WasmMacroGenTest, Expressions) {
   EXPECT_SIZE(2, WASM_LOAD_GLOBAL(0));
   EXPECT_SIZE(2, WASM_LOAD_GLOBAL(1));
   EXPECT_SIZE(2, WASM_LOAD_GLOBAL(12));
-  EXPECT_SIZE(4, WASM_LOAD_MEM(MachineType::Int32(), WASM_ZERO));
-  EXPECT_SIZE(4, WASM_LOAD_MEM(MachineType::Float64(), WASM_ZERO));
-  EXPECT_SIZE(4, WASM_LOAD_MEM(MachineType::Float32(), WASM_ZERO));
+  EXPECT_SIZE(5, WASM_LOAD_MEM(MachineType::Int32(), WASM_ZERO));
+  EXPECT_SIZE(5, WASM_LOAD_MEM(MachineType::Float64(), WASM_ZERO));
+  EXPECT_SIZE(5, WASM_LOAD_MEM(MachineType::Float32(), WASM_ZERO));
 
   EXPECT_SIZE(3, WASM_NOT(WASM_ZERO));
 
@@ -302,10 +302,10 @@ static const MachineType kMemTypes[] = {
 
 TEST_F(WasmMacroGenTest, LoadsAndStores) {
   for (size_t i = 0; i < arraysize(kMemTypes); i++) {
-    EXPECT_SIZE(4, WASM_LOAD_MEM(kMemTypes[i], WASM_ZERO));
+    EXPECT_SIZE(5, WASM_LOAD_MEM(kMemTypes[i], WASM_ZERO));
   }
   for (size_t i = 0; i < arraysize(kMemTypes); i++) {
-    EXPECT_SIZE(6, WASM_STORE_MEM(kMemTypes[i], WASM_ZERO, WASM_GET_LOCAL(0)));
+    EXPECT_SIZE(7, WASM_STORE_MEM(kMemTypes[i], WASM_ZERO, WASM_GET_LOCAL(0)));
   }
 }