From 968a707134179c0d1bc0b20973d1392969e1473c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 29 Dec 2018 15:14:54 +0100 Subject: [PATCH] deps: V8: cherry-pick 56f6a76 Original commit message: [turbofan] Fix -0 check for subnormals. Previously we'd check `x` for -0 by testing `(1.0 / x) == -Infinity`, but this will yield the wrong results when `x` is a subnormal, i.e. really close to 0. In CSA we already perform bit checks to test for -0, so teach TurboFan to do the same for comparisons to -0 (via `Object.is`). We introduce a new NumberIsMinusZero simplified operator to handle the case where SimplifiedLowering already knows that the input is a number. Bug: chromium:903043, v8:6882 Change-Id: I0cb7c568029b461a92fc183104d5f359b4bfe7f4 Reviewed-on: https://chromium-review.googlesource.com/c/1328802 Commit-Queue: Benedikt Meurer Reviewed-by: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#57382} Refs: https://github.com/v8/v8/commit/56f6a763c27d77afbee997a50baa34996e97ba40 --- common.gypi | 2 +- .../src/compiler/effect-control-linearizer.cc | 47 +++++++++++++++++-- .../src/compiler/effect-control-linearizer.h | 1 + deps/v8/src/compiler/opcodes.h | 1 + deps/v8/src/compiler/simplified-lowering.cc | 12 +---- deps/v8/src/compiler/simplified-operator.cc | 1 + deps/v8/src/compiler/simplified-operator.h | 1 + deps/v8/src/compiler/typer.cc | 11 +++++ deps/v8/src/compiler/verifier.cc | 1 + .../mjsunit/regress/regress-crbug-903043.js | 39 +++++++++++++++ 10 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-crbug-903043.js diff --git a/common.gypi b/common.gypi index dd4f91484c9148..37f7e298ee5fdc 100644 --- a/common.gypi +++ b/common.gypi @@ -38,7 +38,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.8', + 'v8_embedder_string': '-node.9', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/compiler/effect-control-linearizer.cc b/deps/v8/src/compiler/effect-control-linearizer.cc index 97f78418d01df8..5f56d080d9e053 100644 --- a/deps/v8/src/compiler/effect-control-linearizer.cc +++ b/deps/v8/src/compiler/effect-control-linearizer.cc @@ -797,6 +797,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, case IrOpcode::kObjectIsMinusZero: result = LowerObjectIsMinusZero(node); break; + case IrOpcode::kNumberIsMinusZero: + result = LowerNumberIsMinusZero(node); + break; case IrOpcode::kObjectIsNaN: result = LowerObjectIsNaN(node); break; @@ -2543,6 +2546,14 @@ Node* EffectControlLinearizer::LowerObjectIsSafeInteger(Node* node) { return done.PhiAt(0); } +namespace { + +const int64_t kMinusZeroBits = bit_cast(-0.0); +const int32_t kMinusZeroLoBits = static_cast(kMinusZeroBits); +const int32_t kMinusZeroHiBits = static_cast(kMinusZeroBits >> 32); + +} // namespace + Node* EffectControlLinearizer::LowerObjectIsMinusZero(Node* node) { Node* value = node->InputAt(0); Node* zero = __ Int32Constant(0); @@ -2559,15 +2570,43 @@ Node* EffectControlLinearizer::LowerObjectIsMinusZero(Node* node) { // Check if {value} contains -0. Node* value_value = __ LoadField(AccessBuilder::ForHeapNumberValue(), value); - __ Goto(&done, - __ Float64Equal( - __ Float64Div(__ Float64Constant(1.0), value_value), - __ Float64Constant(-std::numeric_limits::infinity()))); + if (machine()->Is64()) { + Node* value64 = __ BitcastFloat64ToInt64(value_value); + __ Goto(&done, __ Word64Equal(value64, __ Int64Constant(kMinusZeroBits))); + } else { + Node* value_lo = __ Float64ExtractLowWord32(value_value); + __ GotoIfNot(__ Word32Equal(value_lo, __ Int32Constant(kMinusZeroLoBits)), + &done, zero); + Node* value_hi = __ Float64ExtractHighWord32(value_value); + __ Goto(&done, + __ Word32Equal(value_hi, __ Int32Constant(kMinusZeroHiBits))); + } __ Bind(&done); return done.PhiAt(0); } +Node* EffectControlLinearizer::LowerNumberIsMinusZero(Node* node) { + Node* value = node->InputAt(0); + + if (machine()->Is64()) { + Node* value64 = __ BitcastFloat64ToInt64(value); + return __ Word64Equal(value64, __ Int64Constant(kMinusZeroBits)); + } else { + auto done = __ MakeLabel(MachineRepresentation::kBit); + + Node* value_lo = __ Float64ExtractLowWord32(value); + __ GotoIfNot(__ Word32Equal(value_lo, __ Int32Constant(kMinusZeroLoBits)), + &done, __ Int32Constant(0)); + Node* value_hi = __ Float64ExtractHighWord32(value); + __ Goto(&done, + __ Word32Equal(value_hi, __ Int32Constant(kMinusZeroHiBits))); + + __ Bind(&done); + return done.PhiAt(0); + } +} + Node* EffectControlLinearizer::LowerObjectIsNaN(Node* node) { Node* value = node->InputAt(0); Node* zero = __ Int32Constant(0); diff --git a/deps/v8/src/compiler/effect-control-linearizer.h b/deps/v8/src/compiler/effect-control-linearizer.h index fcc4cad728e709..20c94b3d4f5482 100644 --- a/deps/v8/src/compiler/effect-control-linearizer.h +++ b/deps/v8/src/compiler/effect-control-linearizer.h @@ -106,6 +106,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer { Node* LowerObjectIsConstructor(Node* node); Node* LowerObjectIsDetectableCallable(Node* node); Node* LowerObjectIsMinusZero(Node* node); + Node* LowerNumberIsMinusZero(Node* node); Node* LowerObjectIsNaN(Node* node); Node* LowerNumberIsNaN(Node* node); Node* LowerObjectIsNonCallable(Node* node); diff --git a/deps/v8/src/compiler/opcodes.h b/deps/v8/src/compiler/opcodes.h index b6777ac4393857..ab854e6eb6160f 100644 --- a/deps/v8/src/compiler/opcodes.h +++ b/deps/v8/src/compiler/opcodes.h @@ -417,6 +417,7 @@ V(ObjectIsConstructor) \ V(ObjectIsDetectableCallable) \ V(ObjectIsMinusZero) \ + V(NumberIsMinusZero) \ V(ObjectIsNaN) \ V(NumberIsNaN) \ V(ObjectIsNonCallable) \ diff --git a/deps/v8/src/compiler/simplified-lowering.cc b/deps/v8/src/compiler/simplified-lowering.cc index 739f81f90d4bce..a06cb7423761ab 100644 --- a/deps/v8/src/compiler/simplified-lowering.cc +++ b/deps/v8/src/compiler/simplified-lowering.cc @@ -3008,17 +3008,7 @@ class RepresentationSelector { VisitUnop(node, UseInfo::TruncatingFloat64(), MachineRepresentation::kBit); if (lower()) { - // ObjectIsMinusZero(x:kRepFloat64) - // => Float64Equal(Float64Div(1.0,x),-Infinity) - Node* const input = node->InputAt(0); - node->ReplaceInput( - 0, jsgraph_->graph()->NewNode( - lowering->machine()->Float64Div(), - lowering->jsgraph()->Float64Constant(1.0), input)); - node->AppendInput(jsgraph_->zone(), - jsgraph_->Float64Constant( - -std::numeric_limits::infinity())); - NodeProperties::ChangeOp(node, lowering->machine()->Float64Equal()); + NodeProperties::ChangeOp(node, simplified()->NumberIsMinusZero()); } } else { VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kBit); diff --git a/deps/v8/src/compiler/simplified-operator.cc b/deps/v8/src/compiler/simplified-operator.cc index 3aa3d30a27349a..a898b715a584bd 100644 --- a/deps/v8/src/compiler/simplified-operator.cc +++ b/deps/v8/src/compiler/simplified-operator.cc @@ -745,6 +745,7 @@ bool operator==(CheckMinusZeroParameters const& lhs, V(ObjectIsConstructor, Operator::kNoProperties, 1, 0) \ V(ObjectIsDetectableCallable, Operator::kNoProperties, 1, 0) \ V(ObjectIsMinusZero, Operator::kNoProperties, 1, 0) \ + V(NumberIsMinusZero, Operator::kNoProperties, 1, 0) \ V(ObjectIsNaN, Operator::kNoProperties, 1, 0) \ V(NumberIsNaN, Operator::kNoProperties, 1, 0) \ V(ObjectIsNonCallable, Operator::kNoProperties, 1, 0) \ diff --git a/deps/v8/src/compiler/simplified-operator.h b/deps/v8/src/compiler/simplified-operator.h index df823fb0b0a2f5..4cea393a15cd74 100644 --- a/deps/v8/src/compiler/simplified-operator.h +++ b/deps/v8/src/compiler/simplified-operator.h @@ -719,6 +719,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final const Operator* ObjectIsConstructor(); const Operator* ObjectIsDetectableCallable(); const Operator* ObjectIsMinusZero(); + const Operator* NumberIsMinusZero(); const Operator* ObjectIsNaN(); const Operator* NumberIsNaN(); const Operator* ObjectIsNonCallable(); diff --git a/deps/v8/src/compiler/typer.cc b/deps/v8/src/compiler/typer.cc index eb357054e07a13..7ef20e7faea111 100644 --- a/deps/v8/src/compiler/typer.cc +++ b/deps/v8/src/compiler/typer.cc @@ -290,6 +290,7 @@ class Typer::Visitor : public Reducer { static Type ObjectIsConstructor(Type, Typer*); static Type ObjectIsDetectableCallable(Type, Typer*); static Type ObjectIsMinusZero(Type, Typer*); + static Type NumberIsMinusZero(Type, Typer*); static Type ObjectIsNaN(Type, Typer*); static Type NumberIsNaN(Type, Typer*); static Type ObjectIsNonCallable(Type, Typer*); @@ -597,6 +598,12 @@ Type Typer::Visitor::ObjectIsMinusZero(Type type, Typer* t) { return Type::Boolean(); } +Type Typer::Visitor::NumberIsMinusZero(Type type, Typer* t) { + if (type.Is(Type::MinusZero())) return t->singleton_true_; + if (!type.Maybe(Type::MinusZero())) return t->singleton_false_; + return Type::Boolean(); +} + Type Typer::Visitor::ObjectIsNaN(Type type, Typer* t) { if (type.Is(Type::NaN())) return t->singleton_true_; if (!type.Maybe(Type::NaN())) return t->singleton_false_; @@ -2104,6 +2111,10 @@ Type Typer::Visitor::TypeObjectIsMinusZero(Node* node) { return TypeUnaryOp(node, ObjectIsMinusZero); } +Type Typer::Visitor::TypeNumberIsMinusZero(Node* node) { + return TypeUnaryOp(node, NumberIsMinusZero); +} + Type Typer::Visitor::TypeNumberIsFloat64Hole(Node* node) { return Type::Boolean(); } diff --git a/deps/v8/src/compiler/verifier.cc b/deps/v8/src/compiler/verifier.cc index 913a2631dc9beb..7eedd2b37b8f68 100644 --- a/deps/v8/src/compiler/verifier.cc +++ b/deps/v8/src/compiler/verifier.cc @@ -1188,6 +1188,7 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) { CheckValueInputIs(node, 0, Type::Number()); CheckTypeIs(node, Type::Boolean()); break; + case IrOpcode::kNumberIsMinusZero: case IrOpcode::kNumberIsNaN: CheckValueInputIs(node, 0, Type::Number()); CheckTypeIs(node, Type::Boolean()); diff --git a/deps/v8/test/mjsunit/regress/regress-crbug-903043.js b/deps/v8/test/mjsunit/regress/regress-crbug-903043.js new file mode 100644 index 00000000000000..a877e6e12ab7ed --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-crbug-903043.js @@ -0,0 +1,39 @@ +// 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() { + function foo() { + const x = 1e-1; + return Object.is(-0, x * (-1e-308)); + } + + assertFalse(foo()); + assertFalse(foo()); + %OptimizeFunctionOnNextCall(foo); + assertFalse(foo()); +})(); + +(function() { + function foo(x) { + return Object.is(-0, x * (-1e-308)); + } + + assertFalse(foo(1e-1)); + assertFalse(foo(1e-1)); + %OptimizeFunctionOnNextCall(foo); + assertFalse(foo(1e-1)); +})(); + +(function() { + function foo(x) { + return Object.is(-0, x); + } + + assertFalse(foo(1e-1 * (-1e-308))); + assertFalse(foo(1e-1 * (-1e-308))); + %OptimizeFunctionOnNextCall(foo); + assertFalse(foo(1e-1 * (-1e-308))); +})();