| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
lgtm
auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(newline
return ReplaceWith(reducer_.GetBooleanConstant(newline
return BuildCallBuiltinWithTaggedInputs<Builtin::kSameValue>({lhs, rhs});Would be nice to have a high-level node for this so that escape analysis can constant-fold when one side is an elided object but the other side isn't (this should always produce false, right?).
CheckType(rhs, NodeType::kString)) {same
} else if (CheckType(lhs, NodeType::kString) &&same
} else if (CheckType(lhs, kReferenceEqual) ||same
if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {`NodeTypeIs(lhs_type`
if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {`NodeTypeIs(rhs_type`
ValueNode* int32_lhs = TryGetInt32(lhs);DCHECK that the type of the inputs is Number here maybe.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Commit-Queue | +2 |
Done (using CheckType, which is side-effect free, so it's DCHECK-safe).
Added a TODO. And yes: a fresh (elided) allocation is only SameValue to that very object, so pairing it with any distinct object or non-object always yields false — a high-level node would let escape analysis fold exactly that case. Left it as a follow-up to keep this CL focused.
auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(Victor Gomesnewline
Done
return ReplaceWith(reducer_.GetBooleanConstant(Victor Gomesnewline
Done
return BuildCallBuiltinWithTaggedInputs<Builtin::kSameValue>({lhs, rhs});Would be nice to have a high-level node for this so that escape analysis can constant-fold when one side is an elided object but the other side isn't (this should always produce false, right?).
I mean you could just inspect CallBuiltin<kSameValue>, no?
Similar argument might be true for other builtins.
CheckType(rhs, NodeType::kString)) {Victor Gomessame
Done
} else if (CheckType(lhs, NodeType::kString) &&Victor Gomessame
Done
CheckType(rhs, kReferenceEqual)) {Victor Gomessame
Done
} else if (CheckType(lhs, kReferenceEqual) ||Victor Gomessame
Done
if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {Victor Gomes`NodeTypeIs(lhs_type`
Done
if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {Victor Gomes`NodeTypeIs(rhs_type`
Done
DCHECK that the type of the inputs is Number here maybe.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
📍 Job mac-m4-mini-perf/jetstream-main.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/16b34596290000
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/maglev/maglev-reducer-inl.h
Insertions: 8, Deletions: 5.
@@ -2716,6 +2716,8 @@
template <typename BaseT>
ReduceResult MaglevReducer<BaseT>::BuildNumberSameValue(ValueNode* lhs,
ValueNode* rhs) {
+ DCHECK(CheckType(lhs, NodeType::kNumber));
+ DCHECK(CheckType(rhs, NodeType::kNumber));
ValueNode* int32_lhs = TryGetInt32(lhs);
ValueNode* int32_rhs = TryGetInt32(rhs);
if (int32_lhs != nullptr && int32_rhs != nullptr) {
@@ -2766,13 +2768,14 @@
const NodeType kReferenceEqual = UnionType(
NodeType::kJSReceiver, UnionType(NodeType::kSymbol, NodeType::kOddball));
- if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
+ if (NodeTypeIs(lhs_type, NodeType::kNumber) &&
+ NodeTypeIs(rhs_type, NodeType::kNumber)) {
return BuildNumberSameValue(lhs, rhs);
- } else if (CheckType(lhs, kReferenceEqual) ||
- CheckType(rhs, kReferenceEqual)) {
+ } else if (NodeTypeIs(lhs_type, kReferenceEqual) ||
+ NodeTypeIs(rhs_type, kReferenceEqual)) {
return BuildTaggedEqual(lhs, rhs);
- } else if (CheckType(lhs, NodeType::kString) &&
- CheckType(rhs, NodeType::kString)) {
+ } else if (NodeTypeIs(lhs_type, NodeType::kString) &&
+ NodeTypeIs(rhs_type, NodeType::kString)) {
return AddNewNode<StringEqual>({lhs, rhs},
StringEqualInputMode::kOnlyStrings);
}
@@ -4703,11 +4706,11 @@
template <typename BaseT>
ReduceResult MaglevReducer<BaseT>::BuildLoadJSDataViewByteLength(
ValueNode* js_data_view) {
+ bool is_const = !v8_flags.track_array_buffer_views;
// Note: We can't use broker()->byte_length_string() here, because it could
// conflict with redefinitions of the ArrayBufferView byteLength property.
- if (ValueNode* byte_length =
- known_node_aspects().TryFindLoadedConstantProperty(
- js_data_view, PropertyKey::ArrayBufferViewByteLength())) {
+ if (ValueNode* byte_length = known_node_aspects().TryFindLoadedProperty(
+ js_data_view, PropertyKey::ArrayBufferViewByteLength(), is_const)) {
return byte_length;
}
@@ -4715,16 +4718,16 @@
GET_VALUE_OR_ABORT(result,
AddNewNode<LoadDataViewByteLength>({js_data_view}));
RecordKnownProperty(js_data_view, PropertyKey::ArrayBufferViewByteLength(),
- result, true, compiler::AccessMode::kLoad);
+ result, is_const, compiler::AccessMode::kLoad);
return result;
}
template <typename BaseT>
ReduceResult MaglevReducer<BaseT>::BuildLoadJSDataViewDataPointer(
ValueNode* js_data_view) {
- if (ValueNode* backing_store =
- known_node_aspects().TryFindLoadedConstantProperty(
- js_data_view, PropertyKey::ArrayBufferViewDataPointer())) {
+ bool is_const = !v8_flags.track_array_buffer_views;
+ if (ValueNode* backing_store = known_node_aspects().TryFindLoadedProperty(
+ js_data_view, PropertyKey::ArrayBufferViewDataPointer(), is_const)) {
return backing_store;
}
@@ -4732,7 +4735,7 @@
GET_VALUE_OR_ABORT(result,
AddNewNode<LoadDataViewDataPointer>({js_data_view}));
RecordKnownProperty(js_data_view, PropertyKey::ArrayBufferViewDataPointer(),
- result, true, compiler::AccessMode::kLoad);
+ result, is_const, compiler::AccessMode::kLoad);
return result;
}
```
```
The name of the file: test/mjsunit/maglev/object-is.js
Insertions: 22, Deletions: 13.
@@ -72,11 +72,12 @@
(function() {
function smiZero(o) { return Object.is(+o, 0); }
function int32Zero(a, b) { return Object.is(+a, b | 0); }
- function constants() {
- return [Object.is(-0, 0), Object.is(0, -0), Object.is(-0, 0.0),
- Object.is(-0, Math.max(-0.0, 0.0)), Object.is(-0, Math.min(0.0, 0.0)),
- Object.is(-0, Math.min(-0.0, 0.0))];
- }
+ // Both operands constant: a zero constant must not fold via numeric equality.
+ // Math.max(-0, 0) is +0, Math.min(-0, 0) is -0.
+ function negZeroVsZero() { return Object.is(-0, 0); }
+ function zeroVsNegZero() { return Object.is(0, -0); }
+ function negZeroVsMax() { return Object.is(-0, Math.max(-0.0, 0.0)); }
+ function negZeroVsMin() { return Object.is(-0, Math.min(-0.0, 0.0)); }
testOptimized(() => {
assertFalse(smiZero(-0));
assertTrue(smiZero(0));
@@ -84,8 +85,11 @@
assertFalse(int32Zero(-0, 0));
assertTrue(int32Zero(0, 0));
assertFalse(int32Zero(NaN, 0));
- assertEquals([false, false, false, false, false, true], constants());
- }, smiZero, int32Zero, constants);
+ assertFalse(negZeroVsZero());
+ assertFalse(zeroVsNegZero());
+ assertFalse(negZeroVsMax());
+ assertTrue(negZeroVsMin());
+ }, smiZero, int32Zero, negZeroVsZero, zeroVsNegZero, negZeroVsMax, negZeroVsMin);
})();
// The full Float64 same-value comparison.
@@ -130,13 +134,18 @@
// Constant folding of two number constants.
(function() {
- function foo() {
- return [Object.is(NaN, NaN), Object.is(0, -0), Object.is(-0, -0),
- Object.is(1, 1.0), Object.is(1, 2)];
- }
+ function nanNan() { return Object.is(NaN, NaN); }
+ function zeroNegZero() { return Object.is(0, -0); }
+ function negZeroNegZero() { return Object.is(-0, -0); }
+ function oneOne() { return Object.is(1, 1.0); }
+ function oneTwo() { return Object.is(1, 2); }
testOptimized(() => {
- assertEquals([true, false, true, true, false], foo());
- }, foo);
+ assertTrue(nanNan());
+ assertFalse(zeroNegZero());
+ assertTrue(negZeroNegZero());
+ assertTrue(oneOne());
+ assertFalse(oneTwo());
+ }, nanNan, zeroNegZero, negZeroNegZero, oneOne, oneTwo);
})();
// JSReceivers, symbols and oddballs compare by reference.
```
```
The name of the file: src/maglev/maglev-graph-optimizer.cc
Insertions: 2, Deletions: 0.
@@ -3190,10 +3190,12 @@
UseRepresentation::kFloat64, node->input_node(0),
TaggedToFloat64ConversionType::kOnlyNumber);
if (!left) return ProcessResult::kContinue;
+
auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(
UseRepresentation::kFloat64, node->input_node(1),
TaggedToFloat64ConversionType::kOnlyNumber);
if (!right) return ProcessResult::kContinue;
+
return ReplaceWith(reducer_.GetBooleanConstant(
Object::SameNumberValue(left->get_scalar(), right->get_scalar())));
}
```
[maglev] Inline Object.is
Adds a Float64SameValue node for the general two-numbers case; the
remaining shapes reuse existing comparison nodes.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |