[compiler] Fix a bug in BranchElimination

The condition can change between VisitBranch and VisitIf, so VisitIf
can't assume that the condition is not yet in the ControlPathConditions
list. Thanks Manos!

Change-Id: Ic74253b6faf2663cfa5212765d81392cb89d73b9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2773312Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73539}

src/compiler/branch-elimination.cc

@@ -323,8 +323,17 @@ Reduction BranchElimination::UpdateConditions(
 void BranchElimination::ControlPathConditions::AddCondition(
     Zone* zone, Node* condition, Node* branch, bool is_true,
     ControlPathConditions hint) {
-  DCHECK(!LookupCondition(condition, nullptr, nullptr));
-  PushFront({condition, branch, is_true}, zone, hint);
+  if (!LookupCondition(condition)) {
+    PushFront({condition, branch, is_true}, zone, hint);
+  }
+}
+
+bool BranchElimination::ControlPathConditions::LookupCondition(
+    Node* condition) const {
+  for (BranchCondition element : *this) {
+    if (element.condition == condition) return true;
+  }
+  return false;
 }
 
 bool BranchElimination::ControlPathConditions::LookupCondition(

src/compiler/branch-elimination.h

@@ -52,6 +52,7 @@ class V8_EXPORT_PRIVATE BranchElimination final
   // (true or false).
   class ControlPathConditions : public FunctionalList<BranchCondition> {
    public:
+    bool LookupCondition(Node* condition) const;
     bool LookupCondition(Node* condition, Node** branch, bool* is_true) const;
     void AddCondition(Zone* zone, Node* condition, Node* branch, bool is_true,
                       ControlPathConditions hint);

test/mjsunit/regress/wasm/condition-change-during-branch-elimination.js

+// Copyright 2021 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.
+
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+// This test creates the situation where BranchElimination's VisitIf sees a
+// different condition than the preceding VisitBranch (because an interleaved
+// CommonOperatorReducer replaced the condition).
+
+(function foo() {
+  let builder = new WasmModuleBuilder();
+
+  builder.addFunction("main", kSig_v_l)
+    .addLocals(kWasmI32, 2)
+    .addBody([
+      kExprLoop, kWasmStmt,
+        kExprLocalGet, 0x02,
+        kExprLocalTee, 0x01,
+        kExprIf, kWasmStmt,
+        kExprElse,
+          kExprLoop, kWasmStmt,
+            kExprLoop, kWasmStmt,
+              kExprLocalGet, 0x01,
+              kExprIf, kWasmStmt,
+              kExprElse,
+                kExprLocalGet, 0x02,
+                kExprBrIf, 0x04,
+                kExprBr, 0x01,
+                kExprEnd,
+              kExprLocalGet, 0x00,
+              kExprCallFunction, 0x01,
+              kExprLocalTee, 0x02,
+              kExprBrIf, 0x00,
+              kExprEnd,
+            kExprLocalGet, 0x01,
+            kExprBrIf, 0x00,
+            kExprEnd,
+          kExprEnd,
+        kExprBr, 0x00,
+        kExprEnd])
+    .exportAs("main");
+
+  builder.addFunction("callee", kSig_i_l)
+         .addBody([kExprLocalGet, 0, kExprI32ConvertI64]);
+
+  let module = new WebAssembly.Module(builder.toBuffer());
+  let instance = new WebAssembly.Instance(module);
+})();