Skip to content

Commit 56f6a76

Browse files
bmeurerCommit Bot
authored andcommitted
[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}
1 parent 88fe4e5 commit 56f6a76

File tree

9 files changed

+99
-15
lines changed

9 files changed

+99
-15
lines changed

src/compiler/effect-control-linearizer.cc

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
809809
case IrOpcode::kObjectIsMinusZero:
810810
result = LowerObjectIsMinusZero(node);
811811
break;
812+
case IrOpcode::kNumberIsMinusZero:
813+
result = LowerNumberIsMinusZero(node);
814+
break;
812815
case IrOpcode::kObjectIsNaN:
813816
result = LowerObjectIsNaN(node);
814817
break;
@@ -2660,6 +2663,14 @@ Node* EffectControlLinearizer::LowerObjectIsSafeInteger(Node* node) {
26602663
return done.PhiAt(0);
26612664
}
26622665

2666+
namespace {
2667+
2668+
const int64_t kMinusZeroBits = bit_cast<int64_t>(-0.0);
2669+
const int32_t kMinusZeroLoBits = static_cast<int32_t>(kMinusZeroBits);
2670+
const int32_t kMinusZeroHiBits = static_cast<int32_t>(kMinusZeroBits >> 32);
2671+
2672+
} // namespace
2673+
26632674
Node* EffectControlLinearizer::LowerObjectIsMinusZero(Node* node) {
26642675
Node* value = node->InputAt(0);
26652676
Node* zero = __ Int32Constant(0);
@@ -2676,15 +2687,43 @@ Node* EffectControlLinearizer::LowerObjectIsMinusZero(Node* node) {
26762687

26772688
// Check if {value} contains -0.
26782689
Node* value_value = __ LoadField(AccessBuilder::ForHeapNumberValue(), value);
2679-
__ Goto(&done,
2680-
__ Float64Equal(
2681-
__ Float64Div(__ Float64Constant(1.0), value_value),
2682-
__ Float64Constant(-std::numeric_limits<double>::infinity())));
2690+
if (machine()->Is64()) {
2691+
Node* value64 = __ BitcastFloat64ToInt64(value_value);
2692+
__ Goto(&done, __ Word64Equal(value64, __ Int64Constant(kMinusZeroBits)));
2693+
} else {
2694+
Node* value_lo = __ Float64ExtractLowWord32(value_value);
2695+
__ GotoIfNot(__ Word32Equal(value_lo, __ Int32Constant(kMinusZeroLoBits)),
2696+
&done, zero);
2697+
Node* value_hi = __ Float64ExtractHighWord32(value_value);
2698+
__ Goto(&done,
2699+
__ Word32Equal(value_hi, __ Int32Constant(kMinusZeroHiBits)));
2700+
}
26832701

26842702
__ Bind(&done);
26852703
return done.PhiAt(0);
26862704
}
26872705

2706+
Node* EffectControlLinearizer::LowerNumberIsMinusZero(Node* node) {
2707+
Node* value = node->InputAt(0);
2708+
2709+
if (machine()->Is64()) {
2710+
Node* value64 = __ BitcastFloat64ToInt64(value);
2711+
return __ Word64Equal(value64, __ Int64Constant(kMinusZeroBits));
2712+
} else {
2713+
auto done = __ MakeLabel(MachineRepresentation::kBit);
2714+
2715+
Node* value_lo = __ Float64ExtractLowWord32(value);
2716+
__ GotoIfNot(__ Word32Equal(value_lo, __ Int32Constant(kMinusZeroLoBits)),
2717+
&done, __ Int32Constant(0));
2718+
Node* value_hi = __ Float64ExtractHighWord32(value);
2719+
__ Goto(&done,
2720+
__ Word32Equal(value_hi, __ Int32Constant(kMinusZeroHiBits)));
2721+
2722+
__ Bind(&done);
2723+
return done.PhiAt(0);
2724+
}
2725+
}
2726+
26882727
Node* EffectControlLinearizer::LowerObjectIsNaN(Node* node) {
26892728
Node* value = node->InputAt(0);
26902729
Node* zero = __ Int32Constant(0);

src/compiler/effect-control-linearizer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
110110
Node* LowerObjectIsConstructor(Node* node);
111111
Node* LowerObjectIsDetectableCallable(Node* node);
112112
Node* LowerObjectIsMinusZero(Node* node);
113+
Node* LowerNumberIsMinusZero(Node* node);
113114
Node* LowerObjectIsNaN(Node* node);
114115
Node* LowerNumberIsNaN(Node* node);
115116
Node* LowerObjectIsNonCallable(Node* node);

src/compiler/opcodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@
426426
V(ObjectIsConstructor) \
427427
V(ObjectIsDetectableCallable) \
428428
V(ObjectIsMinusZero) \
429+
V(NumberIsMinusZero) \
429430
V(ObjectIsNaN) \
430431
V(NumberIsNaN) \
431432
V(ObjectIsNonCallable) \

src/compiler/simplified-lowering.cc

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3050,17 +3050,7 @@ class RepresentationSelector {
30503050
VisitUnop(node, UseInfo::TruncatingFloat64(),
30513051
MachineRepresentation::kBit);
30523052
if (lower()) {
3053-
// ObjectIsMinusZero(x:kRepFloat64)
3054-
// => Float64Equal(Float64Div(1.0,x),-Infinity)
3055-
Node* const input = node->InputAt(0);
3056-
node->ReplaceInput(
3057-
0, jsgraph_->graph()->NewNode(
3058-
lowering->machine()->Float64Div(),
3059-
lowering->jsgraph()->Float64Constant(1.0), input));
3060-
node->AppendInput(jsgraph_->zone(),
3061-
jsgraph_->Float64Constant(
3062-
-std::numeric_limits<double>::infinity()));
3063-
NodeProperties::ChangeOp(node, lowering->machine()->Float64Equal());
3053+
NodeProperties::ChangeOp(node, simplified()->NumberIsMinusZero());
30643054
}
30653055
} else {
30663056
VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kBit);

src/compiler/simplified-operator.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,7 @@ bool operator==(CheckMinusZeroParameters const& lhs,
747747
V(ObjectIsConstructor, Operator::kNoProperties, 1, 0) \
748748
V(ObjectIsDetectableCallable, Operator::kNoProperties, 1, 0) \
749749
V(ObjectIsMinusZero, Operator::kNoProperties, 1, 0) \
750+
V(NumberIsMinusZero, Operator::kNoProperties, 1, 0) \
750751
V(ObjectIsNaN, Operator::kNoProperties, 1, 0) \
751752
V(NumberIsNaN, Operator::kNoProperties, 1, 0) \
752753
V(ObjectIsNonCallable, Operator::kNoProperties, 1, 0) \

src/compiler/simplified-operator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
726726
const Operator* ObjectIsConstructor();
727727
const Operator* ObjectIsDetectableCallable();
728728
const Operator* ObjectIsMinusZero();
729+
const Operator* NumberIsMinusZero();
729730
const Operator* ObjectIsNaN();
730731
const Operator* NumberIsNaN();
731732
const Operator* ObjectIsNonCallable();

src/compiler/typer.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ class Typer::Visitor : public Reducer {
290290
static Type ObjectIsConstructor(Type, Typer*);
291291
static Type ObjectIsDetectableCallable(Type, Typer*);
292292
static Type ObjectIsMinusZero(Type, Typer*);
293+
static Type NumberIsMinusZero(Type, Typer*);
293294
static Type ObjectIsNaN(Type, Typer*);
294295
static Type NumberIsNaN(Type, Typer*);
295296
static Type ObjectIsNonCallable(Type, Typer*);
@@ -599,6 +600,12 @@ Type Typer::Visitor::ObjectIsMinusZero(Type type, Typer* t) {
599600
return Type::Boolean();
600601
}
601602

603+
Type Typer::Visitor::NumberIsMinusZero(Type type, Typer* t) {
604+
if (type.Is(Type::MinusZero())) return t->singleton_true_;
605+
if (!type.Maybe(Type::MinusZero())) return t->singleton_false_;
606+
return Type::Boolean();
607+
}
608+
602609
Type Typer::Visitor::ObjectIsNaN(Type type, Typer* t) {
603610
if (type.Is(Type::NaN())) return t->singleton_true_;
604611
if (!type.Maybe(Type::NaN())) return t->singleton_false_;
@@ -2142,6 +2149,10 @@ Type Typer::Visitor::TypeObjectIsMinusZero(Node* node) {
21422149
return TypeUnaryOp(node, ObjectIsMinusZero);
21432150
}
21442151

2152+
Type Typer::Visitor::TypeNumberIsMinusZero(Node* node) {
2153+
return TypeUnaryOp(node, NumberIsMinusZero);
2154+
}
2155+
21452156
Type Typer::Visitor::TypeNumberIsFloat64Hole(Node* node) {
21462157
return Type::Boolean();
21472158
}

src/compiler/verifier.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,7 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
12091209
CheckValueInputIs(node, 0, Type::Number());
12101210
CheckTypeIs(node, Type::Boolean());
12111211
break;
1212+
case IrOpcode::kNumberIsMinusZero:
12121213
case IrOpcode::kNumberIsNaN:
12131214
CheckValueInputIs(node, 0, Type::Number());
12141215
CheckTypeIs(node, Type::Boolean());
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2018 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
(function() {
8+
function foo() {
9+
const x = 1e-1;
10+
return Object.is(-0, x * (-1e-308));
11+
}
12+
13+
assertFalse(foo());
14+
assertFalse(foo());
15+
%OptimizeFunctionOnNextCall(foo);
16+
assertFalse(foo());
17+
})();
18+
19+
(function() {
20+
function foo(x) {
21+
return Object.is(-0, x * (-1e-308));
22+
}
23+
24+
assertFalse(foo(1e-1));
25+
assertFalse(foo(1e-1));
26+
%OptimizeFunctionOnNextCall(foo);
27+
assertFalse(foo(1e-1));
28+
})();
29+
30+
(function() {
31+
function foo(x) {
32+
return Object.is(-0, x);
33+
}
34+
35+
assertFalse(foo(1e-1 * (-1e-308)));
36+
assertFalse(foo(1e-1 * (-1e-308)));
37+
%OptimizeFunctionOnNextCall(foo);
38+
assertFalse(foo(1e-1 * (-1e-308)));
39+
})();

0 commit comments

Comments
 (0)