Commit b8b60750 authored by Mike Stanton's avatar Mike Stanton Committed by Commit Bot

[TurboFan] Loop variable analysis requires more sensitivity

Loop variable analysis doesn't recognize that the initial type of the
loop variable phi combined with the increment type may produce a NaN
result through the addition of two infinities of differing sign.

This leads to unreachable code and a SIGINT crash.

The fix is to consider this case before typing the loop variable phi,
falling back to more conservative typing if discovered.

R=neis@chromium.org

Bug: chromium:1028863
Change-Id: Ic4b5189c4c50c5bbe29e46050de630fd0673de9f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1946352
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65291}
parent 0da7ca87
...@@ -94,7 +94,8 @@ Reduction GraphReducer::Reduce(Node* const node) { ...@@ -94,7 +94,8 @@ Reduction GraphReducer::Reduce(Node* const node) {
// all the other reducers for this node, as now there may be more // all the other reducers for this node, as now there may be more
// opportunities for reduction. // opportunities for reduction.
if (FLAG_trace_turbo_reduction) { if (FLAG_trace_turbo_reduction) {
StdoutStream{} << "- In-place update of " << *node << " by reducer " AllowHandleDereference allow_deref;
StdoutStream{} << "- In-place update of #" << *node << " by reducer "
<< (*i)->reducer_name() << std::endl; << (*i)->reducer_name() << std::endl;
} }
skip = i; skip = i;
...@@ -103,7 +104,8 @@ Reduction GraphReducer::Reduce(Node* const node) { ...@@ -103,7 +104,8 @@ Reduction GraphReducer::Reduce(Node* const node) {
} else { } else {
// {node} was replaced by another node. // {node} was replaced by another node.
if (FLAG_trace_turbo_reduction) { if (FLAG_trace_turbo_reduction) {
StdoutStream{} << "- Replacement of " << *node << " with " AllowHandleDereference allow_deref;
StdoutStream{} << "- Replacement of #" << *node << " with #"
<< *(reduction.replacement()) << " by reducer " << *(reduction.replacement()) << " by reducer "
<< (*i)->reducer_name() << std::endl; << (*i)->reducer_name() << std::endl;
} }
......
...@@ -847,13 +847,30 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) { ...@@ -847,13 +847,30 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) {
DCHECK_EQ(IrOpcode::kLoop, NodeProperties::GetControlInput(node)->opcode()); DCHECK_EQ(IrOpcode::kLoop, NodeProperties::GetControlInput(node)->opcode());
DCHECK_EQ(2, NodeProperties::GetControlInput(node)->InputCount()); 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 initial_type = Operand(node, 0);
Type increment_type = Operand(node, 2); 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());
}
// We only handle integer induction variables (otherwise ranges // We only handle integer induction variables (otherwise ranges
// do not apply and we cannot do anything). // do not apply and we cannot do anything).
if (!initial_type.Is(typer_->cache_->kInteger) || if (!both_types_integer || maybe_nan) {
!increment_type.Is(typer_->cache_->kInteger)) {
// Fallback to normal phi typing, but ensure monotonicity. // Fallback to normal phi typing, but ensure monotonicity.
// (Unfortunately, without baking in the previous type, monotonicity might // (Unfortunately, without baking in the previous type, monotonicity might
// be violated because we might not yet have retyped the incrementing // be violated because we might not yet have retyped the incrementing
...@@ -874,12 +891,6 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) { ...@@ -874,12 +891,6 @@ Type Typer::Visitor::TypeInductionVariablePhi(Node* node) {
} }
// Now process the bounds. // 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 min = -V8_INFINITY;
double max = V8_INFINITY; double max = V8_INFINITY;
......
// 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 write(begin, end, step) {
for (var i = begin; i >= end; i += step) {
step = end - begin;
begin >>>= 805306382;
}
}
function bar() {
for (let i = 0; i < 10000; i++) {
write(Infinity, 1, 1);
}
}
%PrepareFunctionForOptimization(write);
%PrepareFunctionForOptimization(bar);
bar();
%OptimizeFunctionOnNextCall(bar);
bar();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment