Skip to content

Commit c33af9b

Browse files
victorgomesV8 LUCI CQ
victorgomes
authored and
V8 LUCI CQ
committed
[maglev] Check for negative zero and NaN in CheckValueEqualsFloat64
Fixed: 406043356 Change-Id: Ibc9b3766846a4ac0a1416767099cf9924dcc28c1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6396659 Auto-Submit: Victor Gomes <[email protected]> Commit-Queue: Victor Gomes <[email protected]> Commit-Queue: Darius Mercadier <[email protected]> Reviewed-by: Darius Mercadier <[email protected]> Cr-Commit-Position: refs/heads/main@{#99476}
1 parent 770accf commit c33af9b

11 files changed

Lines changed: 156 additions & 89 deletions

File tree

‎src/compiler/turboshaft/assembler.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4715,8 +4715,8 @@ class TurboshaftAssemblerOpInterface
47154715
return ReduceIfReachableSameValue(left, right, mode);
47164716
}
47174717

4718-
V<Word32> Float64SameValue(V<Float64> left, V<Float64> right) {
4719-
return ReduceIfReachableFloat64SameValue(left, right);
4718+
V<Word32> Float64SameValue(ConstOrV<Float64> left, ConstOrV<Float64> right) {
4719+
return ReduceIfReachableFloat64SameValue(resolve(left), resolve(right));
47204720
}
47214721

47224722
OpIndex FastApiCall(V<turboshaft::FrameState> frame_state,

‎src/compiler/turboshaft/machine-lowering-reducer-inl.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3203,6 +3203,7 @@ class MachineLoweringReducer : public Next {
32033203
V<Word32> REDUCE(Float64SameValue)(V<Float64> left, V<Float64> right) {
32043204
Label<Word32> done(this);
32053205

3206+
// TODO(dmercadier): Optimize if one of the sides is a constant.
32063207
IF (__ Float64Equal(left, right)) {
32073208
// Even if the values are float64-equal, we still need to distinguish
32083209
// zero and minus zero.

‎src/compiler/turboshaft/maglev-graph-building-phase.cc‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2351,14 +2351,6 @@ class GraphBuildingNodeProcessor {
23512351
SetMap(node, value);
23522352
return maglev::ProcessResult::kContinue;
23532353
}
2354-
maglev::ProcessResult Process(maglev::CheckFloat64IsNan* node,
2355-
const maglev::ProcessingState& state) {
2356-
GET_FRAME_STATE_MAYBE_ABORT(frame_state, node->eager_deopt_info());
2357-
__ DeoptimizeIfNot(__ Float64IsNaN(Map(node->target_input())), frame_state,
2358-
node->deoptimize_reason(),
2359-
node->eager_deopt_info()->feedback_to_update());
2360-
return maglev::ProcessResult::kContinue;
2361-
}
23622354
void CheckMaps(V<Object> receiver_input, V<FrameState> frame_state,
23632355
OptionalV<Map> object_map, const FeedbackSource& feedback,
23642356
const compiler::ZoneRefSet<Map>& maps, bool check_heap_object,
@@ -2461,14 +2453,13 @@ class GraphBuildingNodeProcessor {
24612453
node->eager_deopt_info()->feedback_to_update());
24622454
return maglev::ProcessResult::kContinue;
24632455
}
2464-
maglev::ProcessResult Process(maglev::CheckValueEqualsFloat64* node,
2456+
maglev::ProcessResult Process(maglev::CheckFloat64SameValue* node,
24652457
const maglev::ProcessingState& state) {
2466-
DCHECK(!std::isnan(node->value()));
24672458
GET_FRAME_STATE_MAYBE_ABORT(frame_state, node->eager_deopt_info());
2468-
__ DeoptimizeIfNot(
2469-
__ Float64Equal(Map(node->target_input()), node->value()), frame_state,
2470-
node->deoptimize_reason(),
2471-
node->eager_deopt_info()->feedback_to_update());
2459+
__ DeoptimizeIfNot(__ Float64SameValue(Map(node->target_input()),
2460+
node->value().get_scalar()),
2461+
frame_state, node->deoptimize_reason(),
2462+
node->eager_deopt_info()->feedback_to_update());
24722463
return maglev::ProcessResult::kContinue;
24732464
}
24742465
maglev::ProcessResult Process(maglev::CheckString* node,

‎src/maglev/arm/maglev-ir-arm.cc‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,31 @@ void CheckedIntPtrToInt32::GenerateCode(MaglevAssembler* masm,
172172
// On 32-bit platforms, IntPtr is the same as Int32.
173173
}
174174

175+
void CheckFloat64SameValue::SetValueLocationConstraints() {
176+
UseRegister(target_input());
177+
set_temporaries_needed((value().get_scalar() == 0) ? 1 : 0);
178+
set_double_temporaries_needed(value().is_nan() ? 0 : 1);
179+
}
180+
void CheckFloat64SameValue::GenerateCode(MaglevAssembler* masm,
181+
const ProcessingState& state) {
182+
Label* fail = __ GetDeoptLabel(this, deoptimize_reason());
183+
MaglevAssembler::TemporaryRegisterScope temps(masm);
184+
DoubleRegister double_scratch = temps.AcquireScratchDouble();
185+
DoubleRegister target = ToDoubleRegister(target_input());
186+
if (value().is_nan()) {
187+
__ JumpIfNotNan(target, fail);
188+
} else {
189+
__ Move(double_scratch, value());
190+
__ CompareFloat64AndJumpIf(double_scratch, target, kNotEqual, fail, fail);
191+
if (value().get_scalar() == 0) { // If value is +0.0 or -0.0.
192+
Register scratch = temps.AcquireScratch();
193+
__ VmovHigh(scratch, target);
194+
__ cmp(scratch, Operand(0));
195+
__ JumpIf(value().get_bits() == 0 ? kNotEqual : kEqual, fail);
196+
}
197+
}
198+
}
199+
175200
void Int32AddWithOverflow::SetValueLocationConstraints() {
176201
UseRegister(left_input());
177202
UseRegister(right_input());

‎src/maglev/arm64/maglev-ir-arm64.cc‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,36 @@ void CheckedIntPtrToInt32::GenerateCode(MaglevAssembler* masm,
212212
__ GetDeoptLabel(this, DeoptimizeReason::kNotInt32));
213213
}
214214

215+
void CheckFloat64SameValue::SetValueLocationConstraints() {
216+
UseRegister(target_input());
217+
set_temporaries_needed((value().get_scalar() == 0) ? 1 : 0);
218+
set_double_temporaries_needed(
219+
value().is_nan() || (value().get_scalar() == 0) ? 0 : 1);
220+
}
221+
void CheckFloat64SameValue::GenerateCode(MaglevAssembler* masm,
222+
const ProcessingState& state) {
223+
Label* fail = __ GetDeoptLabel(this, deoptimize_reason());
224+
MaglevAssembler::TemporaryRegisterScope temps(masm);
225+
DoubleRegister target = ToDoubleRegister(target_input());
226+
if (value().is_nan()) {
227+
__ JumpIfNotNan(target, fail);
228+
} else if (value().get_scalar() == 0) { // If value is +0.0 or -0.0.
229+
Register scratch = temps.AcquireScratch();
230+
__ Fcmp(target, value().get_scalar());
231+
__ JumpIf(kNotEqual, fail);
232+
__ Fmov(scratch, target);
233+
if (value().get_bits() == 0) {
234+
__ Tbnz(scratch, 63, fail);
235+
} else {
236+
__ Tbz(scratch, 63, fail);
237+
}
238+
} else {
239+
DoubleRegister double_scratch = temps.AcquireScratchDouble();
240+
__ Move(double_scratch, value());
241+
__ CompareFloat64AndJumpIf(double_scratch, target, kNotEqual, fail, fail);
242+
}
243+
}
244+
215245
void Int32AddWithOverflow::SetValueLocationConstraints() {
216246
UseRegister(left_input());
217247
if (TryGetAddImmediateInt32ConstantInput(this, kRightIndex)) {

‎src/maglev/maglev-graph-builder.cc‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5871,7 +5871,7 @@ MaybeReduceResult MaglevGraphBuilder::TryBuildStoreField(
58715871
TryFoldLoadConstantDoubleField(constant_value, access_info);
58725872
if (constant.has_value()) {
58735873
if (!constant->is_nan()) {
5874-
AddNewNode<CheckValueEqualsFloat64>(
5874+
AddNewNode<CheckFloat64SameValue>(
58755875
{GetAccumulator()}, constant.value(),
58765876
DeoptimizeReason::kStoreToConstant);
58775877
return ReduceResult::Done();
@@ -10883,11 +10883,7 @@ ReduceResult MaglevGraphBuilder::BuildCheckNumericalValue(
1088310883
if (!NodeTypeIs(NodeType::kNumber, GetType(node))) {
1088410884
return EmitUnconditionalDeopt(reason);
1088510885
}
10886-
if (ref_value.is_nan()) {
10887-
AddNewNode<CheckFloat64IsNan>({node}, reason);
10888-
} else {
10889-
AddNewNode<CheckValueEqualsFloat64>({node}, ref_value, reason);
10890-
}
10886+
AddNewNode<CheckFloat64SameValue>({node}, ref_value, reason);
1089110887
}
1089210888

1089310889
SetKnownValue(node, ref, NodeType::kNumber);

‎src/maglev/maglev-ir.cc‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,30 +3353,6 @@ void CheckValueEqualsInt32::GenerateCode(MaglevAssembler* masm,
33533353
__ CompareInt32AndJumpIf(target, value(), kNotEqual, fail);
33543354
}
33553355

3356-
void CheckValueEqualsFloat64::SetValueLocationConstraints() {
3357-
UseRegister(target_input());
3358-
set_double_temporaries_needed(1);
3359-
}
3360-
void CheckValueEqualsFloat64::GenerateCode(MaglevAssembler* masm,
3361-
const ProcessingState& state) {
3362-
Label* fail = __ GetDeoptLabel(this, deoptimize_reason());
3363-
MaglevAssembler::TemporaryRegisterScope temps(masm);
3364-
DoubleRegister scratch = temps.AcquireDouble();
3365-
DoubleRegister target = ToDoubleRegister(target_input());
3366-
__ Move(scratch, value());
3367-
__ CompareFloat64AndJumpIf(scratch, target, kNotEqual, fail, fail);
3368-
}
3369-
3370-
void CheckFloat64IsNan::SetValueLocationConstraints() {
3371-
UseRegister(target_input());
3372-
}
3373-
void CheckFloat64IsNan::GenerateCode(MaglevAssembler* masm,
3374-
const ProcessingState& state) {
3375-
Label* fail = __ GetDeoptLabel(this, deoptimize_reason());
3376-
DoubleRegister target = ToDoubleRegister(target_input());
3377-
__ JumpIfNotNan(target, fail);
3378-
}
3379-
33803356
void CheckValueEqualsString::SetValueLocationConstraints() {
33813357
using D = CallInterfaceDescriptorFor<Builtin::kStringEqual>::type;
33823358
UseFixed(target_input(), D::GetRegisterParameter(D::kLeft));
@@ -7654,11 +7630,6 @@ void TransitionElementsKindOrCheckMap::PrintParams(
76547630
os << "]-->" << *transition_target().object() << ")";
76557631
}
76567632

7657-
void CheckFloat64IsNan::PrintParams(std::ostream& os,
7658-
MaglevGraphLabeller* graph_labeller) const {
7659-
os << "(" << DeoptimizeReasonToString(deoptimize_reason()) << ")";
7660-
}
7661-
76627633
void CheckDynamicValue::PrintParams(std::ostream& os,
76637634
MaglevGraphLabeller* graph_labeller) const {
76647635
os << "(" << DeoptimizeReasonToString(deoptimize_reason()) << ")";
@@ -7676,10 +7647,10 @@ void CheckValueEqualsInt32::PrintParams(
76767647
<< ")";
76777648
}
76787649

7679-
void CheckValueEqualsFloat64::PrintParams(
7650+
void CheckFloat64SameValue::PrintParams(
76807651
std::ostream& os, MaglevGraphLabeller* graph_labeller) const {
7681-
os << "(" << value() << ", " << DeoptimizeReasonToString(deoptimize_reason())
7682-
<< ")";
7652+
os << "(" << value().get_scalar() << ", "
7653+
<< DeoptimizeReasonToString(deoptimize_reason()) << ")";
76837654
}
76847655

76857656
void CheckValueEqualsString::PrintParams(

‎src/maglev/maglev-ir.h‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ class ExceptionHandlerInfo;
328328
V(CheckHeapObject) \
329329
V(CheckInt32Condition) \
330330
V(CheckCacheIndicesNotCleared) \
331-
V(CheckFloat64IsNan) \
332331
V(CheckJSDataViewBounds) \
333332
V(CheckTypedArrayBounds) \
334333
V(CheckTypedArrayNotDetached) \
@@ -347,7 +346,7 @@ class ExceptionHandlerInfo;
347346
V(CheckSymbol) \
348347
V(CheckValue) \
349348
V(CheckValueEqualsInt32) \
350-
V(CheckValueEqualsFloat64) \
349+
V(CheckFloat64SameValue) \
351350
V(CheckValueEqualsString) \
352351
V(CheckInstanceType) \
353352
V(Dead) \
@@ -6663,22 +6662,19 @@ class CheckValueEqualsInt32 : public FixedInputNodeT<1, CheckValueEqualsInt32> {
66636662
const int32_t value_;
66646663
};
66656664

6666-
class CheckValueEqualsFloat64
6667-
: public FixedInputNodeT<1, CheckValueEqualsFloat64> {
6668-
using Base = FixedInputNodeT<1, CheckValueEqualsFloat64>;
6665+
class CheckFloat64SameValue : public FixedInputNodeT<1, CheckFloat64SameValue> {
6666+
using Base = FixedInputNodeT<1, CheckFloat64SameValue>;
66696667

66706668
public:
6671-
explicit CheckValueEqualsFloat64(uint64_t bitfield, Float64 value,
6672-
DeoptimizeReason reason)
6673-
: Base(bitfield | ReasonField::encode(reason)), value_(value) {
6674-
DCHECK(!value.is_nan());
6675-
}
6669+
explicit CheckFloat64SameValue(uint64_t bitfield, Float64 value,
6670+
DeoptimizeReason reason)
6671+
: Base(bitfield | ReasonField::encode(reason)), value_(value) {}
66766672

66776673
static constexpr OpProperties kProperties = OpProperties::EagerDeopt();
66786674
static constexpr
66796675
typename Base::InputTypes kInputTypes{ValueRepresentation::kFloat64};
66806676

6681-
double value() const { return value_.get_scalar(); }
6677+
Float64 value() const { return value_; }
66826678

66836679
static constexpr int kTargetIndex = 0;
66846680
Input& target_input() { return input(kTargetIndex); }
@@ -6695,29 +6691,6 @@ class CheckValueEqualsFloat64
66956691
const Float64 value_;
66966692
};
66976693

6698-
class CheckFloat64IsNan : public FixedInputNodeT<1, CheckFloat64IsNan> {
6699-
using Base = FixedInputNodeT<1, CheckFloat64IsNan>;
6700-
6701-
public:
6702-
explicit CheckFloat64IsNan(uint64_t bitfield, DeoptimizeReason reason)
6703-
: Base(bitfield | ReasonField::encode(reason)) {}
6704-
6705-
static constexpr OpProperties kProperties = OpProperties::EagerDeopt();
6706-
static constexpr
6707-
typename Base::InputTypes kInputTypes{ValueRepresentation::kFloat64};
6708-
6709-
static constexpr int kTargetIndex = 0;
6710-
Input& target_input() { return input(kTargetIndex); }
6711-
6712-
void SetValueLocationConstraints();
6713-
void GenerateCode(MaglevAssembler*, const ProcessingState&);
6714-
void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
6715-
6716-
auto options() const { return std::tuple{deoptimize_reason()}; }
6717-
6718-
DEOPTIMIZE_REASON_FIELD
6719-
};
6720-
67216694
class CheckValueEqualsString
67226695
: public FixedInputNodeT<1, CheckValueEqualsString> {
67236696
using Base = FixedInputNodeT<1, CheckValueEqualsString>;

‎src/maglev/x64/maglev-ir-x64.cc‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,29 @@ void CheckedIntPtrToInt32::GenerateCode(MaglevAssembler* masm,
136136
__ EmitEagerDeoptIf(not_equal, DeoptimizeReason::kNotInt32, this);
137137
}
138138

139+
void CheckFloat64SameValue::SetValueLocationConstraints() {
140+
UseRegister(target_input());
141+
}
142+
void CheckFloat64SameValue::GenerateCode(MaglevAssembler* masm,
143+
const ProcessingState& state) {
144+
Label* fail = __ GetDeoptLabel(this, deoptimize_reason());
145+
MaglevAssembler::TemporaryRegisterScope temps(masm);
146+
DoubleRegister double_scratch = temps.AcquireScratchDouble();
147+
DoubleRegister target = ToDoubleRegister(target_input());
148+
if (value().is_nan()) {
149+
__ JumpIfNotNan(target, fail);
150+
} else {
151+
__ Move(double_scratch, value());
152+
__ CompareFloat64AndJumpIf(double_scratch, target, kNotEqual, fail, fail);
153+
if (value().get_scalar() == 0) { // If value is +0.0 or -0.0.
154+
Register scratch = temps.AcquireScratch();
155+
__ movq(scratch, target);
156+
__ testq(scratch, scratch);
157+
__ JumpIf(value().get_bits() == 0 ? kNotEqual : kEqual, fail);
158+
}
159+
}
160+
}
161+
139162
int BuiltinStringFromCharCode::MaxCallStackArgs() const {
140163
return AllocateDescriptor::GetStackParameterCount();
141164
}

‎test/mjsunit/maglev/regress-406043356.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright 2025 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 --maglev
6+
7+
(function() {
8+
function foo(a) {
9+
v = a % a;
10+
return 1 / v;
11+
}
12+
%PrepareFunctionForOptimization(foo);
13+
// Ensures global {v} has constant cell -0.
14+
foo(-1);
15+
foo(-1);
16+
// Ensures we deopt when comparing 0 to -0.
17+
%OptimizeFunctionOnNextCall(foo);
18+
assertEquals(Infinity, foo(1));
19+
assertFalse(isOptimized(foo));
20+
})();
21+
22+
(function() {
23+
function foo(a) {
24+
w = a % a;
25+
return 1 / w;
26+
}
27+
%PrepareFunctionForOptimization(foo);
28+
// Ensures global {w} has constant cell 0.
29+
foo(1);
30+
foo(1);
31+
// Ensures we deopt when comparing 0 to -0.
32+
%OptimizeFunctionOnNextCall(foo);
33+
assertEquals(-Infinity, foo(-1));
34+
assertFalse(isOptimized(foo));
35+
})();
36+
37+
(function() {
38+
let ab = new ArrayBuffer(16);
39+
let int32view = new Int32Array(ab);
40+
int32view[1] = 0x7FFF_FFFF;
41+
int32view[0] = 0x0000_0000;
42+
int32view[3] = 0x7FFF_FFFF;
43+
int32view[2] = 0x0000_0001;
44+
let float64view = new Float64Array(ab);
45+
function foo(a) {
46+
vvv = a;
47+
}
48+
%PrepareFunctionForOptimization(foo);
49+
foo(float64view[0]);
50+
foo(float64view[0]);
51+
52+
%OptimizeFunctionOnNextCall(foo);
53+
// Ensures we don't deopt if we use a different NaN bit
54+
// representation.
55+
foo(float64view[1]);
56+
assertTrue(isOptimized(foo));
57+
})();

0 commit comments

Comments
 (0)