[wasm] Remove implicit returns

Instead, return directly when branching to the outermost block. When
falling through the end of the function block, generate a standard
return, otherwise do not generate a return at the end of the function
block.

R=titzer@chromium.org

Bug: v8:8423
Change-Id: I3c5ffacfa4ef7a960d41bea62920bd98e63d78df
Reviewed-on: https://chromium-review.googlesource.com/c/1369958
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58151}

src/wasm/baseline/liftoff-compiler.cc

@@ -495,11 +495,6 @@ class LiftoffCompiler {
   }
 
   void FallThruTo(FullDecoder* decoder, Control* c) {
-    // Liftoff never breaks to the implicit return. It returns directly
-    // instead. So no need to generate any code for this FallThru.
-    // TODO(clemensh): Simplify this by removing implicit returns from TF.
-    if (is_outermost(c, decoder)) return;
-
     if (c->end_merge.reached) {
       __ MergeFullStackWith(c->label_state);
     } else if (c->is_onearmed_if()) {
@@ -1091,13 +1086,7 @@ class LiftoffCompiler {
         static_cast<uint32_t>(descriptor_->StackParameterCount()));
   }
 
-  void DoReturn(FullDecoder* decoder, Vector<Value> /*values*/, bool implicit) {
-    // Liftoff never breaks to the implicit return. It returns directly
-    // instead. So we only need to generate return code here if we actually fall
-    // thru from the function block.
-    DCHECK_IMPLIES(implicit, decoder->control_depth() == 1);
-    if (implicit && !decoder->control_at(0)->reachable()) return;
-
+  void DoReturn(FullDecoder* decoder, Vector<Value> /*values*/) {
     ReturnImpl(decoder);
   }
 
@@ -1260,16 +1249,20 @@ class LiftoffCompiler {
 
   void Br(FullDecoder* decoder, Control* target) { BrImpl(target); }
 
-  void BrIf(FullDecoder* decoder, const Value& cond, Control* target) {
+  void BrOrRet(FullDecoder* decoder, uint32_t depth) {
+    if (depth == decoder->control_depth() - 1) {
+      ReturnImpl(decoder);
+    } else {
+      BrImpl(decoder->control_at(depth));
+    }
+  }
+
+  void BrIf(FullDecoder* decoder, const Value& cond, uint32_t depth) {
     Label cont_false;
     Register value = __ PopToRegister().gp();
     __ emit_cond_jump(kEqual, &cont_false, kWasmI32, value);
 
-    if (is_outermost(target, decoder)) {
-      ReturnImpl(decoder);
-    } else {
-      BrImpl(target);
-    }
+    BrOrRet(decoder, depth);
     __ bind(&cont_false);
   }
 
@@ -1282,11 +1275,7 @@ class LiftoffCompiler {
       __ jmp(label.get());
     } else {
       __ bind(label.get());
-      if (br_depth == decoder->control_depth() - 1) {
-        ReturnImpl(decoder);
-      } else {
-        BrImpl(decoder->control_at(br_depth));
-      }
+      BrOrRet(decoder, br_depth);
     }
   }
 
@@ -1921,10 +1910,6 @@ class LiftoffCompiler {
 #endif
   }
 
-  bool is_outermost(Control* c, FullDecoder* decoder) {
-    return c == decoder->control_at(decoder->control_depth() - 1);
-  }
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(LiftoffCompiler);
 };
 

src/wasm/function-body-decoder-impl.h

@@ -690,7 +690,7 @@ struct ControlWithNamedConstructors : public ControlBase<Value> {
   F(F64Const, Value* result, double value)                                    \
   F(RefNull, Value* result)                                                   \
   F(Drop, const Value& value)                                                 \
-  F(DoReturn, Vector<Value> values, bool implicit)                            \
+  F(DoReturn, Vector<Value> values)                                           \
   F(GetLocal, Value* result, const LocalIndexImmediate<validate>& imm)        \
   F(SetLocal, const Value& value, const LocalIndexImmediate<validate>& imm)   \
   F(TeeLocal, const Value& value, Value* result,                              \
@@ -701,7 +701,7 @@ struct ControlWithNamedConstructors : public ControlBase<Value> {
   F(Select, const Value& cond, const Value& fval, const Value& tval,          \
     Value* result)                                                            \
   F(Br, Control* target)                                                      \
-  F(BrIf, const Value& cond, Control* target)                                 \
+  F(BrIf, const Value& cond, uint32_t depth)                                  \
   F(BrTable, const BranchTableImmediate<validate>& imm, const Value& key)     \
   F(Else, Control* if_block)                                                  \
   F(LoadMem, LoadType type, const MemoryAccessImmediate<validate>& imm,       \
@@ -1794,11 +1794,11 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               EndControl();
             }
 
+            if (control_.size() > 1) {
               FallThruTo(c);
               // A loop just leaves the values on the stack.
               if (!c->is_loop()) PushMergeValues(c, &c->end_merge);
-
-            if (control_.size() == 1) {
+            } else {
               // If at the last (implicit) control, check we are at end.
               if (!VALIDATE(this->pc_ + 1 == this->end_)) {
                 this->error(this->pc_ + 1, "trailing code after function end");
@@ -1808,7 +1808,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               // The result of the block is the return value.
               TRACE_PART("\n" TRACE_INST_FORMAT, startrel(this->pc_),
                          "(implicit) return");
-              DoReturn(c, true);
+              if (!TypeCheckFallThru(c)) break;
+              DoReturn();
             }
 
             PopControl(c);
@@ -1825,16 +1826,14 @@ class WasmFullDecoder : public WasmDecoder<validate> {
           case kExprBr: {
             BreakDepthImmediate<validate> imm(this, this->pc_);
             if (!this->Validate(this->pc_, imm, control_.size())) break;
-            if (imm.depth == control_.size() - 1) {
-              DoReturn(&control_.back(), false);
-            } else {
             Control* c = control_at(imm.depth);
             if (!TypeCheckBreak(c)) break;
-              if (control_.back().reachable()) {
+            if (imm.depth == control_.size() - 1) {
+              DoReturn();
+            } else if (control_.back().reachable()) {
               CALL_INTERFACE(Br, c);
               c->br_merge()->reached = true;
             }
-            }
             len = 1 + imm.length;
             EndControl();
             break;
@@ -1847,7 +1846,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             Control* c = control_at(imm.depth);
             if (!TypeCheckBreak(c)) break;
             if (control_.back().reachable()) {
-              CALL_INTERFACE(BrIf, cond, c);
+              CALL_INTERFACE(BrIf, cond, imm.depth);
               c->br_merge()->reached = true;
             }
             len = 1 + imm.length;
@@ -1903,7 +1902,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             break;
           }
           case kExprReturn: {
-            DoReturn(&control_.back(), false);
+            if (!TypeCheckReturn()) break;
+            DoReturn();
             break;
           }
           case kExprUnreachable: {
@@ -2571,18 +2571,15 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     return len;
   }
 
-  void DoReturn(Control* c, bool implicit) {
-    if (!TypeCheckReturn()) return;
-
+  void DoReturn() {
     size_t return_count = this->sig_->return_count();
+    DCHECK_GE(stack_.size(), return_count);
     Vector<Value> return_values =
         return_count == 0
             ? Vector<Value>{}
             : Vector<Value>{&*(stack_.end() - return_count), return_count};
 
-    // Simulate that an implicit return morally comes after the current block.
-    bool reached = c->reachable() || (implicit && c->end_merge.reached);
-    if (reached) CALL_INTERFACE(DoReturn, return_values, implicit);
+    CALL_INTERFACE_IF_REACHABLE(DoReturn, return_values);
 
     EndControl();
   }

src/wasm/graph-builder-interface.cc

@@ -243,11 +243,7 @@ class WasmGraphBuildingInterface {
 
   void Drop(FullDecoder* decoder, const Value& value) {}
 
-  void DoReturn(FullDecoder* decoder, Vector<Value> values, bool implicit) {
-    if (implicit) {
-      DCHECK_EQ(1, decoder->control_depth());
-      SetEnv(decoder->control_at(0)->end_env);
-    }
+  void DoReturn(FullDecoder* decoder, Vector<Value> values) {
     TFNode** nodes = GetNodes(values);
     BUILD(Return, static_cast<uint32_t>(values.size()), nodes);
   }
@@ -296,18 +292,30 @@ class WasmGraphBuildingInterface {
     ssa_env_->control = merge;
   }
 
+  void BrOrRet(FullDecoder* decoder, uint32_t depth) {
+    if (depth == decoder->control_depth() - 1) {
+      uint32_t ret_count = static_cast<uint32_t>(decoder->sig_->return_count());
+      TFNode** values =
+          ret_count == 0 ? nullptr
+                         : GetNodes(decoder->stack_value(ret_count), ret_count);
+      BUILD(Return, ret_count, values);
+    } else {
+      Br(decoder, decoder->control_at(depth));
+    }
+  }
+
   void Br(FullDecoder* decoder, Control* target) {
     MergeValuesInto(decoder, target, target->br_merge());
   }
 
-  void BrIf(FullDecoder* decoder, const Value& cond, Control* target) {
+  void BrIf(FullDecoder* decoder, const Value& cond, uint32_t depth) {
     SsaEnv* fenv = ssa_env_;
     SsaEnv* tenv = Split(decoder, fenv);
     fenv->SetNotMerged();
     BUILD(BranchNoHint, cond.node, &tenv->control, &fenv->control);
-    ssa_env_ = tenv;
-    Br(decoder, target);
-    ssa_env_ = fenv;
+    SetEnv(tenv);
+    BrOrRet(decoder, depth);
+    SetEnv(fenv);
   }
 
   void BrTable(FullDecoder* decoder, const BranchTableImmediate<validate>& imm,
@@ -315,7 +323,7 @@ class WasmGraphBuildingInterface {
     if (imm.table_count == 0) {
       // Only a default target. Do the equivalent of br.
       uint32_t target = BranchTableIterator<validate>(decoder, imm).next();
-      Br(decoder, decoder->control_at(target));
+      BrOrRet(decoder, target);
       return;
     }
 
@@ -324,18 +332,18 @@ class WasmGraphBuildingInterface {
     TFNode* sw = BUILD(Switch, imm.table_count + 1, key.node);
 
     SsaEnv* copy = Steal(decoder->zone(), break_env);
-    ssa_env_ = copy;
+    SetEnv(copy);
     BranchTableIterator<validate> iterator(decoder, imm);
     while (iterator.has_next()) {
       uint32_t i = iterator.cur_index();
       uint32_t target = iterator.next();
-      ssa_env_ = Split(decoder, copy);
+      SetEnv(Split(decoder, copy));
       ssa_env_->control =
           (i == imm.table_count) ? BUILD(IfDefault, sw) : BUILD(IfValue, i, sw);
-      Br(decoder, decoder->control_at(target));
+      BrOrRet(decoder, target);
     }
     DCHECK(decoder->ok());
-    ssa_env_ = break_env;
+    SetEnv(break_env);
   }
 
   void Else(FullDecoder* decoder, Control* if_block) {