[turbofan] fix escape analysis bug: revisit phis

Bug: chromium:974476
Change-Id: I719812e93345b5f7aa9b1e4e594d02ae9a1c4208
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1664063Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62239}

src/compiler/escape-analysis.cc

@@ -441,10 +441,12 @@ VariableTracker::State VariableTracker::MergeInputs(Node* effect_phi) {
         // [old_value] cannot originate from the inputs. Thus [old_value]
         // must have been created by a previous reduction of this [effect_phi].
         for (int i = 0; i < arity; ++i) {
-          NodeProperties::ReplaceValueInput(
-              old_value, buffer_[i] ? buffer_[i] : graph_->Dead(), i);
-          // This change cannot affect the rest of the reducer, so there is no
-          // need to trigger additional revisitations.
+          Node* old_input = NodeProperties::GetValueInput(old_value, i);
+          Node* new_input = buffer_[i] ? buffer_[i] : graph_->Dead();
+          if (old_input != new_input) {
+            NodeProperties::ReplaceValueInput(old_value, new_input, i);
+            reducer_->Revisit(old_value);
+          }
         }
         result.Set(var, old_value);
       } else {
@@ -701,21 +703,19 @@ void ReduceNode(const Operator* op, EscapeAnalysisTracker::Scope* current,
       } else if (right_object && !right_object->HasEscaped()) {
         replacement = jsgraph->FalseConstant();
       }
-      if (replacement) {
-        // TODO(tebbi) This is a workaround for uninhabited types. If we
-        // replaced a value of uninhabited type with a constant, we would
-        // widen the type of the node. This could produce inconsistent
-        // types (which might confuse representation selection). We get
-        // around this by refusing to constant-fold and escape-analyze
-        // if the type is not inhabited.
-        if (!NodeProperties::GetType(left).IsNone() &&
-            !NodeProperties::GetType(right).IsNone()) {
-          current->SetReplacement(replacement);
-        } else {
-          current->SetEscaped(left);
-          current->SetEscaped(right);
-        }
+      // TODO(tebbi) This is a workaround for uninhabited types. If we
+      // replaced a value of uninhabited type with a constant, we would
+      // widen the type of the node. This could produce inconsistent
+      // types (which might confuse representation selection). We get
+      // around this by refusing to constant-fold and escape-analyze
+      // if the type is not inhabited.
+      if (replacement && !NodeProperties::GetType(left).IsNone() &&
+          !NodeProperties::GetType(right).IsNone()) {
+        current->SetReplacement(replacement);
+        break;
       }
+      current->SetEscaped(left);
+      current->SetEscaped(right);
       break;
     }
     case IrOpcode::kCheckMaps: {

test/mjsunit/compiler/regress-crbug-974476.js

+// Copyright 2019 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: --allow-natives-syntax
+
+function use(x) { return x; }
+%NeverOptimizeFunction(use);
+
+function foo() {
+  let result = undefined;
+  (function () {
+    const a = {};
+    for (_ of [0]) {
+      const empty = [];
+      (function () {
+        result = 42;
+        for (_ of [0]) {
+          for (_ of [0]) {
+            use(empty);
+          }
+        }
+        result = a;
+      })();
+    }
+  })();
+  return result;
+}
+
+%PrepareFunctionForOptimization(foo);
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();