From c17b44bd3a3ae84fa2c39afd7ebbfdfbcbcfefed Mon Sep 17 00:00:00 2001 From: verwaest Date: Thu, 30 Jun 2016 08:15:46 -0700 Subject: [PATCH] Fix double canonicalization This turns the blacklist back into a white-list adding all binary operations as allowed operations. The one known fix is that it forces canonicalization of the double-hole as double constant. BUG=chromium:621147 Review-Url: https://codereview.chromium.org/2106393002 Cr-Commit-Position: refs/heads/master@{#37452} --- src/crankshaft/hydrogen-instructions.cc | 25 +++++++++++-------- .../regress-double-canonicalization.js | 24 ++++++++++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 test/mjsunit/regress/regress-double-canonicalization.js diff --git a/src/crankshaft/hydrogen-instructions.cc b/src/crankshaft/hydrogen-instructions.cc index 4b117d63b7..9947dee6d4 100644 --- a/src/crankshaft/hydrogen-instructions.cc +++ b/src/crankshaft/hydrogen-instructions.cc @@ -2194,7 +2194,11 @@ HConstant::HConstant(Handle object, Representation r) int32_value_ = DoubleToInt32(n); bit_field_ = HasSmiValueField::update( bit_field_, has_int32_value && Smi::IsValid(int32_value_)); - double_value_ = n; + if (std::isnan(n)) { + double_value_ = std::numeric_limits::quiet_NaN(); + } else { + double_value_ = n; + } bit_field_ = HasDoubleValueField::update(bit_field_, true); } @@ -2247,7 +2251,6 @@ HConstant::HConstant(int32_t integer_value, Representation r, Initialize(r); } - HConstant::HConstant(double double_value, Representation r, bool is_not_in_new_space, Unique object) : object_(object), @@ -2261,8 +2264,7 @@ HConstant::HConstant(double double_value, Representation r, !std::isnan(double_value)) | IsUndetectableField::encode(false) | InstanceTypeField::encode(kUnknownInstanceType)), - int32_value_(DoubleToInt32(double_value)), - double_value_(double_value) { + int32_value_(DoubleToInt32(double_value)) { bit_field_ = HasSmiValueField::update( bit_field_, HasInteger32Value() && Smi::IsValid(int32_value_)); // It's possible to create a constant with a value in Smi-range but stored @@ -2270,6 +2272,11 @@ HConstant::HConstant(double double_value, Representation r, bool could_be_heapobject = r.IsTagged() && !object.handle().is_null(); bool is_smi = HasSmiValue() && !could_be_heapobject; set_type(is_smi ? HType::Smi() : HType::TaggedNumber()); + if (std::isnan(double_value)) { + double_value_ = std::numeric_limits::quiet_NaN(); + } else { + double_value_ = double_value; + } Initialize(r); } @@ -3288,13 +3295,11 @@ bool HStoreKeyed::NeedsCanonicalization() { Representation from = HChange::cast(value())->from(); return from.IsTagged() || from.IsHeapObject(); } - case kLoadNamedField: - case kPhi: { - // Better safe than sorry... - return true; - } - default: + case kConstant: + // Double constants are canonicalized upon construction. return false; + default: + return !value()->IsBinaryOperation(); } } diff --git a/test/mjsunit/regress/regress-double-canonicalization.js b/test/mjsunit/regress/regress-double-canonicalization.js new file mode 100644 index 0000000000..2b345d2bb7 --- /dev/null +++ b/test/mjsunit/regress/regress-double-canonicalization.js @@ -0,0 +1,24 @@ +// 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 ab = new ArrayBuffer(8); +var i_view = new Int32Array(ab); +i_view[0] = %GetHoleNaNUpper() +i_view[1] = %GetHoleNaNLower(); +var hole_nan = (new Float64Array(ab))[0]; + +var array = []; + +function write() { + array[0] = hole_nan; +} + +write(); +%OptimizeFunctionOnNextCall(write); +write(); +array[1] = undefined; +assertTrue(isNaN(array[0])); +assertEquals("number", typeof array[0]);