Reland "[wasm][liftoff] Optimize one-armed ifs"

This is a reland of c2aaf0a6

Original change's description:
> [wasm][liftoff] Optimize one-armed ifs
> 
> Do not implement one-armed ifs by emulating an empty else branch. In
> Liftoff, we can generate better code and save compile time by handling
> this specially. If the merge point at the end of the if is not reached
> by the if-branch, we do not need to generate any merge code.
> 
> R=titzer@chromium.org
> 
> Bug: v8:6600, v8:8423
> Change-Id: Ie8ea69dd7491f225605a8e1b986d275d869aa90b
> Reviewed-on: https://chromium-review.googlesource.com/c/1356508
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#57968}

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

src/wasm/baseline/liftoff-compiler.cc

@@ -498,8 +498,10 @@ class LiftoffCompiler {
     if (c->end_merge.reached) {
       __ MergeFullStackWith(c->label_state);
     } else if (c->is_onearmed_if()) {
-      c->label_state.InitMerge(*__ cache_state(), __ num_locals(),
-                               c->br_merge()->arity);
+      // Init the merge point from the else state, then merge the if state into
+      // that.
+      DCHECK_EQ(0, c->end_merge.arity);
+      c->label_state.InitMerge(c->else_state->state, __ num_locals(), 0);
       __ MergeFullStackWith(c->label_state);
     } else {
       c->label_state.Split(*__ cache_state());
@@ -508,7 +510,22 @@ class LiftoffCompiler {
   }
 
   void PopControl(FullDecoder* decoder, Control* c) {
-    if (!c->is_loop() && c->end_merge.reached) {
+    if (c->is_onearmed_if()) {
+      if (c->end_merge.reached) {
+        // Generate the code to merge the else state into the end state.
+        // TODO(clemensh): Do this without switching to the else state first.
+        __ emit_jump(c->label.get());
+        __ bind(c->else_state->label.get());
+        __ cache_state()->Steal(c->else_state->state);
+        __ MergeFullStackWith(c->label_state);
+        __ cache_state()->Steal(c->label_state);
+      } else {
+        // There is no merge at the end of the if, so just continue with the
+        // else state.
+        __ bind(c->else_state->label.get());
+        __ cache_state()->Steal(c->else_state->state);
+      }
+    } else if (!c->is_loop() && c->end_merge.reached) {
       __ cache_state()->Steal(c->label_state);
     }
     if (!c->label.get()->is_bound()) {
@@ -1310,10 +1327,17 @@ class LiftoffCompiler {
     DCHECK(!table_iterator.has_next());
   }
 
-  void Else(FullDecoder* decoder, Control* if_block) {
-    if (if_block->reachable()) __ emit_jump(if_block->label.get());
-    __ bind(if_block->else_state->label.get());
-    __ cache_state()->Steal(if_block->else_state->state);
+  void Else(FullDecoder* decoder, Control* c) {
+    if (c->reachable()) {
+      if (!c->end_merge.reached) {
+        c->label_state.InitMerge(*__ cache_state(), __ num_locals(),
+                                 c->end_merge.arity);
+      }
+      __ MergeFullStackWith(c->label_state);
+      __ emit_jump(c->label.get());
+    }
+    __ bind(c->else_state->label.get());
+    __ cache_state()->Steal(c->else_state->state);
   }
 
   Label* AddOutOfLineTrap(WasmCodePosition position,

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

@@ -1748,9 +1748,10 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               this->error(this->pc_, "else already present for if");
               break;
             }
-            FallThruTo(c);
+            if (!TypeCheckFallThru(c)) break;
             c->kind = kControlIfElse;
             CALL_INTERFACE_IF_PARENT_REACHABLE(Else, c);
+            if (c->reachable()) c->end_merge.reached = true;
             PushMergeValues(c, &c->start_merge);
             c->reachability = control_at(1)->innerReachability();
             break;
@@ -1758,7 +1759,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
           case kExprEnd: {
             if (!VALIDATE(!control_.empty())) {
               this->error("end does not match any if, try, or block");
-              return;
+              break;
             }
             Control* c = &control_.back();
             if (!VALIDATE(!c->is_incomplete_try())) {
@@ -1766,12 +1767,12 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               break;
             }
             if (c->is_onearmed_if()) {
-              // Emulate empty else arm.
-              FallThruTo(c);
-              if (this->failed()) break;
-              CALL_INTERFACE_IF_PARENT_REACHABLE(Else, c);
-              PushMergeValues(c, &c->start_merge);
-              c->reachability = control_at(1)->innerReachability();
+              if (!VALIDATE(c->end_merge.arity == c->start_merge.arity)) {
+                this->error(
+                    c->pc,
+                    "start-arity and end-arity of one-armed if must match");
+                break;
+              }
             }
             if (c->is_try_catch()) {
               // Emulate catch-all + re-throw.
@@ -2301,7 +2302,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   void PopControl(Control* c) {
     DCHECK_EQ(c, &control_.back());
     CALL_INTERFACE_IF_PARENT_REACHABLE(PopControl, c);
-    bool reached = c->end_merge.reached;
+    bool reached = c->end_merge.reached || c->is_onearmed_if();
     control_.pop_back();
     // If the parent block was reachable before, but the popped control does not
     // return to here, this block becomes indirectly unreachable.

src/wasm/graph-builder-interface.cc

@@ -200,6 +200,11 @@ class WasmGraphBuildingInterface {
   }
 
   void PopControl(FullDecoder* decoder, Control* block) {
+    if (block->is_onearmed_if()) {
+      // Merge the else branch into the end merge.
+      SetEnv(block->false_env);
+      MergeValuesInto(decoder, block, &block->end_merge);
+    }
     if (!block->is_loop()) SetEnv(block->end_env);
   }
 
@@ -338,6 +343,10 @@ class WasmGraphBuildingInterface {
   }
 
   void Else(FullDecoder* decoder, Control* if_block) {
+    if (if_block->reachable()) {
+      // Merge the if branch into the end merge.
+      MergeValuesInto(decoder, if_block, &if_block->end_merge);
+    }
     SetEnv(if_block->false_env);
   }
 

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -18,6 +18,7 @@
 #include "test/common/wasm/flag-utils.h"
 #include "test/common/wasm/test-signatures.h"
 #include "test/common/wasm/wasm-macro-gen.h"
+#include "testing/gmock-support.h"
 
 namespace v8 {
 namespace internal {
@@ -146,6 +147,9 @@ class FunctionBodyDecoderTest : public TestWithZone {
       str << "Verification successed, expected failure; pc = +" << pc;
     }
     EXPECT_EQ(result.ok(), expected_success) << str.str();
+    if (!expected_success && message) {
+      EXPECT_THAT(result.error_msg(), ::testing::HasSubstr(message));
+    }
   }
 
   void TestBinop(WasmOpcode opcode, FunctionSig* success) {
@@ -786,6 +790,13 @@ TEST_F(FunctionBodyDecoderTest, IfElseUnreachable2) {
   }
 }
 
+TEST_F(FunctionBodyDecoderTest, OneArmedIfWithArity) {
+  static const byte code[] = {WASM_ZERO, kExprIf, kLocalI32, WASM_ONE,
+                              kExprEnd};
+  EXPECT_FAILURE_C(i_v, code,
+                   "start-arity and end-arity of one-armed if must match");
+}
+
 TEST_F(FunctionBodyDecoderTest, IfBreak) {
   EXPECT_VERIFIES(v_i, WASM_IF(WASM_GET_LOCAL(0), WASM_BR(0)));
   EXPECT_VERIFIES(v_i, WASM_IF(WASM_GET_LOCAL(0), WASM_BR(1)));