[wasm] Put the condition last in kExprBrIf.

To bring V8 into line with the proposed design changes in:

 https://github.com/WebAssembly/design/pull/489

(This CL is forked from https://codereview.chromium.org/1634673002/.
That CL doesn't merge cleanly, and I can't update it.)

TBR=titzer@chromium.org
LOG=Y
BUG=chromium:575167

Review URL: https://codereview.chromium.org/1682443002

Cr-Commit-Position: refs/heads/master@{#33828}

src/wasm/ast-decoder.cc

@@ -998,17 +998,16 @@ class LR_WasmDecoder : public WasmDecoder {
         break;
       }
       case kExprBrIf: {
-        if (p->index == 1) {
+        if (p->done()) {
           TypeCheckLast(p, kAstI32);
-        } else if (p->done()) {
           BreakDepthOperand operand(this, p->pc());
           CHECK(Validate(p->pc(), operand, blocks_));
           SsaEnv* fenv = ssa_env_;
           SsaEnv* tenv = Split(fenv);
-          BUILD(Branch, p->tree->children[0]->node, &tenv->control,
+          BUILD(Branch, p->tree->children[1]->node, &tenv->control,
                 &fenv->control);
           ssa_env_ = tenv;
-          ReduceBreakToExprBlock(p, operand.target);
+          ReduceBreakToExprBlock(p, operand.target, p->tree->children[0]);
           ssa_env_ = fenv;
         }
         break;
@@ -1219,13 +1218,17 @@ class LR_WasmDecoder : public WasmDecoder {
   }
 
   void ReduceBreakToExprBlock(Production* p, Block* block) {
+    ReduceBreakToExprBlock(p, block, p->tree->count > 0 ? p->last() : nullptr);
+  }
+
+  void ReduceBreakToExprBlock(Production* p, Block* block, Tree* val) {
     if (block->stack_depth < 0) {
       // This is the inner loop block, which does not have a value.
       Goto(ssa_env_, block->ssa_env);
     } else {
       // Merge the value into the production for the block.
       Production* bp = &stack_[block->stack_depth];
-      MergeIntoProduction(bp, block->ssa_env, p->last());
+      MergeIntoProduction(bp, block->ssa_env, val);
     }
   }
 
@@ -1234,7 +1237,7 @@ class LR_WasmDecoder : public WasmDecoder {
 
     bool first = target->state == SsaEnv::kUnreachable;
     Goto(ssa_env_, target);
-    if (expr->type == kAstEnd) return;
+    if (expr == nullptr || expr->type == kAstEnd) return;
 
     if (first) {
       // first merge to this environment; set the type and the node.

src/wasm/wasm-macro-gen.h

@@ -22,10 +22,10 @@
 #define WASM_SELECT(cond, tval, fval) kExprSelect, cond, tval, fval
 #define WASM_BR(depth) kExprBr, static_cast<byte>(depth), kExprNop
 #define WASM_BR_IF(depth, cond) \
-  kExprBrIf, static_cast<byte>(depth), cond, kExprNop
+  kExprBrIf, static_cast<byte>(depth), kExprNop, cond
 #define WASM_BRV(depth, val) kExprBr, static_cast<byte>(depth), val
-#define WASM_BRV_IF(depth, cond, val) \
-  kExprBrIf, static_cast<byte>(depth), cond, val
+#define WASM_BRV_IF(depth, val, cond) \
+  kExprBrIf, static_cast<byte>(depth), val, cond
 #define WASM_BREAK(depth) kExprBr, static_cast<byte>(depth + 1), kExprNop
 #define WASM_CONTINUE(depth) kExprBr, static_cast<byte>(depth), kExprNop
 #define WASM_BREAKV(depth, val) kExprBr, static_cast<byte>(depth + 1), val

test/cctest/wasm/test-run-wasm.cc

@@ -1320,7 +1320,7 @@ TEST(Run_Wasm_Block_If_P) {
 
 TEST(Run_Wasm_Block_BrIf_P) {
   WasmRunner<int32_t> r(MachineType::Int32());
-  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_I8(51)),
+  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_I8(51), WASM_GET_LOCAL(0)),
                       WASM_I8(52)));
   FOR_INT32_INPUTS(i) {
     int32_t expected = *i ? 51 : 52;
@@ -2054,7 +2054,7 @@ TEST(Run_Wasm_Infinite_Loop_not_taken2) {
 
 TEST(Run_Wasm_Infinite_Loop_not_taken2_brif) {
   WasmRunner<int32_t> r(MachineType::Int32());
-  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_I8(45)),
+  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_I8(45), WASM_GET_LOCAL(0)),
                       WASM_INFINITE_LOOP));
   // Run the code, but don't go into the infinite loop.
   CHECK_EQ(45, r.Call(1));
@@ -2725,7 +2725,7 @@ TEST(Run_Wasm_ExprBlock2b) {
 
 TEST(Run_Wasm_ExprBlock2c) {
   WasmRunner<int32_t> r(MachineType::Int32());
-  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_I8(1)),
+  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_I8(1), WASM_GET_LOCAL(0)),
                       WASM_I8(1)));
   CHECK_EQ(1, r.Call(0));
   CHECK_EQ(1, r.Call(1));
@@ -2734,7 +2734,7 @@ TEST(Run_Wasm_ExprBlock2c) {
 
 TEST(Run_Wasm_ExprBlock2d) {
   WasmRunner<int32_t> r(MachineType::Int32());
-  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_I8(1)),
+  BUILD(r, WASM_BLOCK(2, WASM_BRV_IF(0, WASM_I8(1), WASM_GET_LOCAL(0)),
                       WASM_I8(2)));
   CHECK_EQ(2, r.Call(0));
   CHECK_EQ(1, r.Call(1));
@@ -2767,16 +2767,16 @@ TEST(Run_Wasm_ExprBlock_ManualSwitch) {
 TEST(Run_Wasm_ExprBlock_ManualSwitch_brif) {
   WasmRunner<int32_t> r(MachineType::Int32());
   BUILD(r,
-        WASM_BLOCK(6, WASM_BRV_IF(0, WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(1)),
-                                  WASM_I8(11)),
-                   WASM_BRV_IF(0, WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(2)),
-                               WASM_I8(12)),
-                   WASM_BRV_IF(0, WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(3)),
-                               WASM_I8(13)),
-                   WASM_BRV_IF(0, WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(4)),
-                               WASM_I8(14)),
-                   WASM_BRV_IF(0, WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(5)),
-                               WASM_I8(15)),
+        WASM_BLOCK(6, WASM_BRV_IF(0, WASM_I8(11),
+                                  WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(1))),
+                   WASM_BRV_IF(0, WASM_I8(12),
+                               WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(2))),
+                   WASM_BRV_IF(0, WASM_I8(13),
+                               WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(3))),
+                   WASM_BRV_IF(0, WASM_I8(14),
+                               WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(4))),
+                   WASM_BRV_IF(0, WASM_I8(15),
+                               WASM_I32_EQ(WASM_GET_LOCAL(0), WASM_I8(5))),
                    WASM_I8(99)));
   CHECK_EQ(99, r.Call(0));
   CHECK_EQ(11, r.Call(1));

test/unittests/wasm/ast-decoder-unittest.cc

@@ -35,6 +35,8 @@ static const WasmOpcode kInt32BinopOpcodes[] = {
     kExprI32Shl,  kExprI32ShrU, kExprI32ShrS, kExprI32Eq,   kExprI32LtS,
     kExprI32LeS,  kExprI32LtU,  kExprI32LeU};
 
+#define WASM_BRV_IF_ZERO(depth, val) \
+  kExprBrIf, static_cast<byte>(depth), val, WASM_ZERO
 
 #define EXPECT_VERIFIES(env, x) Verify(kSuccess, env, x, x + arraysize(x))
 
@@ -1550,19 +1552,18 @@ TEST_F(WasmDecoderTest, BreakNesting3) {
 
 TEST_F(WasmDecoderTest, BreaksWithMultipleTypes) {
   EXPECT_FAILURE_INLINE(
-      &env_i_i,
-      WASM_BLOCK(2, WASM_BRV_IF(0, WASM_ZERO, WASM_I8(7)), WASM_F32(7.7)));
-  EXPECT_FAILURE_INLINE(&env_i_i,
-                        WASM_BLOCK(2, WASM_BRV_IF(0, WASM_ZERO, WASM_I8(7)),
-                                   WASM_BRV_IF(0, WASM_ZERO, WASM_F32(7.7))));
+      &env_i_i, WASM_BLOCK(2, WASM_BRV_IF_ZERO(0, WASM_I8(7)), WASM_F32(7.7)));
+
   EXPECT_FAILURE_INLINE(&env_i_i,
-                        WASM_BLOCK(3, WASM_BRV_IF(0, WASM_ZERO, WASM_I8(8)),
-                                   WASM_BRV_IF(0, WASM_ZERO, WASM_I8(0)),
-                                   WASM_BRV_IF(0, WASM_ZERO, WASM_F32(7.7))));
+                        WASM_BLOCK(2, WASM_BRV_IF_ZERO(0, WASM_I8(7)),
+                                   WASM_BRV_IF_ZERO(0, WASM_F32(7.7))));
   EXPECT_FAILURE_INLINE(&env_i_i,
-                        WASM_BLOCK(3, WASM_BRV_IF(0, WASM_ZERO, WASM_I8(9)),
-                                   WASM_BRV_IF(0, WASM_ZERO, WASM_F32(7.7)),
-                                   WASM_BRV_IF(0, WASM_ZERO, WASM_I8(11))));
+                        WASM_BLOCK(3, WASM_BRV_IF_ZERO(0, WASM_I8(8)),
+                                   WASM_BRV_IF_ZERO(0, WASM_I8(0)),
+                                   WASM_BRV_IF_ZERO(0, WASM_F32(7.7))));
+  EXPECT_FAILURE_INLINE(&env_i_i, WASM_BLOCK(3, WASM_BRV_IF_ZERO(0, WASM_I8(9)),
+                                             WASM_BRV_IF_ZERO(0, WASM_F32(7.7)),
+                                             WASM_BRV_IF_ZERO(0, WASM_I8(11))));
 }
 
 
@@ -1625,11 +1626,10 @@ TEST_F(WasmDecoderTest, ExprBreak_TypeCheckAll) {
   byte code1[] = {WASM_BLOCK(2,
                              WASM_IF(WASM_ZERO, WASM_BRV(0, WASM_GET_LOCAL(0))),
                              WASM_GET_LOCAL(1))};
-  byte code2[] = {WASM_BLOCK(
-      2, WASM_IF(WASM_ZERO, WASM_BRV_IF(0, WASM_ZERO, WASM_GET_LOCAL(0))),
+  byte code2[] = {
+      WASM_BLOCK(2, WASM_IF(WASM_ZERO, WASM_BRV_IF_ZERO(0, WASM_GET_LOCAL(0))),
                  WASM_GET_LOCAL(1))};
 
-
   for (size_t i = 0; i < arraysize(kLocalTypes); i++) {
     for (size_t j = 0; j < arraysize(kLocalTypes); j++) {
       FunctionEnv env;
@@ -1678,37 +1678,42 @@ TEST_F(WasmDecoderTest, ExprBr_Unify) {
   }
 }
 
-
-TEST_F(WasmDecoderTest, ExprBrIf_type) {
-  EXPECT_VERIFIES_INLINE(
-      &env_i_i,
-      WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(0)),
-                 WASM_GET_LOCAL(0)));
-  EXPECT_FAILURE_INLINE(
-      &env_d_dd,
-      WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(0)),
-                 WASM_GET_LOCAL(0)));
-
+TEST_F(WasmDecoderTest, ExprBrIf_cond_type) {
   FunctionEnv env;
+  byte code[] = {
+      WASM_BLOCK(1, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1)))};
   for (size_t i = 0; i < arraysize(kLocalTypes); i++) {
-    LocalType type = kLocalTypes[i];
-    LocalType storage[] = {kAstI32, kAstI32, type};
-    FunctionSig sig(1, 2, storage);
-    init_env(&env, &sig);  // (i32, X) -> i32
+    for (size_t j = 0; j < arraysize(kLocalTypes); j++) {
+      LocalType types[] = {kLocalTypes[i], kLocalTypes[j]};
+      FunctionSig sig(0, 2, types);
+      init_env(&env, &sig);
 
-    byte code1[] = {
-        WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1)),
-                   WASM_GET_LOCAL(0))};
+      if (types[1] == kAstI32) {
+        EXPECT_VERIFIES(&env, code);
+      } else {
+        EXPECT_FAILURE(&env, code);
+      }
+    }
+  }
+}
 
-    byte code2[] = {
-        WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(1), WASM_GET_LOCAL(0)),
+TEST_F(WasmDecoderTest, ExprBrIf_val_type) {
+  FunctionEnv env;
+  byte code[] = {
+      WASM_BLOCK(2, WASM_BRV_IF(0, WASM_GET_LOCAL(1), WASM_GET_LOCAL(2)),
                  WASM_GET_LOCAL(0))};
-    if (type == kAstI32) {
-      EXPECT_VERIFIES(&env, code1);
-      EXPECT_VERIFIES(&env, code2);
+  for (size_t i = 0; i < arraysize(kLocalTypes); i++) {
+    for (size_t j = 0; j < arraysize(kLocalTypes); j++) {
+      LocalType types[] = {kLocalTypes[i], kLocalTypes[i], kLocalTypes[j],
+                           kAstI32};
+      FunctionSig sig(1, 3, types);
+      init_env(&env, &sig);
+
+      if (i == j) {
+        EXPECT_VERIFIES(&env, code);
       } else {
-      EXPECT_FAILURE(&env, code1);
-      EXPECT_FAILURE(&env, code2);
+        EXPECT_FAILURE(&env, code);
+      }
     }
   }
 }
@@ -1724,14 +1729,11 @@ TEST_F(WasmDecoderTest, ExprBrIf_Unify) {
       FunctionSig sig(1, 2, storage);
       init_env(&env, &sig);  // (i32, X) -> i32
 
-      byte code1[] = {
-          WASM_BLOCK(2, WASM_BRV_IF(0, WASM_ZERO, WASM_GET_LOCAL(which)),
+      byte code1[] = {WASM_BLOCK(2, WASM_BRV_IF_ZERO(0, WASM_GET_LOCAL(which)),
                                  WASM_GET_LOCAL(which ^ 1))};
-      byte code2[] = {
-          WASM_LOOP(2, WASM_BRV_IF(1, WASM_ZERO, WASM_GET_LOCAL(which)),
+      byte code2[] = {WASM_LOOP(2, WASM_BRV_IF_ZERO(1, WASM_GET_LOCAL(which)),
                                 WASM_GET_LOCAL(which ^ 1))};
 
-
       if (type == kAstI32) {
         EXPECT_VERIFIES(&env, code1);
         EXPECT_VERIFIES(&env, code2);