From 9fdacb9e554e2b1ba235d9bb8e7f5cd8ee653883 Mon Sep 17 00:00:00 2001 From: jarin Date: Mon, 4 Jul 2016 01:42:24 -0700 Subject: [PATCH] [turbofan] Better handling of empty type in simplified lowering. The re-typer now only types a node if its inputs are all typed with the exception of phi nodes. This works because all cycles in the graph have to contain a phi node. BUG=chromium:625558 Review-Url: https://codereview.chromium.org/2120243002 Cr-Commit-Position: refs/heads/master@{#37493} --- src/compiler/operation-typer.cc | 27 ++++++++++++++ src/compiler/simplified-lowering.cc | 47 +++++++++++-------------- test/mjsunit/compiler/regress-625558.js | 14 ++++++++ test/mjsunit/mjsunit.status | 8 +---- 4 files changed, 63 insertions(+), 33 deletions(-) create mode 100644 test/mjsunit/compiler/regress-625558.js diff --git a/src/compiler/operation-typer.cc b/src/compiler/operation-typer.cc index b2860e00eb..514ba085c3 100644 --- a/src/compiler/operation-typer.cc +++ b/src/compiler/operation-typer.cc @@ -289,6 +289,10 @@ Type* OperationTyper::NumericAdd(Type* lhs, Type* rhs) { DCHECK(lhs->Is(Type::Number())); DCHECK(rhs->Is(Type::Number())); + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + // We can give more precise types for integers. if (!lhs->Is(cache_.kIntegerOrMinusZeroOrNaN) || !rhs->Is(cache_.kIntegerOrMinusZeroOrNaN)) { @@ -311,6 +315,10 @@ Type* OperationTyper::NumericSubtract(Type* lhs, Type* rhs) { DCHECK(lhs->Is(Type::Number())); DCHECK(rhs->Is(Type::Number())); + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + lhs = Rangify(lhs); rhs = Rangify(rhs); if (lhs->Is(Type::NaN()) || rhs->Is(Type::NaN())) return Type::NaN(); @@ -324,6 +332,11 @@ Type* OperationTyper::NumericSubtract(Type* lhs, Type* rhs) { Type* OperationTyper::NumericMultiply(Type* lhs, Type* rhs) { DCHECK(lhs->Is(Type::Number())); DCHECK(rhs->Is(Type::Number())); + + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + lhs = Rangify(lhs); rhs = Rangify(rhs); if (lhs->Is(Type::NaN()) || rhs->Is(Type::NaN())) return Type::NaN(); @@ -337,6 +350,10 @@ Type* OperationTyper::NumericDivide(Type* lhs, Type* rhs) { DCHECK(lhs->Is(Type::Number())); DCHECK(rhs->Is(Type::Number())); + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + if (lhs->Is(Type::NaN()) || rhs->Is(Type::NaN())) return Type::NaN(); // Division is tricky, so all we do is try ruling out nan. bool maybe_nan = @@ -349,6 +366,11 @@ Type* OperationTyper::NumericDivide(Type* lhs, Type* rhs) { Type* OperationTyper::NumericModulus(Type* lhs, Type* rhs) { DCHECK(lhs->Is(Type::Number())); DCHECK(rhs->Is(Type::Number())); + + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + if (lhs->Is(Type::NaN()) || rhs->Is(Type::NaN())) return Type::NaN(); if (lhs->Maybe(Type::NaN()) || rhs->Maybe(cache_.kZeroish) || @@ -403,6 +425,11 @@ Type* OperationTyper::FalsifyUndefined(ComparisonOutcome outcome) { Type* OperationTyper::TypeJSAdd(Type* lhs, Type* rhs) { lhs = ToPrimitive(lhs); rhs = ToPrimitive(rhs); + + if (!lhs->IsInhabited() || !rhs->IsInhabited()) { + return Type::None(); + } + if (lhs->Maybe(Type::String()) || rhs->Maybe(Type::String())) { if (lhs->Is(Type::String()) || rhs->Is(Type::String())) { return Type::String(); diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index df482e377b..9b5b73f205 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -357,17 +357,25 @@ class RepresentationSelector { Type* type = info->feedback_type(); Type* new_type = type; + // For any non-phi node just wait until we get all inputs typed. We only + // allow untyped inputs for phi nodes because phis are the only places + // where cycles need to be broken. + if (node->opcode() != IrOpcode::kPhi) { + for (int i = 0; i < node->op()->ValueInputCount(); i++) { + if (GetInfo(node->InputAt(i))->feedback_type() == nullptr) { + return false; + } + } + } + switch (node->opcode()) { case IrOpcode::kSpeculativeNumberAdd: { - Type* lhs = FeedbackTypeOf(node->InputAt(0)); - Type* rhs = FeedbackTypeOf(node->InputAt(1)); - if (lhs->Is(Type::None()) || rhs->Is(Type::None())) return false; // TODO(jarin) The ToNumber conversion is too conservative here, // e.g. it will treat true as 1 even though the number check will // fail on a boolean. OperationTyper should have a function that // computes a more precise type. - lhs = op_typer_.ToNumber(lhs); - rhs = op_typer_.ToNumber(rhs); + Type* lhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0))); + Type* rhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(1))); Type* static_type = op_typer_.NumericAdd(lhs, rhs); if (info->type_check() == TypeCheckKind::kNone) { new_type = static_type; @@ -379,15 +387,12 @@ class RepresentationSelector { } case IrOpcode::kSpeculativeNumberSubtract: { - Type* lhs = FeedbackTypeOf(node->InputAt(0)); - Type* rhs = FeedbackTypeOf(node->InputAt(1)); - if (lhs->Is(Type::None()) || rhs->Is(Type::None())) return false; // TODO(jarin) The ToNumber conversion is too conservative here, // e.g. it will treat true as 1 even though the number check will // fail on a boolean. OperationTyper should have a function that // computes a more precise type. - lhs = op_typer_.ToNumber(lhs); - rhs = op_typer_.ToNumber(rhs); + Type* lhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0))); + Type* rhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(1))); Type* static_type = op_typer_.NumericSubtract(lhs, rhs); if (info->type_check() == TypeCheckKind::kNone) { new_type = static_type; @@ -399,15 +404,12 @@ class RepresentationSelector { } case IrOpcode::kSpeculativeNumberMultiply: { - Type* lhs = FeedbackTypeOf(node->InputAt(0)); - Type* rhs = FeedbackTypeOf(node->InputAt(1)); - if (lhs->Is(Type::None()) || rhs->Is(Type::None())) return false; // TODO(jarin) The ToNumber conversion is too conservative here, // e.g. it will treat true as 1 even though the number check will // fail on a boolean. OperationTyper should have a function that // computes a more precise type. - lhs = op_typer_.ToNumber(lhs); - rhs = op_typer_.ToNumber(rhs); + Type* lhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0))); + Type* rhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(1))); Type* static_type = op_typer_.NumericMultiply(lhs, rhs); if (info->type_check() == TypeCheckKind::kNone) { new_type = static_type; @@ -419,15 +421,12 @@ class RepresentationSelector { } case IrOpcode::kSpeculativeNumberDivide: { - Type* lhs = FeedbackTypeOf(node->InputAt(0)); - Type* rhs = FeedbackTypeOf(node->InputAt(1)); - if (lhs->Is(Type::None()) || rhs->Is(Type::None())) return false; // TODO(jarin) The ToNumber conversion is too conservative here, // e.g. it will treat true as 1 even though the number check will // fail on a boolean. OperationTyper should have a function that // computes a more precise type. - lhs = op_typer_.ToNumber(lhs); - rhs = op_typer_.ToNumber(rhs); + Type* lhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0))); + Type* rhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(1))); Type* static_type = op_typer_.NumericDivide(lhs, rhs); if (info->type_check() == TypeCheckKind::kNone) { new_type = static_type; @@ -439,15 +438,12 @@ class RepresentationSelector { } case IrOpcode::kSpeculativeNumberModulus: { - Type* lhs = FeedbackTypeOf(node->InputAt(0)); - Type* rhs = FeedbackTypeOf(node->InputAt(1)); - if (lhs->Is(Type::None()) || rhs->Is(Type::None())) return false; // TODO(jarin) The ToNumber conversion is too conservative here, // e.g. it will treat true as 1 even though the number check will // fail on a boolean. OperationTyper should have a function that // computes a more precise type. - lhs = op_typer_.ToNumber(lhs); - rhs = op_typer_.ToNumber(rhs); + Type* lhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0))); + Type* rhs = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(1))); Type* static_type = op_typer_.NumericModulus(lhs, rhs); if (info->type_check() == TypeCheckKind::kNone) { new_type = static_type; @@ -945,7 +941,6 @@ class RepresentationSelector { } MachineSemantic DeoptValueSemanticOf(Type* type) { - CHECK(!type->Is(Type::None())); // We only need signedness to do deopt correctly. if (type->Is(Type::Signed32())) { return MachineSemantic::kInt32; diff --git a/test/mjsunit/compiler/regress-625558.js b/test/mjsunit/compiler/regress-625558.js new file mode 100644 index 0000000000..5d6b372632 --- /dev/null +++ b/test/mjsunit/compiler/regress-625558.js @@ -0,0 +1,14 @@ +// Copyright 2016 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 + +for (var global = 0; global <= 256; global++) { } + +function f() { + global = "luft"; + global += ++global; +} + +f(); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 7ce8286cf4..5d6c7178c4 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -251,7 +251,7 @@ 'debug-scopes': [PASS, SLOW], 'numops-fuzz-part*': [PASS, ['mode == debug', SLOW]], 'readonly': [PASS, SLOW], - 'regress/regress-1200351': [PASS, ['mode == debug', SLOW]], + 'regress/regress-1200351': [PASS, SLOW], 'regress/regress-crbug-474297': [PASS, ['mode == debug', SLOW]], 'es6/tail-call-megatest*': [PASS, SLOW, FAST_VARIANTS, ['tsan', SKIP]], @@ -413,12 +413,6 @@ 'wasm/embenchen/fasta': [PASS, FAST_VARIANTS], }], # 'gc_stress == True' -############################################################################## -['no_i18n == True and mode == debug', { - # Tests too slow for no18n debug. - 'regress/regress-1200351': [SKIP], -}], # 'no_i18n == True and mode == debug' - ############################################################################## ['byteorder == big', { # Emscripten requires little-endian, skip all tests on big endian platforms.