From 9f5018c9b0ec2fef9949e2f5f434987ef3392f7b Mon Sep 17 00:00:00 2001
From: Clemens Hammacher <clemensh@chromium.org>
Date: Tue, 8 May 2018 12:48:27 +0200
Subject: [PATCH] [Liftoff] Add code comments on debug builds

Code comments help a lot to understand the generated code. Add a
comment before each instruction, and some special comments for longer
instructions.

R=titzer@chromium.org

Bug: v8:6600
Change-Id: Ic18974e5cc89e23533e3abc54b0389723b77ff73
Reviewed-on: https://chromium-review.googlesource.com/1049626
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53064}
---
 src/wasm/baseline/liftoff-compiler.cc | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/wasm/baseline/liftoff-compiler.cc b/src/wasm/baseline/liftoff-compiler.cc
index 53b9a65616..d766024afe 100644
--- a/src/wasm/baseline/liftoff-compiler.cc
+++ b/src/wasm/baseline/liftoff-compiler.cc
@@ -43,6 +43,15 @@ namespace {
   __ LoadFromInstance(dst.gp(), WASM_INSTANCE_OBJECT_OFFSET(name), \
                       LoadType(type).size());
 
+#ifdef DEBUG
+#define DEBUG_CODE_COMMENT(str) \
+  do {                          \
+    __ RecordComment(str);      \
+  } while (false)
+#else
+#define DEBUG_CODE_COMMENT(str) ((void)0)
+#endif
+
 constexpr LoadType::LoadTypeValue kPointerLoadType =
     kPointerSize == 8 ? LoadType::kI64Load : LoadType::kI32Load;
 
@@ -493,8 +502,9 @@ class LiftoffCompiler {
     BindUnboundLabels(decoder);
   }
 
-  void NextInstruction(Decoder* decoder, WasmOpcode) {
+  void NextInstruction(Decoder* decoder, WasmOpcode opcode) {
     TraceCacheState(decoder);
+    DEBUG_CODE_COMMENT(WasmOpcodes::OpcodeName(opcode));
   }
 
   void Block(Decoder* decoder, Control* block) {
@@ -1473,6 +1483,7 @@ class LiftoffCompiler {
     if (!FLAG_untrusted_code_mitigations || env_->use_trap_handler) {
       return index;
     }
+    DEBUG_CODE_COMMENT("Mask memory index");
     // Make sure that we can overwrite {index}.
     if (__ cache_state()->is_used(index)) {
       LiftoffRegister old_index = index;
@@ -1501,6 +1512,7 @@ class LiftoffCompiler {
     }
     uint32_t offset = imm.offset;
     index = AddMemoryMasking(index, &offset, pinned);
+    DEBUG_CODE_COMMENT("Load from memory");
     LiftoffRegister addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
     LOAD_INSTANCE_FIELD(addr, MemoryStart, kPointerLoadType);
     RegClass rc = reg_class_for(value_type);
@@ -1534,6 +1546,7 @@ class LiftoffCompiler {
     }
     uint32_t offset = imm.offset;
     index = AddMemoryMasking(index, &offset, pinned);
+    DEBUG_CODE_COMMENT("Store to memory");
     LiftoffRegister addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
     LOAD_INSTANCE_FIELD(addr, MemoryStart, kPointerLoadType);
     uint32_t protected_store_pc = 0;
@@ -1741,6 +1754,7 @@ class LiftoffCompiler {
     __ emit_cond_jump(kUnsignedGreaterEqual, invalid_func_label, kWasmI32,
                       index.gp(), tmp_const.gp());
 
+    DEBUG_CODE_COMMENT("Check indirect call signature");
     // Load the signature from {instance->ift_sig_ids[key]}
     LOAD_INSTANCE_FIELD(table, IndirectFunctionTableSigIds, kPointerLoadType);
     __ LoadConstant(tmp_const,
@@ -1759,6 +1773,7 @@ class LiftoffCompiler {
                       LiftoffAssembler::kWasmIntPtr, scratch.gp(),
                       tmp_const.gp());
 
+    DEBUG_CODE_COMMENT("Execute indirect call");
     if (kPointerSize == 8) {
       // {index} has already been multiplied by 4. Multiply by another 2.
       __ LoadConstant(tmp_const, WasmValue(2));
@@ -1964,6 +1979,7 @@ void LiftoffCompilationUnit::AbortCompilation() {
 #undef TRACE
 #undef WASM_INSTANCE_OBJECT_OFFSET
 #undef LOAD_INSTANCE_FIELD
+#undef DEBUG_CODE_COMMENT
 
 }  // namespace wasm
 }  // namespace internal