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

This reverts commit c2aaf0a6.

Reason for revert: Benchmarks fail, and ClusterFuzz is not happy (issue 911406, issue 911271)

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}

TBR=titzer@chromium.org,clemensh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

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

src/wasm/baseline/liftoff-compiler.cc

@@ -498,10 +498,8 @@ class LiftoffCompiler {
     if (c->end_merge.reached) {
       __ MergeFullStackWith(c->label_state);
     } else if (c->is_onearmed_if()) {
-      // 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);
+      c->label_state.InitMerge(*__ cache_state(), __ num_locals(),
+                               c->br_merge()->arity);
       __ MergeFullStackWith(c->label_state);
     } else {
       c->label_state.Split(*__ cache_state());
@@ -510,20 +508,7 @@ class LiftoffCompiler {
   }
 
   void PopControl(FullDecoder* decoder, Control* c) {
-    if (c->is_onearmed_if()) {
-      __ bind(c->else_state->label.get());
-      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.
-        __ 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.
-        __ cache_state()->Steal(c->else_state->state);
-      }
-    } else if (!c->is_loop() && c->end_merge.reached) {
+    if (!c->is_loop() && c->end_merge.reached) {
       __ cache_state()->Steal(c->label_state);
     }
     if (!c->label.get()->is_bound()) {
@@ -1325,17 +1310,10 @@ class LiftoffCompiler {
     DCHECK(!table_iterator.has_next());
   }
 
-  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);
+  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);
   }
 
   Label* AddOutOfLineTrap(WasmCodePosition position,

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

@@ -1750,10 +1750,9 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               this->error(this->pc_, "else already present for if");
               break;
             }
-            if (!TypeCheckFallThru(c)) break;
+            FallThruTo(c);
             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;
@@ -1761,7 +1760,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
           case kExprEnd: {
             if (!VALIDATE(!control_.empty())) {
               this->error("end does not match any if, try, or block");
-              break;
+              return;
             }
             Control* c = &control_.back();
             if (!VALIDATE(!c->is_incomplete_try())) {
@@ -1769,12 +1768,12 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               break;
             }
             if (c->is_onearmed_if()) {
-              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;
-              }
+              // 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 (c->is_try_catch()) {
               // Emulate catch-all + re-throw.
@@ -2304,7 +2303,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 || c->is_onearmed_if();
+    bool reached = c->end_merge.reached;
     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,11 +200,6 @@ 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);
   }
 
@@ -343,10 +338,6 @@ 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,7 +18,6 @@
 #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 {
@@ -147,9 +146,6 @@ 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) {
@@ -790,13 +786,6 @@ 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)));