Reland "[maglev] Check for strings in polymorphic loads"
This is a reland of commit 7f4a04671a
- Add heap number check.
- Use object_map in range instance check.
Original change's description:
> [maglev] Check for strings in polymorphic loads
>
> Bug: v8:7700
> Change-Id: Id3d523446f5061a78a46d1c52cf8f8339566356d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4212402
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Auto-Submit: Victor Gomes <victorgomes@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#85626}
Bug: v8:7700
Change-Id: I72cfe2e2bf19141dffbb8df5c34600eca4d70594
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4218508
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85642}
This commit is contained in:
parent
a6966097a0
commit
49f1450b3f
@ -519,6 +519,20 @@ inline void MaglevAssembler::CompareObjectTypeRange(Register heap_object,
|
|||||||
CompareInstanceTypeRange(scratch, scratch, lower_limit, higher_limit);
|
CompareInstanceTypeRange(scratch, scratch, lower_limit, higher_limit);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
inline void MaglevAssembler::CompareInstanceTypeRange(
|
||||||
|
Register map, InstanceType lower_limit, InstanceType higher_limit) {
|
||||||
|
ScratchRegisterScope temps(this);
|
||||||
|
Register scratch = temps.Acquire();
|
||||||
|
CompareInstanceTypeRange(map, scratch, lower_limit, higher_limit);
|
||||||
|
}
|
||||||
|
|
||||||
|
inline void MaglevAssembler::CompareInstanceTypeRange(
|
||||||
|
Register map, Register instance_type_out, InstanceType lower_limit,
|
||||||
|
InstanceType higher_limit) {
|
||||||
|
MacroAssembler::CompareInstanceTypeRange(map, instance_type_out, lower_limit,
|
||||||
|
higher_limit);
|
||||||
|
}
|
||||||
|
|
||||||
inline void MaglevAssembler::CompareTagged(Register reg,
|
inline void MaglevAssembler::CompareTagged(Register reg,
|
||||||
Handle<HeapObject> obj) {
|
Handle<HeapObject> obj) {
|
||||||
ScratchRegisterScope temps(this);
|
ScratchRegisterScope temps(this);
|
||||||
|
@ -199,6 +199,12 @@ class MaglevAssembler : public MacroAssembler {
|
|||||||
InstanceType lower_limit,
|
InstanceType lower_limit,
|
||||||
InstanceType higher_limit);
|
InstanceType higher_limit);
|
||||||
|
|
||||||
|
inline void CompareInstanceTypeRange(Register map, InstanceType lower_limit,
|
||||||
|
InstanceType higher_limit);
|
||||||
|
inline void CompareInstanceTypeRange(Register map, Register instance_type_out,
|
||||||
|
InstanceType lower_limit,
|
||||||
|
InstanceType higher_limit);
|
||||||
|
|
||||||
inline void CompareTagged(Register reg, Handle<HeapObject> obj);
|
inline void CompareTagged(Register reg, Handle<HeapObject> obj);
|
||||||
|
|
||||||
inline void CompareInt32(Register reg, int32_t imm);
|
inline void CompareInt32(Register reg, int32_t imm);
|
||||||
|
@ -2005,23 +2005,6 @@ bool MaglevGraphBuilder::TryBuildPropertyAccess(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
namespace {
|
|
||||||
bool HasOnlyStringMaps(base::Vector<const compiler::MapRef> maps) {
|
|
||||||
for (compiler::MapRef map : maps) {
|
|
||||||
if (!map.IsStringMap()) return false;
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
bool HasOnlyNumberMaps(base::Vector<const compiler::MapRef> maps) {
|
|
||||||
for (compiler::MapRef map : maps) {
|
|
||||||
if (map.instance_type() != HEAP_NUMBER_TYPE) return false;
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
} // namespace
|
|
||||||
|
|
||||||
bool MaglevGraphBuilder::TryBuildNamedAccess(
|
bool MaglevGraphBuilder::TryBuildNamedAccess(
|
||||||
ValueNode* receiver, ValueNode* lookup_start_object,
|
ValueNode* receiver, ValueNode* lookup_start_object,
|
||||||
compiler::NamedAccessFeedback const& feedback,
|
compiler::NamedAccessFeedback const& feedback,
|
||||||
|
@ -988,26 +988,29 @@ void EmitPolymorphicAccesses(MaglevAssembler* masm, NodeT* node,
|
|||||||
for (const PolymorphicAccessInfo& access_info : node->access_infos()) {
|
for (const PolymorphicAccessInfo& access_info : node->access_infos()) {
|
||||||
Label next;
|
Label next;
|
||||||
Label map_found;
|
Label map_found;
|
||||||
bool has_heap_number_map = false;
|
auto& maps = access_info.maps();
|
||||||
|
|
||||||
for (auto it = access_info.maps().begin(); it != access_info.maps().end();
|
if (HasOnlyNumberMaps(base::VectorOf(maps))) {
|
||||||
++it) {
|
__ CompareRoot(object_map, RootIndex::kHeapNumberMap);
|
||||||
if (it->IsHeapNumberMap()) {
|
__ JumpIf(kNotEqual, &next);
|
||||||
has_heap_number_map = true;
|
// Fallthrough... to map_found.
|
||||||
}
|
|
||||||
__ CompareTagged(object_map, it->object());
|
|
||||||
if (it == access_info.maps().end() - 1) {
|
|
||||||
__ JumpIf(kNotEqual, &next);
|
|
||||||
// Fallthrough... to map_found.
|
|
||||||
} else {
|
|
||||||
__ JumpIf(kEqual, &map_found);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Bind number case here if one of the maps is HeapNumber.
|
|
||||||
if (has_heap_number_map) {
|
|
||||||
DCHECK(!is_number.is_bound());
|
DCHECK(!is_number.is_bound());
|
||||||
__ bind(&is_number);
|
__ bind(&is_number);
|
||||||
|
} else if (HasOnlyStringMaps(base::VectorOf(maps))) {
|
||||||
|
__ CompareInstanceTypeRange(object_map, FIRST_STRING_TYPE,
|
||||||
|
LAST_STRING_TYPE);
|
||||||
|
__ JumpIf(kUnsignedGreaterThan, &next);
|
||||||
|
// Fallthrough... to map_found.
|
||||||
|
} else {
|
||||||
|
for (auto it = maps.begin(); it != maps.end(); ++it) {
|
||||||
|
__ CompareTagged(object_map, it->object());
|
||||||
|
if (it == maps.end() - 1) {
|
||||||
|
__ JumpIf(kNotEqual, &next);
|
||||||
|
// Fallthrough... to map_found.
|
||||||
|
} else {
|
||||||
|
__ JumpIf(kEqual, &map_found);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
__ bind(&map_found);
|
__ bind(&map_found);
|
||||||
|
@ -463,6 +463,20 @@ inline std::ostream& operator<<(std::ostream& os,
|
|||||||
return os;
|
return os;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
inline bool HasOnlyStringMaps(base::Vector<const compiler::MapRef> maps) {
|
||||||
|
for (compiler::MapRef map : maps) {
|
||||||
|
if (!map.IsStringMap()) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
inline bool HasOnlyNumberMaps(base::Vector<const compiler::MapRef> maps) {
|
||||||
|
for (compiler::MapRef map : maps) {
|
||||||
|
if (map.instance_type() != HEAP_NUMBER_TYPE) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
#define DEF_FORWARD_DECLARATION(type, ...) class type;
|
#define DEF_FORWARD_DECLARATION(type, ...) class type;
|
||||||
NODE_BASE_LIST(DEF_FORWARD_DECLARATION)
|
NODE_BASE_LIST(DEF_FORWARD_DECLARATION)
|
||||||
#undef DEF_FORWARD_DECLARATION
|
#undef DEF_FORWARD_DECLARATION
|
||||||
|
@ -405,6 +405,17 @@ inline void MaglevAssembler::CompareObjectTypeRange(Register heap_object,
|
|||||||
higher_limit);
|
higher_limit);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
inline void MaglevAssembler::CompareInstanceTypeRange(
|
||||||
|
Register map, InstanceType lower_limit, InstanceType higher_limit) {
|
||||||
|
CompareInstanceTypeRange(map, kScratchRegister, lower_limit, higher_limit);
|
||||||
|
}
|
||||||
|
|
||||||
|
inline void MaglevAssembler::CompareInstanceTypeRange(
|
||||||
|
Register map, Register instance_type_out, InstanceType lower_limit,
|
||||||
|
InstanceType higher_limit) {
|
||||||
|
CmpInstanceTypeRange(map, instance_type_out, lower_limit, higher_limit);
|
||||||
|
}
|
||||||
|
|
||||||
inline void MaglevAssembler::CompareTagged(Register reg,
|
inline void MaglevAssembler::CompareTagged(Register reg,
|
||||||
Handle<HeapObject> obj) {
|
Handle<HeapObject> obj) {
|
||||||
Cmp(reg, obj);
|
Cmp(reg, obj);
|
||||||
|
18
test/mjsunit/maglev/polymorphic-load-number.js
Normal file
18
test/mjsunit/maglev/polymorphic-load-number.js
Normal file
@ -0,0 +1,18 @@
|
|||||||
|
// Copyright 2023 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 --maglev
|
||||||
|
|
||||||
|
|
||||||
|
function foo(o) {
|
||||||
|
return o.length;
|
||||||
|
}
|
||||||
|
|
||||||
|
%PrepareFunctionForOptimization(foo);
|
||||||
|
assertEquals(6, foo("string"));
|
||||||
|
assertEquals(undefined, foo(4.2));
|
||||||
|
|
||||||
|
%OptimizeMaglevOnNextCall(foo);
|
||||||
|
assertEquals(6, foo("string"));
|
||||||
|
assertEquals(undefined, foo(4.2));
|
Loading…
Reference in New Issue
Block a user