[wasm] Some simplifications in function-body-decoder.cc.

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 81 matching lines @@
 };
 
 static Value* NO_VALUE = nullptr;
 
 enum ControlKind { kControlIf, kControlBlock, kControlLoop, kControlTry };
 
 // An entry on the control stack (i.e. if, block, loop).
 struct Control {
   const byte* pc;
   ControlKind kind;
-  int stack_depth;         // stack height at the beginning of the construct.
+  size_t stack_depth;      // stack height at the beginning of the construct.
   SsaEnv* end_env;         // end environment for the construct.
   SsaEnv* false_env;       // false environment (only for if).
   TryInfo* try_info;       // Information used for compiling try statements.
   int32_t previous_catch;  // The previous Control (on the stack) with a catch.
 
   // Values merged into the end of this control construct.
   MergeValues merge;
 
   inline bool is_if() const { return kind == kControlIf; }
   inline bool is_block() const { return kind == kControlBlock; }
   inline bool is_loop() const { return kind == kControlLoop; }
   inline bool is_try() const { return kind == kControlTry; }
 
   // Named constructors.
-  static Control Block(const byte* pc, int stack_depth, SsaEnv* end_env,
+  static Control Block(const byte* pc, size_t stack_depth, SsaEnv* end_env,
                        int32_t previous_catch) {
     return {pc,      kControlBlock, stack_depth,    end_env,
             nullptr, nullptr,       previous_catch, {0, {NO_VALUE}}};
   }
 
-  static Control If(const byte* pc, int stack_depth, SsaEnv* end_env,
+  static Control If(const byte* pc, size_t stack_depth, SsaEnv* end_env,
                     SsaEnv* false_env, int32_t previous_catch) {
     return {pc,        kControlIf, stack_depth,    end_env,
             false_env, nullptr,    previous_catch, {0, {NO_VALUE}}};
   }
 
-  static Control Loop(const byte* pc, int stack_depth, SsaEnv* end_env,
+  static Control Loop(const byte* pc, size_t stack_depth, SsaEnv* end_env,
                       int32_t previous_catch) {
     return {pc,      kControlLoop, stack_depth,    end_env,
             nullptr, nullptr,      previous_catch, {0, {NO_VALUE}}};
   }
 
-  static Control Try(const byte* pc, int stack_depth, SsaEnv* end_env,
+  static Control Try(const byte* pc, size_t stack_depth, SsaEnv* end_env,
                      Zone* zone, SsaEnv* catch_env, int32_t previous_catch) {
     DCHECK_NOT_NULL(catch_env);
     TryInfo* try_info = new (zone) TryInfo(catch_env);
     return {pc,      kControlTry, stack_depth,    end_env,
             nullptr, try_info,    previous_catch, {0, {NO_VALUE}}};
   }
 };
 
 // Macros that build nodes only if there is a graph and the current SSA
 // environment is reachable from start. This avoids problems with malformed
@@ skipping 503 matching lines @@
       } else if (c->merge.arity > 1) {
         c->merge.vals.array = zone_->NewArray<Value>(c->merge.arity);
         for (unsigned i = 0; i < c->merge.arity; i++) {
           c->merge.vals.array[i] = {pc_, nullptr, sig_->GetReturn(i)};
         }
       }
     }
 
     if (pc_ >= end_) return;  // Nothing to do.
 
     while (true) {  // decoding loop.

[Clemens Hammacher, 2017/01/18 15:46:26]

You can also simplify this loop by removing the if-statement above, and making
this "while (pc_ < end_)". See below for more.

[titzer, 2017/01/18 17:22:17]

Yeah, I originally did this on purpose as a manual loop rotation, but it's
better to just let the C++ compiler do it.

       unsigned len = 1;
       WasmOpcode opcode = static_cast<WasmOpcode>(*pc_);
       if (!WasmOpcodes::IsPrefixOpcode(opcode)) {
         TRACE("  @%-8d #%02x:%-20s|", startrel(pc_), opcode,
               WasmOpcodes::ShortOpcodeName(opcode));
       }
 
       FunctionSig* sig = WasmOpcodes::Signature(opcode);
       if (sig) {
         BuildSimpleOperator(opcode, sig);
@@ skipping 134 matching lines @@
               // A loop just leaves the values on the stack.
               TypeCheckLoopFallThru(c);
               PopControl();
               SetEnv("loop:end", ssa_env_);
               break;
             }
             if (c->is_if()) {
               if (c->false_env != nullptr) {
                 // End the true branch of a one-armed if.
                 Goto(c->false_env, c->end_env);
-                if (ssa_env_->go() &&
-                    static_cast<int>(stack_.size()) != c->stack_depth) {
+                if (ssa_env_->go() && stack_.size() != c->stack_depth) {
                   error("end of if expected empty stack");
                   stack_.resize(c->stack_depth);
                 }
                 if (c->merge.arity > 0) {
                   error("non-void one-armed if");
                 }
                 name = "if:merge";
               } else {
                 // End the false branch of a two-armed if.
                 name = "if_else:merge";
@@ skipping 13 matching lines @@
             // Push the end values onto the stack.
             stack_.resize(c->stack_depth);
             if (c->merge.arity == 1) {
               stack_.push_back(c->merge.vals.first);
             } else {
               for (unsigned i = 0; i < c->merge.arity; i++) {
                 stack_.push_back(c->merge.vals.array[i]);
               }
             }
 
-            PopControl();
-
-            if (control_.empty()) {
+            if (control_.size() == 1) {
               // If the last (implicit) control was popped, check we are at end.

[rossberg, 2017/01/18 16:21:42]

Nit: s/was/is/

[titzer, 2017/01/18 17:22:17]

Done.

               if (pc_ + 1 != end_) {
                 error(pc_, pc_ + 1, "trailing code after function end");
+                break;
               }
               last_end_found_ = true;
               if (ssa_env_->go()) {
                 // The result of the block is the return value.
                 TRACE("  @%-8d #xx:%-20s|", startrel(pc_), "ImplicitReturn");
                 DoReturn();
                 TRACE("\n");
               }
-              return;
             }
+            PopControl();
             break;
           }
           case kExprSelect: {
             Value cond = Pop(2, kWasmI32);
             Value fval = Pop();
             Value tval = Pop();
             if (tval.type == kWasmStmt || tval.type != fval.type) {
               if (tval.type != kWasmEnd && fval.type != kWasmEnd) {
                 error("type mismatch in select");
                 break;
@@ skipping 359 matching lines @@
               break;
             }
             default:
               break;
           }
         }
         PrintF("\n");
       }
 #endif
       pc_ += len;
       if (pc_ >= end_) {

[Clemens Hammacher, 2017/01/18 15:46:26]

This condition could go away then, just put this after the loop:
if (pc_ > end_) error("Beyond end of code");

[titzer, 2017/01/18 17:22:17]

Done.

         // End of code reached or exceeded.
         if (pc_ > end_ && ok()) error("Beyond end of code");
         return;
       }
     }  // end decode loop
   }
 
   void EndControl() { ssa_env_->Kill(SsaEnv::kControlEnd); }
 
   void SetBlockType(Control* c, BlockTypeOperand& operand) {
@@ skipping 24 matching lines @@
       }
       return nullptr;
     }
   }
 
   ValueType GetReturnType(FunctionSig* sig) {
     return sig->return_count() == 0 ? kWasmStmt : sig->GetReturn();
   }
 
   void PushBlock(SsaEnv* end_env) {
-    const int stack_depth = static_cast<int>(stack_.size());
     control_.emplace_back(
-        Control::Block(pc_, stack_depth, end_env, current_catch_));
+        Control::Block(pc_, stack_.size(), end_env, current_catch_));
   }
 
   void PushLoop(SsaEnv* end_env) {
-    const int stack_depth = static_cast<int>(stack_.size());
     control_.emplace_back(
-        Control::Loop(pc_, stack_depth, end_env, current_catch_));
+        Control::Loop(pc_, stack_.size(), end_env, current_catch_));
   }
 
   void PushIf(SsaEnv* end_env, SsaEnv* false_env) {
-    const int stack_depth = static_cast<int>(stack_.size());
     control_.emplace_back(
-        Control::If(pc_, stack_depth, end_env, false_env, current_catch_));
+        Control::If(pc_, stack_.size(), end_env, false_env, current_catch_));
   }
 
   void PushTry(SsaEnv* end_env, SsaEnv* catch_env) {
-    const int stack_depth = static_cast<int>(stack_.size());
-    control_.emplace_back(Control::Try(pc_, stack_depth, end_env, zone_,
+    control_.emplace_back(Control::Try(pc_, stack_.size(), end_env, zone_,
                                        catch_env, current_catch_));
     current_catch_ = static_cast<int32_t>(control_.size() - 1);
   }
 
   void PopControl() { control_.pop_back(); }
 
   int DecodeLoadMem(ValueType type, MachineType mem_type) {
     if (!CheckHasMemory()) return 0;
     MemoryAccessOperand operand(this, pc_,
                                 ElementSizeLog2Of(mem_type.representation()));
@@ skipping 105 matching lines @@
       Push(sig->GetReturn(i), rets ? rets[i] : nullptr);
     }
   }
 
   const char* SafeOpcodeNameAt(const byte* pc) {
     if (pc >= end_) return "<end>";
     return WasmOpcodes::ShortOpcodeName(static_cast<WasmOpcode>(*pc));
   }
 
   Value Pop(int index, ValueType expected) {
-    if (!ssa_env_->go()) {
-      // Unreachable code is essentially not typechecked.
-      return {pc_, nullptr, expected};
-    }
     Value val = Pop();
     if (val.type != expected) {
       if (val.type != kWasmEnd) {
         error(pc_, val.pc, "%s[%d] expected type %s, found %s of type %s",
               SafeOpcodeNameAt(pc_), index, WasmOpcodes::TypeName(expected),
               SafeOpcodeNameAt(val.pc), WasmOpcodes::TypeName(val.type));
       }
     }
     return val;
   }
 
   Value Pop() {
-    if (!ssa_env_->go()) {
-      // Unreachable code is essentially not typechecked.
-      return {pc_, nullptr, kWasmEnd};
-    }
     size_t limit = control_.empty() ? 0 : control_.back().stack_depth;
     if (stack_.size() <= limit) {
-      Value val = {pc_, nullptr, kWasmStmt};
-      error(pc_, pc_, "%s found empty stack", SafeOpcodeNameAt(pc_));
+      Value val = {pc_, nullptr, kWasmEnd};
+      if (ssa_env_->go()) {
+        // Popping past the current control start in reachable code.
+        error(pc_, pc_, "%s found empty stack", SafeOpcodeNameAt(pc_));
+      }
       return val;
     }
     Value val = stack_.back();
     stack_.pop_back();
     return val;
   }
 
-  Value PopUpTo(int stack_depth) {
-    if (!ssa_env_->go()) {
-      // Unreachable code is essentially not typechecked.
-      return {pc_, nullptr, kWasmEnd};
-    }
-    if (stack_depth == static_cast<int>(stack_.size())) {
-      Value val = {pc_, nullptr, kWasmStmt};
-      return val;
-    } else {
-      DCHECK_LE(stack_depth, stack_.size());
-      Value val = Pop();
-      stack_.resize(stack_depth);
-      return val;
-    }
-  }
-
   int baserel(const byte* ptr) {
     return base_ ? static_cast<int>(ptr - base_) : 0;
   }
 
   int startrel(const byte* ptr) { return static_cast<int>(ptr - start_); }
 
   void BreakTo(unsigned depth) {
     if (!ssa_env_->go()) return;
     Control* c = &control_[control_.size() - depth - 1];
     if (c->is_loop()) {
@@ skipping 10 matching lines @@
         return;
       }
       MergeValuesInto(c);
     }
   }
 
   void FallThruTo(Control* c) {
     if (!ssa_env_->go()) return;
     // Merge the value(s) into the end of the block.
     int arity = static_cast<int>(c->merge.arity);
-    if (c->stack_depth + arity != static_cast<int>(stack_.size())) {
+    if (c->stack_depth + arity != stack_.size()) {
       error(pc_, pc_, "expected %d elements on the stack for fallthru to @%d",
             arity, startrel(c->pc));
       return;
     }
     MergeValuesInto(c);
   }
 
   inline Value& GetMergeValueFromStack(Control* c, int i) {
     return stack_[stack_.size() - c->merge.arity + i];
   }
 
   void TypeCheckLoopFallThru(Control* c) {
     if (!ssa_env_->go()) return;
     // Fallthru must match arity exactly.
     int arity = static_cast<int>(c->merge.arity);
-    if (c->stack_depth + arity != static_cast<int>(stack_.size())) {
+    if (c->stack_depth + arity != stack_.size()) {
       error(pc_, pc_, "expected %d elements on the stack for fallthru to @%d",
             arity, startrel(c->pc));
       return;
     }
     // Typecheck the values left on the stack.
     for (unsigned i = 0; i < c->merge.arity; i++) {
       Value& val = GetMergeValueFromStack(c, i);
       Value& old =
           c->merge.arity == 1 ? c->merge.vals.first : c->merge.vals.array[i];
       if (val.type != old.type) {
@@ skipping 499 matching lines @@
 BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
                                            const byte* start, const byte* end) {
   Decoder decoder(start, end);
   return WasmDecoder::AnalyzeLoopAssignment(&decoder, start,
                                             static_cast<int>(num_locals), zone);
 }
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8