Skip to content

Commit

Permalink
deps: cherry-pick 56f6a76 from upstream V8
Browse files Browse the repository at this point in the history
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 <bmeurer@chromium.org>
    Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57382}

PR-URL: #25269
Refs: v8/v8@56f6a76
Fixes: #25268
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR authored and danbev committed Jan 9, 2019
1 parent e54d11e commit ddbb7d7
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 16 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####

Expand Down
47 changes: 43 additions & 4 deletions deps/v8/src/compiler/effect-control-linearizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2543,6 +2546,14 @@ Node* EffectControlLinearizer::LowerObjectIsSafeInteger(Node* node) {
return done.PhiAt(0);
}

namespace {

const int64_t kMinusZeroBits = bit_cast<int64_t>(-0.0);
const int32_t kMinusZeroLoBits = static_cast<int32_t>(kMinusZeroBits);
const int32_t kMinusZeroHiBits = static_cast<int32_t>(kMinusZeroBits >> 32);

} // namespace

Node* EffectControlLinearizer::LowerObjectIsMinusZero(Node* node) {
Node* value = node->InputAt(0);
Node* zero = __ Int32Constant(0);
Expand All @@ -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<double>::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);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/effect-control-linearizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/opcodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@
V(ObjectIsConstructor) \
V(ObjectIsDetectableCallable) \
V(ObjectIsMinusZero) \
V(NumberIsMinusZero) \
V(ObjectIsNaN) \
V(NumberIsNaN) \
V(ObjectIsNonCallable) \
Expand Down
12 changes: 1 addition & 11 deletions deps/v8/src/compiler/simplified-lowering.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<double>::infinity()));
NodeProperties::ChangeOp(node, lowering->machine()->Float64Equal());
NodeProperties::ChangeOp(node, simplified()->NumberIsMinusZero());
}
} else {
VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kBit);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/simplified-operator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/simplified-operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/src/compiler/typer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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*);
Expand Down Expand Up @@ -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_;
Expand Down Expand Up @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/verifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
39 changes: 39 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-crbug-903043.js
Original file line number Diff line number Diff line change
@@ -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)));
})();

0 comments on commit ddbb7d7

Please sign in to comment.