Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[crankshaft] Fix invalid ToNumber optimization.
Browse files Browse the repository at this point in the history
We cannot optimize away ToNumber conversions based on the Type that we
see in Crankshaft, as this might be the (unchecked or even pretruncated)
lower bound. We can only use the HType, which is based on the definition.

R=jkummerow@chromium.org
BUG=chromium:590989
LOG=n

Review URL: https://codereview.chromium.org/1757013002

Cr-Commit-Position: refs/heads/master@{#34445}
  • Loading branch information
bmeurer authored and Commit bot committed Mar 2, 2016
1 parent 017375f commit 0c35579
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 8 deletions.
12 changes: 5 additions & 7 deletions src/crankshaft/hydrogen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2050,9 +2050,8 @@ HValue* HGraphBuilder::BuildNumberToString(HValue* object, Type* type) {
return Pop();
}


HValue* HGraphBuilder::BuildToNumber(HValue* input, Type* input_type) {
if (input->type().IsTaggedNumber() || input_type->Is(Type::Number())) {
HValue* HGraphBuilder::BuildToNumber(HValue* input) {
if (input->type().IsTaggedNumber()) {
return input;
}
Callable callable = CodeFactory::ToNumber(isolate());
Expand Down Expand Up @@ -11081,10 +11080,10 @@ HValue* HGraphBuilder::BuildBinaryOperation(Token::Value op, HValue* left,
// Special case for +x here.
if (op == Token::MUL) {
if (left->EqualsInteger32Constant(1)) {
return BuildToNumber(right, right_type);
return BuildToNumber(right);
}
if (right->EqualsInteger32Constant(1)) {
return BuildToNumber(left, left_type);
return BuildToNumber(left);
}
}

Expand Down Expand Up @@ -12307,8 +12306,7 @@ void HOptimizedGraphBuilder::GenerateToNumber(CallRuntime* call) {
CHECK_ALIVE(VisitForValue(call->arguments()->at(0)));
Callable callable = CodeFactory::ToNumber(isolate());
HValue* input = Pop();
Type* input_type = Type::Any();
HValue* result = BuildToNumber(input, input_type);
HValue* result = BuildToNumber(input);
if (result->HasObservableSideEffects()) {
if (!ast_context()->IsEffect()) Push(result);
Add<HSimulate>(call->id(), REMOVABLE_SIMULATE);
Expand Down
2 changes: 1 addition & 1 deletion src/crankshaft/hydrogen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ class HGraphBuilder {
bool is_jsarray);

HValue* BuildNumberToString(HValue* object, Type* type);
HValue* BuildToNumber(HValue* input, Type* input_type);
HValue* BuildToNumber(HValue* input);
HValue* BuildToObject(HValue* receiver);

void BuildJSObjectCheck(HValue* receiver,
Expand Down
18 changes: 18 additions & 0 deletions test/mjsunit/regress/regress-crbug-590989-1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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

var o = {}
var p = {foo: 1.5}

function g(x) { return x.foo === +x.foo; }

assertEquals(false, g(o));
assertEquals(false, g(o));
%OptimizeFunctionOnNextCall(g);
assertEquals(false, g(o)); // Still fine here.
assertEquals(true, g(p));
%OptimizeFunctionOnNextCall(g);
assertEquals(false, g(o)); // Confused by type feedback.
12 changes: 12 additions & 0 deletions test/mjsunit/regress/regress-crbug-590989-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// 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

function f(x) { return x === +x; }

assertEquals(false, f(undefined));
assertEquals(false, f(undefined));
%OptimizeFunctionOnNextCall(f);
assertEquals(false, f(undefined)); // Interestingly this fails right away.

0 comments on commit 0c35579

Please sign in to comment.