[wasm] Refactor memory tracing

Instead of passing four arguments to the runtime function, just pass
one pointer to a struct containing all information. This makes it much
easier to implement memory tracing in Liftoff in a follow-up CL.
Also fix a few other minor things like the namespace and the include
guards.

R=titzer@chromium.org

Change-Id: I47d8827cbb896a581585947f594af52f42bdb37c
Reviewed-on: https://chromium-review.googlesource.com/863673Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50568}

src/compiler/wasm-compiler.cc

@@ -34,6 +34,7 @@
 #include "src/log-inl.h"
 #include "src/trap-handler/trap-handler.h"
 #include "src/wasm/function-body-decoder.h"
+#include "src/wasm/memory-tracing.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-module.h"
@@ -3637,21 +3638,28 @@ Node* WasmGraphBuilder::TraceMemoryOperation(bool is_store,
                                              MachineRepresentation rep,
                                              Node* index, uint32_t offset,
                                              wasm::WasmCodePosition position) {
+  int kAlign = 4;  // Ensure that the LSB is 0, such that this looks like a Smi.
+  Node* info = graph()->NewNode(
+      jsgraph()->machine()->StackSlot(sizeof(wasm::MemoryTracingInfo), kAlign));
+
   Node* address = graph()->NewNode(jsgraph()->machine()->Int32Add(),
                                    Int32Constant(offset), index);
-  Node* addr_low = BuildChangeInt32ToSmi(graph()->NewNode(
-      jsgraph()->machine()->Word32And(), address, Int32Constant(0xFFFF)));
-  Node* addr_high = BuildChangeInt32ToSmi(graph()->NewNode(
-      jsgraph()->machine()->Word32Shr(), address, Int32Constant(16)));
-  int32_t rep_i = static_cast<int32_t>(rep);
-  Node* params[] = {
-      jsgraph()->SmiConstant(is_store),  // is_store
-      jsgraph()->SmiConstant(rep_i),     // mem rep
-      addr_low,                          // address lower half word
-      addr_high                          // address higher half word
+  auto store = [&](int offset, MachineRepresentation rep, Node* data) {
+    *effect_ = graph()->NewNode(
+        jsgraph()->machine()->Store(StoreRepresentation(rep, kNoWriteBarrier)),
+        info, jsgraph()->Int32Constant(offset), data, *effect_, *control_);
   };
-  Node* call =
-      BuildCallToRuntime(Runtime::kWasmTraceMemory, params, arraysize(params));
+  // Store address, is_store, and mem_rep.
+  store(offsetof(wasm::MemoryTracingInfo, address),
+        MachineRepresentation::kWord32, address);
+  store(offsetof(wasm::MemoryTracingInfo, is_store),
+        MachineRepresentation::kWord8,
+        jsgraph()->Int32Constant(is_store ? 1 : 0));
+  store(offsetof(wasm::MemoryTracingInfo, mem_rep),
+        MachineRepresentation::kWord8,
+        jsgraph()->Int32Constant(static_cast<int>(rep)));
+
+  Node* call = BuildCallToRuntime(Runtime::kWasmTraceMemory, &info, 1);
   SetSourcePosition(call, position);
   return call;
 }

src/runtime/runtime-test.cc

@@ -1105,11 +1105,11 @@ RUNTIME_FUNCTION(Runtime_RedirectToWasmInterpreter) {
 
 RUNTIME_FUNCTION(Runtime_WasmTraceMemory) {
   HandleScope hs(isolate);
-  DCHECK_EQ(4, args.length());
-  CONVERT_SMI_ARG_CHECKED(is_store, 0);
-  CONVERT_SMI_ARG_CHECKED(mem_rep, 1);
-  CONVERT_SMI_ARG_CHECKED(addr_low, 2);
-  CONVERT_SMI_ARG_CHECKED(addr_high, 3);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_CHECKED(Smi, info_addr, 0);
+
+  wasm::MemoryTracingInfo* info =
+      reinterpret_cast<wasm::MemoryTracingInfo*>(info_addr);
 
   // Find the caller wasm frame.
   StackTraceFrameIterator it(isolate);
@@ -1117,8 +1117,6 @@ RUNTIME_FUNCTION(Runtime_WasmTraceMemory) {
   DCHECK(it.is_wasm());
   WasmCompiledFrame* frame = WasmCompiledFrame::cast(it.frame());
 
-  uint32_t addr = (static_cast<uint32_t>(addr_low) & 0xFFFF) |
-                  (static_cast<uint32_t>(addr_high) << 16);
   uint8_t* mem_start = reinterpret_cast<uint8_t*>(frame->wasm_instance()
                                                       ->memory_object()
                                                       ->array_buffer()
@@ -1128,9 +1126,9 @@ RUNTIME_FUNCTION(Runtime_WasmTraceMemory) {
   // TODO(titzer): eliminate dependency on WasmModule definition here.
   int func_start =
       frame->wasm_instance()->module()->functions[func_index].code.offset();
-  tracing::TraceMemoryOperation(tracing::kWasmCompiled, is_store,
-                                MachineRepresentation(mem_rep), addr,
-                                func_index, pos - func_start, mem_start);
+  // TODO(clemensh): Determine compiler (TurboFan / Liftoff).
+  wasm::TraceMemoryOperation(wasm::ExecutionEngine::kTurbofan, info, func_index,
+                             pos - func_start, mem_start);
   return isolate->heap()->undefined_value();
 }
 

src/runtime/runtime.h

@@ -632,7 +632,7 @@ namespace internal {
   F(HeapObjectVerify, 1, 1)                   \
   F(WasmNumInterpretedCalls, 1, 1)            \
   F(RedirectToWasmInterpreter, 2, 1)          \
-  F(WasmTraceMemory, 4, 1)                    \
+  F(WasmTraceMemory, 1, 1)                    \
   F(CompleteInobjectSlackTracking, 1, 1)      \
   F(IsLiftoffFunction, 1, 1)                  \
   F(FreezeWasmLazyCompilation, 1, 1)

src/wasm/memory-tracing.cc

@@ -8,18 +8,18 @@
 
 namespace v8 {
 namespace internal {
-namespace tracing {
+namespace wasm {
 
-void TraceMemoryOperation(ExecutionEngine engine, bool is_store,
-                          MachineRepresentation rep, uint32_t addr,
+void TraceMemoryOperation(ExecutionEngine engine, const MemoryTracingInfo* info,
                           int func_index, int position, uint8_t* mem_start) {
   EmbeddedVector<char, 64> value;
-  switch (rep) {
-#define TRACE_TYPE(rep, str, format, ctype1, ctype2)           \
-  case MachineRepresentation::rep:                             \
-    SNPrintF(value, str ":" format,                            \
-             ReadLittleEndianValue<ctype1>(mem_start + addr),  \
-             ReadLittleEndianValue<ctype2>(mem_start + addr)); \
+  auto mem_rep = static_cast<MachineRepresentation>(info->mem_rep);
+  switch (mem_rep) {
+#define TRACE_TYPE(rep, str, format, ctype1, ctype2)                    \
+  case MachineRepresentation::rep:                                      \
+    SNPrintF(value, str ":" format,                                     \
+             ReadLittleEndianValue<ctype1>(mem_start + info->address),  \
+             ReadLittleEndianValue<ctype2>(mem_start + info->address)); \
     break;
     TRACE_TYPE(kWord8, " i8", "%d / %02x", uint8_t, uint8_t)
     TRACE_TYPE(kWord16, "i16", "%d / %04x", uint16_t, uint16_t)
@@ -33,17 +33,20 @@ void TraceMemoryOperation(ExecutionEngine engine, bool is_store,
   }
   char eng_c = '?';
   switch (engine) {
-    case kWasmCompiled:
-      eng_c = 'C';
+    case ExecutionEngine::kTurbofan:
+      eng_c = 'T';
       break;
-    case kWasmInterpreted:
+    case ExecutionEngine::kLiftoff:
+      eng_c = 'L';
+      break;
+    case ExecutionEngine::kInterpreter:
       eng_c = 'I';
       break;
   }
   printf("%c %8d+0x%-6x %s @%08x %s\n", eng_c, func_index, position,
-         is_store ? "store" : "read ", addr, value.start());
+         info->is_store ? "store" : "load ", info->address, value.start());
 }
 
-}  // namespace tracing
+}  // namespace wasm
 }  // namespace internal
 }  // namespace v8

src/wasm/memory-tracing.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_MEMORY_TRACING_H
-#define V8_MEMORY_TRACING_H
+#ifndef V8_WASM_MEMORY_TRACING_H_
+#define V8_WASM_MEMORY_TRACING_H_
 
 #include <cstdint>
 
@@ -11,18 +11,31 @@
 
 namespace v8 {
 namespace internal {
-namespace tracing {
+namespace wasm {
 
-enum ExecutionEngine { kWasmCompiled, kWasmInterpreted };
+enum class ExecutionEngine { kTurbofan, kLiftoff, kInterpreter };
+
+// This struct is create in generated code, hence use low-level types.
+struct MemoryTracingInfo {
+  uint32_t address;
+  uint8_t is_store;  // 0 or 1
+  uint8_t mem_rep;
+  static_assert(
+      std::is_same<decltype(mem_rep),
+                   std::underlying_type<MachineRepresentation>::type>::value,
+      "MachineRepresentation uses uint8_t");
+
+  MemoryTracingInfo(uint32_t addr, bool is_store, MachineRepresentation rep)
+      : address(addr), is_store(is_store), mem_rep(static_cast<uint8_t>(rep)) {}
+};
 
 // Callback for tracing a memory operation for debugging.
 // Triggered by --wasm-trace-memory.
-void TraceMemoryOperation(ExecutionEngine, bool is_store, MachineRepresentation,
-                          uint32_t addr, int func_index, int position,
-                          uint8_t* mem_start);
+void TraceMemoryOperation(ExecutionEngine, const MemoryTracingInfo* info,
+                          int func_index, int position, uint8_t* mem_start);
 
-}  // namespace tracing
+}  // namespace wasm
 }  // namespace internal
 }  // namespace v8
 
-#endif /* !V8_MEMORY_TRACING_H */
+#endif  // V8_WASM_MEMORY_TRACING_H_

src/wasm/wasm-interpreter.cc

@@ -1495,10 +1495,10 @@ class ThreadImpl {
     len = 1 + operand.length;
 
     if (FLAG_wasm_trace_memory) {
-      tracing::TraceMemoryOperation(
-          tracing::kWasmInterpreted, false, rep, operand.offset + index,
-          code->function->func_index, static_cast<int>(pc),
-          wasm_context_->mem_start);
+      wasm::MemoryTracingInfo info(operand.offset + index, false, rep);
+      TraceMemoryOperation(ExecutionEngine::kInterpreter, &info,
+                           code->function->func_index, static_cast<int>(pc),
+                           wasm_context_->mem_start);
     }
 
     return true;
@@ -1521,10 +1521,10 @@ class ThreadImpl {
     len = 1 + operand.length;
 
     if (FLAG_wasm_trace_memory) {
-      tracing::TraceMemoryOperation(
-          tracing::kWasmInterpreted, true, rep, operand.offset + index,
-          code->function->func_index, static_cast<int>(pc),
-          wasm_context_->mem_start);
+      wasm::MemoryTracingInfo info(operand.offset + index, true, rep);
+      TraceMemoryOperation(ExecutionEngine::kInterpreter, &info,
+                           code->function->func_index, static_cast<int>(pc),
+                           wasm_context_->mem_start);
     }
 
     return true;

test/message/wasm-trace-memory-interpreted.out

-I        0+0x3      read  @00000004 i32:0 / 00000000
-I        1+0x3      read  @00000001  i8:0 / 00
+I        0+0x3      load  @00000004 i32:0 / 00000000
+I        1+0x3      load  @00000001  i8:0 / 00
 I        3+0x5      store @00000004 i32:305419896 / 12345678
-I        0+0x3      read  @00000002 i32:1450704896 / 56780000
-I        1+0x3      read  @00000006  i8:52 / 34
-I        2+0x3      read  @00000002 f32:68169720922112.000000 / 56780000
+I        0+0x3      load  @00000002 i32:1450704896 / 56780000
+I        1+0x3      load  @00000006  i8:52 / 34
+I        2+0x3      load  @00000002 f32:68169720922112.000000 / 56780000
 I        4+0x5      store @00000004  i8:171 / ab
-I        0+0x3      read  @00000002 i32:1454047232 / 56ab0000
-I        2+0x3      read  @00000002 f32:94008244174848.000000 / 56ab0000
+I        0+0x3      load  @00000002 i32:1454047232 / 56ab0000
+I        2+0x3      load  @00000002 f32:94008244174848.000000 / 56ab0000

test/message/wasm-trace-memory.out

-C        0+0x3      read  @00000004 i32:0 / 00000000
-C        1+0x3      read  @00000001  i8:0 / 00
-C        3+0x5      store @00000004 i32:305419896 / 12345678
-C        0+0x3      read  @00000002 i32:1450704896 / 56780000
-C        1+0x3      read  @00000006  i8:52 / 34
-C        2+0x3      read  @00000002 f32:68169720922112.000000 / 56780000
-C        4+0x5      store @00000004  i8:171 / ab
-C        0+0x3      read  @00000002 i32:1454047232 / 56ab0000
-C        2+0x3      read  @00000002 f32:94008244174848.000000 / 56ab0000
+T        0+0x3      load  @00000004 i32:0 / 00000000
+T        1+0x3      load  @00000001  i8:0 / 00
+T        3+0x5      store @00000004 i32:305419896 / 12345678
+T        0+0x3      load  @00000002 i32:1450704896 / 56780000
+T        1+0x3      load  @00000006  i8:52 / 34
+T        2+0x3      load  @00000002 f32:68169720922112.000000 / 56780000
+T        4+0x5      store @00000004  i8:171 / ab
+T        0+0x3      load  @00000002 i32:1454047232 / 56ab0000
+T        2+0x3      load  @00000002 f32:94008244174848.000000 / 56ab0000