[turbofan] Do more checks for dead nodes in BranchElimination

Bug: chromium:1109174
Change-Id: I25924afe9ad9c147e7f89299983032c82f74626d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2320668
Auto-Submit: Georg Neis <neis@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69106}

src/compiler/branch-elimination.cc

@@ -130,18 +130,7 @@ Reduction BranchElimination::ReduceBranch(Node* node) {
   bool condition_value;
   // If we know the condition we can discard the branch.
   if (from_input.LookupCondition(condition, &branch, &condition_value)) {
-    // Mark the branch as a safety check if necessary.
-    // Check if {branch} is dead because we might have a stale side-table entry.
-    if (!branch->IsDead() && branch->opcode() != IrOpcode::kDead) {
-      IsSafetyCheck branch_safety = IsSafetyCheckOf(branch->op());
-      IsSafetyCheck combined_safety =
-          CombineSafetyChecks(branch_safety, IsSafetyCheckOf(node->op()));
-      if (branch_safety != combined_safety) {
-        NodeProperties::ChangeOp(
-            branch, common()->MarkAsSafetyCheck(branch->op(), combined_safety));
-      }
-    }
-
+    MarkAsSafetyCheckIfNeeded(branch, node);
     for (Node* const use : node->uses()) {
       switch (use->opcode()) {
         case IrOpcode::kIfTrue:
@@ -184,17 +173,9 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
   ControlPathConditions conditions = node_conditions_.Get(control);
   bool condition_value;
   Node* branch;
+  // If we know the condition we can discard the branch.
   if (conditions.LookupCondition(condition, &branch, &condition_value)) {
-    // Mark the branch as a safety check.
-    IsSafetyCheck branch_safety = IsSafetyCheckOf(branch->op());
-    IsSafetyCheck combined_safety =
-        CombineSafetyChecks(branch_safety, p.is_safety_check());
-    if (branch_safety != combined_safety) {
-      NodeProperties::ChangeOp(
-          branch, common()->MarkAsSafetyCheck(branch->op(), combined_safety));
-    }
-
-    // If we know the condition we can discard the branch.
+    MarkAsSafetyCheckIfNeeded(branch, node);
     if (condition_is_true == condition_value) {
       // We don't update the conditions here, because we're replacing {node}
       // with the {control} node that already contains the right information.
@@ -303,7 +284,7 @@ Reduction BranchElimination::UpdateConditions(
 void BranchElimination::ControlPathConditions::AddCondition(
     Zone* zone, Node* condition, Node* branch, bool is_true,
     ControlPathConditions hint) {
-  DCHECK_EQ(false, LookupCondition(condition, nullptr, nullptr));
+  DCHECK(!LookupCondition(condition, nullptr, nullptr));
   PushFront({condition, branch, is_true}, zone, hint);
 }
 
@@ -319,6 +300,19 @@ bool BranchElimination::ControlPathConditions::LookupCondition(
   return false;
 }
 
+void BranchElimination::MarkAsSafetyCheckIfNeeded(Node* branch, Node* node) {
+  // Check if {branch} is dead because we might have a stale side-table entry.
+  if (!branch->IsDead() && branch->opcode() != IrOpcode::kDead) {
+    IsSafetyCheck branch_safety = IsSafetyCheckOf(branch->op());
+    IsSafetyCheck combined_safety =
+        CombineSafetyChecks(branch_safety, IsSafetyCheckOf(node->op()));
+    if (branch_safety != combined_safety) {
+      NodeProperties::ChangeOp(
+          branch, common()->MarkAsSafetyCheck(branch->op(), combined_safety));
+    }
+  }
+}
+
 Graph* BranchElimination::graph() const { return jsgraph()->graph(); }
 
 Isolate* BranchElimination::isolate() const { return jsgraph()->isolate(); }

src/compiler/branch-elimination.h

@@ -74,6 +74,7 @@ class V8_EXPORT_PRIVATE BranchElimination final
   Reduction UpdateConditions(Node* node, ControlPathConditions prev_conditions,
                              Node* current_condition, Node* current_branch,
                              bool is_true_branch);
+  void MarkAsSafetyCheckIfNeeded(Node* branch, Node* node);
 
   Node* dead() const { return dead_; }
   Graph* graph() const;

test/mjsunit/compiler/regress-1109174.js

+// Copyright 2020 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.
+
+// Flags: --interrupt-budget=1024
+
+(function() {
+  let n = 0;
+  do {
+    for (let i = 0; i < 100; i++) {
+      function unused() {}
+      let a = [i];
+      let b = String.fromCodePoint(i, 1).padStart();
+      a.reduce(parseInt, b);
+    }
+    for (let j = 0; j < 1; j++) {}
+  } while (++n < 2);
+})();