[wasm] simplify dependencies between graph builder and function decoder

src/wasm/function-body-decoder.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/signature.h"
 
 #include "src/bit-vector.h"
 #include "src/flags.h"
 #include "src/handles.h"
 #include "src/zone/zone-containers.h"
@@ skipping 13 matching lines @@
 
 #if DEBUG
 #define TRACE(...)                                    \
   do {                                                \
     if (FLAG_trace_wasm_decoder) PrintF(__VA_ARGS__); \
   } while (false)
 #else
 #define TRACE(...)
 #endif
 
-#define CHECK_PROTOTYPE_OPCODE(flag)                        \
-  if (module_ && module_->module->origin == kAsmJsOrigin) { \
-    error("Opcode not supported for asmjs modules");        \
-  }                                                         \
-  if (!FLAG_##flag) {                                       \
-    error("Invalid opcode (enable with --" #flag ")");      \
-    break;                                                  \
+#define CHECK_PROTOTYPE_OPCODE(flag)                           \
+  if (module_ != nullptr && module_->origin == kAsmJsOrigin) { \
+    error("Opcode not supported for asmjs modules");           \
+  }                                                            \
+  if (!FLAG_##flag) {                                          \
+    error("Invalid opcode (enable with --" #flag ")");         \
+    break;                                                     \
   }
 // TODO(titzer): this is only for intermediate migration.
 #define IMPLICIT_FUNCTION_END 1
 
 // An SsaEnv environment carries the current local variable renaming
 // as well as the current effect and control dependency in the TF graph.
 // It maintains a control state that tracks whether the environment
 // is reachable, has reached a control end, or has been merged.
 struct SsaEnv {
   enum State { kControlEnd, kUnreachable, kReached, kMerged };
@@ skipping 106 matching lines @@
   inline LaneOperand(Decoder* decoder, const byte* pc) {
     lane = decoder->checked_read_u8(pc, 2, "lane");
     length = 1;
   }
 };
 
 // Generic Wasm bytecode decoder with utilities for decoding operands,
 // lengths, etc.
 class WasmDecoder : public Decoder {
  public:
-  WasmDecoder(ModuleEnv* module, FunctionSig* sig, const byte* start,
+  WasmDecoder(const WasmModule* module, FunctionSig* sig, const byte* start,
               const byte* end)
       : Decoder(start, end),
         module_(module),
         sig_(sig),
         total_locals_(0),
         local_types_(nullptr) {}
-  ModuleEnv* module_;
+  const WasmModule* module_;
   FunctionSig* sig_;
   size_t total_locals_;
   ZoneVector<ValueType>* local_types_;
 
   inline bool Validate(const byte* pc, LocalIndexOperand& operand) {
     if (operand.index < total_locals_) {
       if (local_types_) {
         operand.type = local_types_->at(operand.index);
       } else {
         operand.type = kWasmStmt;
       }
       return true;
     }
     error(pc, pc + 1, "invalid local index: %u", operand.index);
     return false;
   }
 
   inline bool Validate(const byte* pc, GlobalIndexOperand& operand) {
-    ModuleEnv* m = module_;
-    if (m && m->module && operand.index < m->module->globals.size()) {
-      operand.global = &m->module->globals[operand.index];
+    if (module_ != nullptr && operand.index < module_->globals.size()) {
+      operand.global = &module_->globals[operand.index];
       operand.type = operand.global->type;
       return true;
     }
     error(pc, pc + 1, "invalid global index: %u", operand.index);
     return false;
   }
 
   inline bool Complete(const byte* pc, CallFunctionOperand& operand) {
-    ModuleEnv* m = module_;
-    if (m && m->module && operand.index < m->module->functions.size()) {
-      operand.sig = m->module->functions[operand.index].sig;
+    if (module_ != nullptr && operand.index < module_->functions.size()) {
+      operand.sig = module_->functions[operand.index].sig;
       return true;
     }
     return false;
   }
 
   inline bool Validate(const byte* pc, CallFunctionOperand& operand) {
     if (Complete(pc, operand)) {
       return true;
     }
     error(pc, pc + 1, "invalid function index: %u", operand.index);
     return false;
   }
 
   inline bool Complete(const byte* pc, CallIndirectOperand& operand) {
-    ModuleEnv* m = module_;
-    if (m && m->module && operand.index < m->module->signatures.size()) {
-      operand.sig = m->module->signatures[operand.index];
+    if (module_ != nullptr && operand.index < module_->signatures.size()) {
+      operand.sig = module_->signatures[operand.index];
       return true;
     }
     return false;
   }
 
   inline bool Validate(const byte* pc, CallIndirectOperand& operand) {
     uint32_t table_index = 0;
-    if (!module_->IsValidTable(table_index)) {
+    if (module_ == nullptr || module_->function_tables.empty()) {
       error("function table has to exist to execute call_indirect");
       return false;
     }
     if (Complete(pc, operand)) {
       return true;
     }
     error(pc, pc + 1, "invalid signature index: #%u", operand.index);
     return false;
   }
 
@@ skipping 114 matching lines @@
         }
       }
       default:
         return 1;
     }
   }
 };
 
 static const int32_t kNullCatch = -1;
 
-// The full WASM decoder for bytecode. Both verifies bytecode and generates
+// The full WASM decoder for bytecode. Verifies bytecode or generates

[titzer, 2017/01/04 03:32:41]

The comment should reflect that in the later case, it also verifies the bytecode
as a side-effect.

[Mircea Trofin, 2017/01/04 04:55:33]

Good point - done.

 // a TurboFan IR graph.
 class WasmFullDecoder : public WasmDecoder {
  public:
+  WasmFullDecoder(Zone* zone, const FunctionBody& body)

[titzer, 2017/01/04 03:32:41]

I think it's better to have only a single constructor instead of constructor
chaining.

[Mircea Trofin, 2017/01/04 04:55:33]

I tend to agree. I will make more changes in the upcoming days, which
should make the issue moot. Right now, it helps me understand the 
usages a bit better.

+      : WasmFullDecoder(zone, nullptr, nullptr, body) {}
+
+  WasmFullDecoder(Zone* zone, const wasm::WasmModule* module,
+                  const FunctionBody& body)
+      : WasmFullDecoder(zone, module, nullptr, body) {}
+
   WasmFullDecoder(Zone* zone, TFBuilder* builder, const FunctionBody& body)
-      : WasmDecoder(body.module, body.sig, body.start, body.end),
-        zone_(zone),
-        builder_(builder),
-        base_(body.base),
-        local_type_vec_(zone),
-        stack_(zone),
-        control_(zone),
-        last_end_found_(false),
-        current_catch_(kNullCatch) {
-    local_types_ = &local_type_vec_;
-  }
+      : WasmFullDecoder(zone, builder->module_env() == nullptr
+                                  ? nullptr
+                                  : builder->module_env()->module,
+                        builder, body) {}
 
   bool Decode() {
     if (FLAG_wasm_code_fuzzer_gen_test) {
       PrintWasmCodeForDebugging(start_, end_);
     }
     base::ElapsedTimer decode_timer;
     if (FLAG_trace_wasm_decode_time) {
       decode_timer.Start();
     }
     stack_.clear();
@@ skipping 93 matching lines @@
     total_locals_ = num_locals;
     local_type_vec_.reserve(num_locals);
     if (num_locals > local_type_vec_.size()) {
       local_type_vec_.insert(local_type_vec_.end(),
                              num_locals - local_type_vec_.size(), kWasmI32);
     }
     return AnalyzeLoopAssignment(pc);
   }
 
  private:
+  WasmFullDecoder(Zone* zone, const wasm::WasmModule* module,
+                  TFBuilder* builder, const FunctionBody& body)
+      : WasmDecoder(module, body.sig, body.start, body.end),
+        zone_(zone),
+        builder_(builder),
+        base_(body.base),
+        local_type_vec_(zone),
+        stack_(zone),
+        control_(zone),
+        last_end_found_(false),
+        current_catch_(kNullCatch) {
+    local_types_ = &local_type_vec_;
+  }
+
   static const size_t kErrorMsgSize = 128;
 
   Zone* zone_;
   TFBuilder* builder_;
   const byte* base_;
 
   SsaEnv* ssa_env_;
 
   ZoneVector<ValueType> local_type_vec_;  // types of local variables.
   ZoneVector<Value> stack_;               // stack of values.
@@ skipping 24 matching lines @@
       }
       while (index < local_type_vec_.size()) {
         ValueType type = local_type_vec_[index];
         TFNode* node = DefaultValue(type);
         while (index < local_type_vec_.size() &&
                local_type_vec_[index] == type) {
           // Do a whole run of like-typed locals at a time.
           ssa_env->locals[index++] = node;
         }
       }
-      builder_->set_module(module_);
     }
     ssa_env->control = start;
     ssa_env->effect = start;
     SetEnv("initial", ssa_env);
     if (builder_) {
       builder_->StackCheck(position());
     }
   }
 
   TFNode* DefaultValue(ValueType type) {
@@ skipping 561 matching lines @@
             len = DecodeStoreMem(kWasmI64, MachineType::Int64());
             break;
           case kExprF32StoreMem:
             len = DecodeStoreMem(kWasmF32, MachineType::Float32());
             break;
           case kExprF64StoreMem:
             len = DecodeStoreMem(kWasmF64, MachineType::Float64());
             break;
           case kExprGrowMemory: {
             MemoryIndexOperand operand(this, pc_);
-            if (module_->module->origin != kAsmJsOrigin) {
+            DCHECK_NOT_NULL(module_);
+            if (module_->origin != kAsmJsOrigin) {
               Value val = Pop(0, kWasmI32);
               Push(kWasmI32, BUILD(GrowMemory, val.node));
             } else {
               error("grow_memory is not supported for asmjs modules");
             }
             len = 1 + operand.length;
             break;
           }
           case kExprMemorySize: {
             MemoryIndexOperand operand(this, pc_);
@@ skipping 29 matching lines @@
             CHECK_PROTOTYPE_OPCODE(wasm_simd_prototype);
             len++;
             byte simd_index = checked_read_u8(pc_, 1, "simd index");
             opcode = static_cast<WasmOpcode>(opcode << 8 | simd_index);
             TRACE("  @%-4d #%02x #%02x:%-20s|", startrel(pc_), kSimdPrefix,
                   simd_index, WasmOpcodes::ShortOpcodeName(opcode));
             len += DecodeSimdOpcode(opcode);
             break;
           }
           case kAtomicPrefix: {
-            if (!module_ || module_->module->origin != kAsmJsOrigin) {
+            if (module_ == nullptr || module_->origin != kAsmJsOrigin) {
               error("Atomics are allowed only in AsmJs modules");
               break;
             }
             if (!FLAG_wasm_atomics_prototype) {
               error("Invalid opcode (enable with --wasm_atomics_prototype)");
               break;
             }
             len = 2;
             byte atomic_opcode = checked_read_u8(pc_, 1, "atomic index");
             opcode = static_cast<WasmOpcode>(opcode << 8 | atomic_opcode);
             sig = WasmOpcodes::AtomicSignature(opcode);
             if (sig) {
               BuildAtomicOperator(opcode);
             }
             break;
           }
           default: {
             // Deal with special asmjs opcodes.
-            if (module_ && module_->module->origin == kAsmJsOrigin) {
+            if (module_ != nullptr && module_->origin == kAsmJsOrigin) {
               sig = WasmOpcodes::AsmjsSignature(opcode);
               if (sig) {
                 BuildSimpleOperator(opcode, sig);
               }
             } else {
               error("Invalid opcode");
               return;
             }
           }
         }
@@ skipping 685 matching lines @@
         break;
     }
     Push(GetReturnType(sig), node);
   }
 };
 
 bool DecodeLocalDecls(BodyLocalDecls& decls, const byte* start,
                       const byte* end) {
   AccountingAllocator allocator;
   Zone tmp(&allocator, ZONE_NAME);
-  FunctionBody body = {nullptr, nullptr, nullptr, start, end};
-  WasmFullDecoder decoder(&tmp, nullptr, body);
+  FunctionBody body = {nullptr, nullptr, start, end};
+  WasmFullDecoder decoder(&tmp, body);
   return decoder.DecodeLocalDecls(decls);
 }
 
 BytecodeIterator::BytecodeIterator(const byte* start, const byte* end,
                                    BodyLocalDecls* decls)
     : Decoder(start, end) {
   if (decls != nullptr) {
     if (DecodeLocalDecls(*decls, start, end)) {
       pc_ += decls->decls_encoded_size;
       if (pc_ > end_) pc_ = end_;
     }
   }
 }
 
 DecodeResult VerifyWasmCode(AccountingAllocator* allocator,
+                            const wasm::WasmModule* module,
                             FunctionBody& body) {
   Zone zone(allocator, ZONE_NAME);
-  WasmFullDecoder decoder(&zone, nullptr, body);
+  WasmFullDecoder decoder(&zone, module, body);
   decoder.Decode();
   return decoder.toResult<DecodeStruct*>(nullptr);
 }
 
 DecodeResult BuildTFGraph(AccountingAllocator* allocator, TFBuilder* builder,
                           FunctionBody& body) {
   Zone zone(allocator, ZONE_NAME);
   WasmFullDecoder decoder(&zone, builder, body);
   decoder.Decode();
   return decoder.toResult<DecodeStruct*>(nullptr);
 }
 
 unsigned OpcodeLength(const byte* pc, const byte* end) {
   WasmDecoder decoder(nullptr, nullptr, pc, end);
   return decoder.OpcodeLength(pc);
 }
 
 void PrintWasmCodeForDebugging(const byte* start, const byte* end) {
   AccountingAllocator allocator;
   OFStream os(stdout);
-  PrintWasmCode(&allocator, FunctionBodyForTesting(start, end), os, nullptr);
+  PrintWasmCode(&allocator, FunctionBodyForTesting(start, end), nullptr, os,
+                nullptr);
 }
 
 bool PrintWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
-                   std::ostream& os,
+                   const wasm::WasmModule* module, std::ostream& os,
                    std::vector<std::tuple<uint32_t, int, int>>* offset_table) {
   Zone zone(allocator, ZONE_NAME);
-  WasmFullDecoder decoder(&zone, nullptr, body);
+  WasmFullDecoder decoder(&zone, module, body);
   int line_nr = 0;
 
   // Print the function signature.
   if (body.sig) {
     os << "// signature: " << *body.sig << std::endl;
     ++line_nr;
   }
 
   // Print the local declarations.
   BodyLocalDecls decls(&zone);
@@ skipping 97 matching lines @@
     }
     os << std::endl;
     ++line_nr;
   }
 
   return decoder.ok();
 }
 
 BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
                                            const byte* start, const byte* end) {
-  FunctionBody body = {nullptr, nullptr, nullptr, start, end};
-  WasmFullDecoder decoder(zone, nullptr, body);
+  FunctionBody body = {nullptr, nullptr, start, end};
+  WasmFullDecoder decoder(zone, body);
   return decoder.AnalyzeLoopAssignmentForTesting(start, num_locals);
 }
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8