From ed573cee5c1d1e42158829dc0b92fb697234e121 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Mon, 24 Apr 2017 21:58:02 -0700 Subject: [PATCH] [turbofan] General consolidation of element access. Avoid TransitionElementsKind when storing to objects which only differ in holeyness of their elements kind. Instead go for polymorphic CheckMaps, which can often by optimized and avoid the mutation of the array map. This generalizes the approach https://codereview.chromium.org/2836943003 which covered only element loads. R=yangguo@chromium.org BUG=v8:5267 Review-Url: https://codereview.chromium.org/2836913004 Cr-Commit-Position: refs/heads/master@{#44828} --- src/compiler/access-info.cc | 74 ++++++++++++++++++------------------- src/compiler/access-info.h | 5 ++- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/compiler/access-info.cc b/src/compiler/access-info.cc index bbbf726773..3862d86421 100644 --- a/src/compiler/access-info.cc +++ b/src/compiler/access-info.cc @@ -206,16 +206,20 @@ bool AccessInfoFactory::ComputeElementAccessInfo( bool AccessInfoFactory::ComputeElementAccessInfos( MapHandleList const& maps, AccessMode access_mode, ZoneVector* access_infos) { - if (access_mode == AccessMode::kLoad) { - // For polymorphic loads of similar elements kinds (i.e. all tagged or all - // double), always use the "worst case" code without a transition. This is - // much faster than transitioning the elements to the worst case, trading a - // TransitionElementsKind for a CheckMaps, avoiding mutation of the array. - ElementAccessInfo access_info; - if (ConsolidateElementLoad(maps, &access_info)) { - access_infos->push_back(access_info); - return true; - } + // For polymorphic loads of similar elements kinds (i.e. all tagged or all + // double), always use the "worst case" code without a transition. This is + // much faster than transitioning the elements to the worst case, trading a + // TransitionElementsKind for a CheckMaps, avoiding mutation of the array. + // + // Similarly, for polymorphic stores of compatible elements kind that + // differ only in holeyness, always use the "holey case" code without a + // transition. This is beneficial, because CheckMaps can often be optimized + // whereas TransitionElementsKind can block optimizations. And as above, we + // avoid mutation of the array (we still mutate the array contents). + ElementAccessInfo access_info; + if (ConsolidateElementAccess(maps, access_mode, &access_info)) { + access_infos->push_back(access_info); + return true; } // Collect possible transition targets. @@ -475,32 +479,9 @@ bool AccessInfoFactory::ComputePropertyAccessInfos( return true; } -namespace { - -Maybe GeneralizeElementsKind(ElementsKind this_kind, - ElementsKind that_kind) { - if (IsHoleyElementsKind(this_kind)) { - that_kind = GetHoleyElementsKind(that_kind); - } else if (IsHoleyElementsKind(that_kind)) { - this_kind = GetHoleyElementsKind(this_kind); - } - if (this_kind == that_kind) return Just(this_kind); - if (IsFastDoubleElementsKind(that_kind) == - IsFastDoubleElementsKind(this_kind)) { - if (IsMoreGeneralElementsKindTransition(that_kind, this_kind)) { - return Just(this_kind); - } - if (IsMoreGeneralElementsKindTransition(this_kind, that_kind)) { - return Just(that_kind); - } - } - return Nothing(); -} - -} // namespace - -bool AccessInfoFactory::ConsolidateElementLoad(MapHandleList const& maps, - ElementAccessInfo* access_info) { +bool AccessInfoFactory::ConsolidateElementAccess( + MapHandleList const& maps, AccessMode access_mode, + ElementAccessInfo* access_info) { if (maps.is_empty()) return false; InstanceType instance_type = maps.first()->instance_type(); ElementsKind elements_kind = maps.first()->elements_kind(); @@ -510,9 +491,24 @@ bool AccessInfoFactory::ConsolidateElementLoad(MapHandleList const& maps, if (!CanInlineElementAccess(map) || map->instance_type() != instance_type) { return false; } - if (!GeneralizeElementsKind(elements_kind, map->elements_kind()) - .To(&elements_kind)) { - return false; + ElementsKind other_kind = map->elements_kind(); + if (IsHoleyElementsKind(elements_kind)) { + other_kind = GetHoleyElementsKind(other_kind); + } else if (IsHoleyElementsKind(other_kind)) { + elements_kind = GetHoleyElementsKind(elements_kind); + } + if (elements_kind != other_kind) { + if (access_mode != AccessMode::kLoad) return false; + if (IsFastDoubleElementsKind(elements_kind) != + IsFastDoubleElementsKind(other_kind)) { + return false; + } + if (IsMoreGeneralElementsKindTransition(elements_kind, other_kind)) { + elements_kind = other_kind; + } else if (!IsMoreGeneralElementsKindTransition(other_kind, + elements_kind)) { + return false; + } } receiver_maps[i] = map; } diff --git a/src/compiler/access-info.h b/src/compiler/access-info.h index f0a671c315..861896e1c6 100644 --- a/src/compiler/access-info.h +++ b/src/compiler/access-info.h @@ -149,8 +149,9 @@ class AccessInfoFactory final { ZoneVector* access_infos); private: - bool ConsolidateElementLoad(MapHandleList const& maps, - ElementAccessInfo* access_info); + bool ConsolidateElementAccess(MapHandleList const& maps, + AccessMode access_mode, + ElementAccessInfo* access_info); bool LookupSpecialFieldAccessor(Handle map, Handle name, PropertyAccessInfo* access_info); bool LookupTransition(Handle map, Handle name,