[turbofan] Fix bug in Typer::TypeInductionVariablePhi

The fix in b8b60750 was insufficient.

The bug is that induction variable typing does not take into account
that the value can become NaN through addition or subtraction of
Infinities. The previous fix incorrectly assumed that this can only
happen when the initial value of the loop variable is an Infinity.

Bug: chromium:1051017
Change-Id: I8c9ffb2925288b80c00e18e7bc22a556bf540733
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2051957
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66258}

src/compiler/typer.cc

@@ -847,30 +847,24 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) {
   DCHECK_EQ(IrOpcode::kLoop, NodeProperties::GetControlInput(node)->opcode());
   DCHECK_EQ(2, NodeProperties::GetControlInput(node)->InputCount());
 
-  auto res = induction_vars_->induction_variables().find(node->id());
-  DCHECK(res != induction_vars_->induction_variables().end());
-  InductionVariable* induction_var = res->second;
-  InductionVariable::ArithmeticType arithmetic_type = induction_var->Type();
   Type initial_type = Operand(node, 0);
   Type increment_type = Operand(node, 2);
 
-  const bool both_types_integer = initial_type.Is(typer_->cache_->kInteger) &&
-                                  increment_type.Is(typer_->cache_->kInteger);
-  bool maybe_nan = false;
-  // The addition or subtraction could still produce a NaN, if the integer
-  // ranges touch infinity.
-  if (both_types_integer) {
-    Type resultant_type =
-        (arithmetic_type == InductionVariable::ArithmeticType::kAddition)
-            ? typer_->operation_typer()->NumberAdd(initial_type, increment_type)
-            : typer_->operation_typer()->NumberSubtract(initial_type,
-                                                        increment_type);
-    maybe_nan = resultant_type.Maybe(Type::NaN());
+  // If we do not have enough type information for the initial value or
+  // the increment, just return the initial value's type.
+  if (initial_type.IsNone() ||
+      increment_type.Is(typer_->cache_->kSingletonZero)) {
+    return initial_type;
   }
 
-  // We only handle integer induction variables (otherwise ranges
-  // do not apply and we cannot do anything).
-  if (!both_types_integer || maybe_nan) {
+  // We only handle integer induction variables (otherwise ranges do not apply
+  // and we cannot do anything). Moreover, we don't support infinities in
+  // {increment_type} because the induction variable can become NaN through
+  // addition/subtraction of opposing infinities.
+  if (!initial_type.Is(typer_->cache_->kInteger) ||
+      !increment_type.Is(typer_->cache_->kInteger) ||
+      increment_type.Min() == -V8_INFINITY ||
+      increment_type.Max() == +V8_INFINITY) {
     // Fallback to normal phi typing, but ensure monotonicity.
     // (Unfortunately, without baking in the previous type, monotonicity might
     // be violated because we might not yet have retyped the incrementing
@@ -883,14 +877,13 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) {
     }
     return type;
   }
-  // If we do not have enough type information for the initial value or
-  // the increment, just return the initial value's type.
-  if (initial_type.IsNone() ||
-      increment_type.Is(typer_->cache_->kSingletonZero)) {
-    return initial_type;
-  }
 
   // Now process the bounds.
+  auto res = induction_vars_->induction_variables().find(node->id());
+  DCHECK(res != induction_vars_->induction_variables().end());
+  InductionVariable* induction_var = res->second;
+  InductionVariable::ArithmeticType arithmetic_type = induction_var->Type();
+
   double min = -V8_INFINITY;
   double max = V8_INFINITY;
 
@@ -946,8 +939,8 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) {
     // The lower bound must be at most the initial value's lower bound.
     min = std::min(min, initial_type.Min());
   } else {
-    // Shortcut: If the increment can be both positive and negative,
-    // the variable can go arbitrarily far, so just return integer.
+    // If the increment can be both positive and negative, the variable can go
+    // arbitrarily far.
     return typer_->cache_->kInteger;
   }
   if (FLAG_trace_turbo_loop) {

test/mjsunit/compiler/regress-1051017.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: --allow-natives-syntax
+
+
+function foo1() {
+  var x = -Infinity;
+  var i = 0;
+  for (; i < 1; i += x) {
+    if (i == -Infinity) x = +Infinity;
+  }
+  return i;
+}
+
+%PrepareFunctionForOptimization(foo1);
+assertEquals(NaN, foo1());
+assertEquals(NaN, foo1());
+%OptimizeFunctionOnNextCall(foo1);
+assertEquals(NaN, foo1());
+
+
+function foo2() {
+  var i = -Infinity;
+  for (; i <= 42; i += Infinity) { }
+  return i;
+}
+
+%PrepareFunctionForOptimization(foo2);
+assertEquals(NaN, foo2());
+assertEquals(NaN, foo2());
+%OptimizeFunctionOnNextCall(foo2);
+assertEquals(NaN, foo2());