From 994c2a575d05936958d8c5ba22ed2afcd4eb042b Mon Sep 17 00:00:00 2001 From: Nico Hartmann Date: Thu, 2 Feb 2023 15:01:59 +0000 Subject: [PATCH] Revert "[maglev] Check for strings in polymorphic loads" This reverts commit 7f4a04671a33d119c460e6e6eac14bf6fa17c8a6. Reason for revert: Speculative revert for https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20no-concurrent-marking/13086/overview 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 > Commit-Queue: Leszek Swirski > Auto-Submit: Victor Gomes > Cr-Commit-Position: refs/heads/main@{#85626} Bug: v8:7700 Change-Id: I87473a0cef092d457391d84c051becf06014703b No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4219091 Owners-Override: Nico Hartmann Bot-Commit: Rubber Stamper Auto-Submit: Nico Hartmann Commit-Queue: Nico Hartmann Cr-Commit-Position: refs/heads/main@{#85627} --- src/maglev/arm64/maglev-assembler-arm64-inl.h | 14 -------- src/maglev/maglev-assembler.h | 6 ---- src/maglev/maglev-graph-builder.cc | 17 ++++++++++ src/maglev/maglev-ir.cc | 33 ++++++++++--------- src/maglev/maglev-ir.h | 14 -------- src/maglev/x64/maglev-assembler-x64-inl.h | 11 ------- 6 files changed, 34 insertions(+), 61 deletions(-) diff --git a/src/maglev/arm64/maglev-assembler-arm64-inl.h b/src/maglev/arm64/maglev-assembler-arm64-inl.h index 49ade36a2d..5c8f006d51 100644 --- a/src/maglev/arm64/maglev-assembler-arm64-inl.h +++ b/src/maglev/arm64/maglev-assembler-arm64-inl.h @@ -519,20 +519,6 @@ inline void MaglevAssembler::CompareObjectTypeRange(Register heap_object, 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, Handle obj) { ScratchRegisterScope temps(this); diff --git a/src/maglev/maglev-assembler.h b/src/maglev/maglev-assembler.h index c1deddf13c..7c6cf74395 100644 --- a/src/maglev/maglev-assembler.h +++ b/src/maglev/maglev-assembler.h @@ -201,12 +201,6 @@ class MaglevAssembler : public MacroAssembler { InstanceType lower_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 obj); inline void CompareInt32(Register reg, int32_t imm); diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc index 134a80bd73..fe35cd0e79 100644 --- a/src/maglev/maglev-graph-builder.cc +++ b/src/maglev/maglev-graph-builder.cc @@ -2005,6 +2005,23 @@ bool MaglevGraphBuilder::TryBuildPropertyAccess( } } +namespace { +bool HasOnlyStringMaps(base::Vector maps) { + for (compiler::MapRef map : maps) { + if (!map.IsStringMap()) return false; + } + return true; +} + +bool HasOnlyNumberMaps(base::Vector maps) { + for (compiler::MapRef map : maps) { + if (map.instance_type() != HEAP_NUMBER_TYPE) return false; + } + return true; +} + +} // namespace + bool MaglevGraphBuilder::TryBuildNamedAccess( ValueNode* receiver, ValueNode* lookup_start_object, compiler::NamedAccessFeedback const& feedback, diff --git a/src/maglev/maglev-ir.cc b/src/maglev/maglev-ir.cc index 79a189da92..1b552f8e37 100644 --- a/src/maglev/maglev-ir.cc +++ b/src/maglev/maglev-ir.cc @@ -988,25 +988,26 @@ void EmitPolymorphicAccesses(MaglevAssembler* masm, NodeT* node, for (const PolymorphicAccessInfo& access_info : node->access_infos()) { Label next; Label map_found; - auto& maps = access_info.maps(); + bool has_heap_number_map = false; - if (HasOnlyNumberMaps(base::VectorOf(maps))) { + for (auto it = access_info.maps().begin(); it != access_info.maps().end(); + ++it) { + if (it->IsHeapNumberMap()) { + has_heap_number_map = true; + } + __ 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()); __ bind(&is_number); - } else if (HasOnlyStringMaps(base::VectorOf(maps))) { - __ CompareInstanceTypeRange(object, 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); diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h index 5781ca2d16..d02b9ee4fe 100644 --- a/src/maglev/maglev-ir.h +++ b/src/maglev/maglev-ir.h @@ -463,20 +463,6 @@ inline std::ostream& operator<<(std::ostream& os, return os; } -inline bool HasOnlyStringMaps(base::Vector maps) { - for (compiler::MapRef map : maps) { - if (!map.IsStringMap()) return false; - } - return true; -} - -inline bool HasOnlyNumberMaps(base::Vector maps) { - for (compiler::MapRef map : maps) { - if (map.instance_type() != HEAP_NUMBER_TYPE) return false; - } - return true; -} - #define DEF_FORWARD_DECLARATION(type, ...) class type; NODE_BASE_LIST(DEF_FORWARD_DECLARATION) #undef DEF_FORWARD_DECLARATION diff --git a/src/maglev/x64/maglev-assembler-x64-inl.h b/src/maglev/x64/maglev-assembler-x64-inl.h index 390ea8ab96..b4104040d9 100644 --- a/src/maglev/x64/maglev-assembler-x64-inl.h +++ b/src/maglev/x64/maglev-assembler-x64-inl.h @@ -405,17 +405,6 @@ inline void MaglevAssembler::CompareObjectTypeRange(Register heap_object, 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, Handle obj) { Cmp(reg, obj);