From d9c9b003531ce837d3c61b97fc23d8a41f15ef73 Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Wed, 2 May 2018 13:49:08 +0200 Subject: [PATCH] [turbofan] Fix wrong optimization of Number.parseInt We incorrectly used a TurboFan typer check for {0,10,undefined} on the radix argument on Number.parseInt, which was internally widened to the checking whether radix is in range 0-10 or undefined. This CL introduces two separate checks. Bug: chromium:838766 Change-Id: I5ebfc1c82bad5b9794b4f844e79e4df01f541a83 Reviewed-on: https://chromium-review.googlesource.com/1039197 Reviewed-by: Benedikt Meurer Commit-Queue: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#52914} --- src/compiler/js-typed-lowering.cc | 5 ++++- src/compiler/type-cache.h | 4 ++-- test/mjsunit/regress/regress-838766.js | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 test/mjsunit/regress/regress-838766.js diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 1cb711b299..f5f7e42fec 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -2227,8 +2227,11 @@ Reduction JSTypedLowering::ReduceJSParseInt(Node* node) { Type value_type = NodeProperties::GetType(value); Node* radix = NodeProperties::GetValueInput(node, 1); Type radix_type = NodeProperties::GetType(radix); + // We need kTenOrUndefined and kZeroOrUndefined because + // the type representing {0,10} would become the range 1-10. if (value_type.Is(type_cache_.kSafeInteger) && - radix_type.Is(type_cache_.kZeroOrTenOrUndefined)) { + (radix_type.Is(type_cache_.kTenOrUndefined) || + radix_type.Is(type_cache_.kZeroOrUndefined))) { // Number.parseInt(a:safe-integer) -> a // Number.parseInt(a:safe-integer,b:#0\/undefined) -> a // Number.parseInt(a:safe-integer,b:#10\/undefined) -> a diff --git a/src/compiler/type-cache.h b/src/compiler/type-cache.h index 6e1262c84f..7a02c9a37c 100644 --- a/src/compiler/type-cache.h +++ b/src/compiler/type-cache.h @@ -49,8 +49,8 @@ class TypeCache final { Type::Union(kSingletonZero, Type::MinusZero(), zone()); Type const kZeroOrUndefined = Type::Union(kSingletonZero, Type::Undefined(), zone()); - Type const kZeroOrTenOrUndefined = - Type::Union(kZeroOrUndefined, kSingletonTen, zone()); + Type const kTenOrUndefined = + Type::Union(kSingletonTen, Type::Undefined(), zone()); Type const kMinusOneOrZero = CreateRange(-1.0, 0.0); Type const kMinusOneToOneOrMinusZeroOrNaN = Type::Union( Type::Union(CreateRange(-1.0, 1.0), Type::MinusZero(), zone()), diff --git a/test/mjsunit/regress/regress-838766.js b/test/mjsunit/regress/regress-838766.js new file mode 100644 index 0000000000..1626ee2428 --- /dev/null +++ b/test/mjsunit/regress/regress-838766.js @@ -0,0 +1,14 @@ +// Copyright 2018 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 foo(x) { + x = x | 2147483648; + return Number.parseInt(x + 65535, 8); +} +assertEquals(-72161, foo()); +assertEquals(-72161, foo()); +%OptimizeFunctionOnNextCall(foo); +assertEquals(-72161, foo());