[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

@@ -206,16 +206,20 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
 bool AccessInfoFactory::ComputeElementAccessInfos(
     MapHandleList const& maps, AccessMode access_mode,
     ZoneVector<ElementAccessInfo>* 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<ElementsKind> 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<ElementsKind>();
-}
-
-}  // 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;
   }

src/compiler/access-info.h

@@ -149,8 +149,9 @@ class AccessInfoFactory final {
                                   ZoneVector<PropertyAccessInfo>* 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> map, Handle<Name> name,
                                   PropertyAccessInfo* access_info);
   bool LookupTransition(Handle<Map> map, Handle<Name> name,