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 number truncation assumption on HAdd inputs.
Browse files Browse the repository at this point in the history
In Crankshaft we don't know reliably know that an HAdd might not turn
into a string addition later (via deoptimization), so we cannot set the
HValue::kAllowUndefinedAsNaN flag on the HAdd instruction in those
cases. It doesn't seem to affect performance if we just remove the flag
completely from the HAdd instruction, so let's stick to that approach
for now.

[email protected]
BUG=v8:5074

Review-Url: https://codereview.chromium.org/2048643002
Cr-Commit-Position: refs/heads/master@{#36805}
  • Loading branch information
bmeurer authored and Commit bot committed Jun 8, 2016
1 parent 026ed50 commit f576e29
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/crankshaft/hydrogen-instructions.h
Original file line number Diff line number Diff line change
Expand Up @@ -4370,6 +4370,11 @@ class HAdd final : public HArithmeticBinaryOperation {
SetChangesFlag(kNewSpacePromotion);
ClearFlag(kAllowUndefinedAsNaN);
}
if (!right()->type().IsTaggedNumber() &&
!right()->representation().IsDouble() &&
!right()->representation().IsSmiOrInteger32()) {
ClearFlag(kAllowUndefinedAsNaN);
}
}

Representation RepresentationFromInputs() override;
Expand Down
18 changes: 18 additions & 0 deletions test/mjsunit/compiler/regress-5074.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 s = [,0.1];

function foo(a, b) {
var x = s[a];
s[1] = 0.1;
return x + b;
}

assertEquals(2.1, foo(1, 2));
assertEquals(2.1, foo(1, 2));
%OptimizeFunctionOnNextCall(foo);
assertEquals("undefined2", foo(0, "2"));

0 comments on commit f576e29

Please sign in to comment.