[turbofan] Refactor the lowering of element/property accesses.

Split the monster methods in JSNativeContextSpecialization into smaller
ones, adding appropriate helpers. Improve the condition checking for
strings and numbers using CheckString/CheckNumber when applicable. Also
try to merge compatible PropertyAccessInfos, to avoid running into the
polymorphic case whenever possible.

Drive-by-fix: Don't try to resurrect dead nodes during LoadElimination.
With the improve code generation for monomorphic loads, we seem to
trigger the dead node resurrection.

R=epertoso@chromium.org
BUG=v8:4930,v8:5141

Review-Url: https://codereview.chromium.org/2191823002
Cr-Commit-Position: refs/heads/master@{#38127}

src/compiler/access-info.cc

@@ -56,59 +56,51 @@ std::ostream& operator<<(std::ostream& os, AccessMode access_mode) {
   return os;
 }
 
+ElementAccessInfo::ElementAccessInfo() {}
+
+ElementAccessInfo::ElementAccessInfo(MapList const& receiver_maps,
+                                     ElementsKind elements_kind,
+                                     MaybeHandle<JSObject> holder)
+    : elements_kind_(elements_kind),
+      holder_(holder),
+      receiver_maps_(receiver_maps) {}
 
 // static
-PropertyAccessInfo PropertyAccessInfo::NotFound(Type* receiver_type,
+PropertyAccessInfo PropertyAccessInfo::NotFound(MapList const& receiver_maps,
                                                 MaybeHandle<JSObject> holder) {
-  return PropertyAccessInfo(holder, receiver_type);
+  return PropertyAccessInfo(holder, receiver_maps);
 }
 
-
 // static
 PropertyAccessInfo PropertyAccessInfo::DataConstant(
-    Type* receiver_type, Handle<Object> constant,
+    MapList const& receiver_maps, Handle<Object> constant,
     MaybeHandle<JSObject> holder) {
-  return PropertyAccessInfo(holder, constant, receiver_type);
+  return PropertyAccessInfo(holder, constant, receiver_maps);
 }
 
-
 // static
 PropertyAccessInfo PropertyAccessInfo::DataField(
-    Type* receiver_type, FieldIndex field_index, Type* field_type,
+    MapList const& receiver_maps, FieldIndex field_index, Type* field_type,
     MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map) {
   return PropertyAccessInfo(holder, transition_map, field_index, field_type,
-                            receiver_type);
+                            receiver_maps);
 }
 
-
-ElementAccessInfo::ElementAccessInfo() : receiver_type_(Type::None()) {}
-
-
-ElementAccessInfo::ElementAccessInfo(Type* receiver_type,
-                                     ElementsKind elements_kind,
-                                     MaybeHandle<JSObject> holder)
-    : elements_kind_(elements_kind),
-      holder_(holder),
-      receiver_type_(receiver_type) {}
-
-
 PropertyAccessInfo::PropertyAccessInfo()
-    : kind_(kInvalid), receiver_type_(Type::None()), field_type_(Type::Any()) {}
-
+    : kind_(kInvalid), field_type_(Type::Any()) {}
 
 PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
-                                       Type* receiver_type)
+                                       MapList const& receiver_maps)
     : kind_(kNotFound),
-      receiver_type_(receiver_type),
+      receiver_maps_(receiver_maps),
       holder_(holder),
       field_type_(Type::Any()) {}
 
-
 PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
                                        Handle<Object> constant,
-                                       Type* receiver_type)
+                                       MapList const& receiver_maps)
     : kind_(kDataConstant),
-      receiver_type_(receiver_type),
+      receiver_maps_(receiver_maps),
       constant_(constant),
       holder_(holder),
       field_type_(Type::Any()) {}
@@ -116,14 +108,55 @@ PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
 PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
                                        MaybeHandle<Map> transition_map,
                                        FieldIndex field_index, Type* field_type,
-                                       Type* receiver_type)
+                                       MapList const& receiver_maps)
     : kind_(kDataField),
-      receiver_type_(receiver_type),
+      receiver_maps_(receiver_maps),
       transition_map_(transition_map),
       holder_(holder),
       field_index_(field_index),
       field_type_(field_type) {}
 
+bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that) {
+  if (this->kind_ != that->kind_) return false;
+  if (this->holder_.address() != that->holder_.address()) return false;
+
+  switch (this->kind_) {
+    case kInvalid:
+      break;
+
+    case kNotFound:
+      return true;
+
+    case kDataField: {
+      // Check if we actually access the same field.
+      if (this->transition_map_.address() == that->transition_map_.address() &&
+          this->field_index_ == that->field_index_ &&
+          this->field_type_->Is(that->field_type_) &&
+          that->field_type_->Is(this->field_type_)) {
+        this->receiver_maps_.insert(this->receiver_maps_.end(),
+                                    that->receiver_maps_.begin(),
+                                    that->receiver_maps_.end());
+        return true;
+      }
+      return false;
+    }
+
+    case kDataConstant: {
+      // Check if we actually access the same constant.
+      if (this->constant_.address() == that->constant_.address()) {
+        this->receiver_maps_.insert(this->receiver_maps_.end(),
+                                    that->receiver_maps_.begin(),
+                                    that->receiver_maps_.end());
+        return true;
+      }
+      return false;
+    }
+  }
+
+  UNREACHABLE();
+  return false;
+}
+
 AccessInfoFactory::AccessInfoFactory(CompilationDependencies* dependencies,
                                      Handle<Context> native_context, Zone* zone)
     : dependencies_(dependencies),
@@ -161,8 +194,7 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
     }
   }
 
-  *access_info =
-      ElementAccessInfo(Type::Class(map, zone()), elements_kind, holder);
+  *access_info = ElementAccessInfo(MapList{map}, elements_kind, holder);
   return true;
 }
 
@@ -258,7 +290,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
       }
       if (details.type() == DATA_CONSTANT) {
         *access_info = PropertyAccessInfo::DataConstant(
-            Type::Class(receiver_map, zone()),
+            MapList{receiver_map},
             handle(descriptors->GetValue(number), isolate()), holder);
         return true;
       } else if (details.type() == DATA) {
@@ -294,7 +326,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
           DCHECK(field_type->Is(Type::TaggedPointer()));
         }
         *access_info = PropertyAccessInfo::DataField(
-            Type::Class(receiver_map, zone()), field_index, field_type, holder);
+            MapList{receiver_map}, field_index, field_type, holder);
         return true;
       } else {
         // TODO(bmeurer): Add support for accessors.
@@ -331,8 +363,8 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
         // The property was not found, return undefined or throw depending
         // on the language mode of the load operation.
         // Implemented according to ES6 section 9.1.8 [[Get]] (P, Receiver)
-        *access_info = PropertyAccessInfo::NotFound(
-            Type::Class(receiver_map, zone()), holder);
+        *access_info =
+            PropertyAccessInfo::NotFound(MapList{receiver_map}, holder);
         return true;
       } else {
         return false;
@@ -350,7 +382,6 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
   return false;
 }
 
-
 bool AccessInfoFactory::ComputePropertyAccessInfos(
     MapHandleList const& maps, Handle<Name> name, AccessMode access_mode,
     ZoneVector<PropertyAccessInfo>* access_infos) {
@@ -360,7 +391,15 @@ bool AccessInfoFactory::ComputePropertyAccessInfos(
       if (!ComputePropertyAccessInfo(map, name, access_mode, &access_info)) {
         return false;
       }
-      access_infos->push_back(access_info);
+      // Try to merge the {access_info} with an existing one.
+      bool merged = false;
+      for (PropertyAccessInfo& other_info : *access_infos) {
+        if (other_info.Merge(&access_info)) {
+          merged = true;
+          break;
+        }
+      }
+      if (!merged) access_infos->push_back(access_info);
     }
   }
   return true;
@@ -394,8 +433,8 @@ bool AccessInfoFactory::LookupSpecialFieldAccessor(
         field_type = type_cache_.kJSArrayLengthType;
       }
     }
-    *access_info = PropertyAccessInfo::DataField(Type::Class(map, zone()),
-                                                 field_index, field_type);
+    *access_info =
+        PropertyAccessInfo::DataField(MapList{map}, field_index, field_type);
     return true;
   }
   return false;
@@ -445,9 +484,8 @@ bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
       DCHECK(field_type->Is(Type::TaggedPointer()));
     }
     dependencies()->AssumeMapNotDeprecated(transition_map);
-    *access_info =
-        PropertyAccessInfo::DataField(Type::Class(map, zone()), field_index,
-                                      field_type, holder, transition_map);
+    *access_info = PropertyAccessInfo::DataField(
+        MapList{map}, field_index, field_type, holder, transition_map);
     return true;
   }
   return false;

src/compiler/access-info.h

@@ -19,7 +19,6 @@ class CompilationDependencies;
 class Factory;
 class TypeCache;
 
-
 namespace compiler {
 
 // Whether we are loading a property or storing to a property.
@@ -27,50 +26,51 @@ enum class AccessMode { kLoad, kStore };
 
 std::ostream& operator<<(std::ostream&, AccessMode);
 
+typedef std::vector<Handle<Map>> MapList;
 
 // Mapping of transition source to transition target.
 typedef std::vector<std::pair<Handle<Map>, Handle<Map>>> MapTransitionList;
 
-
 // This class encapsulates all information required to access a certain element.
 class ElementAccessInfo final {
  public:
   ElementAccessInfo();
-  ElementAccessInfo(Type* receiver_type, ElementsKind elements_kind,
+  ElementAccessInfo(MapList const& receiver_maps, ElementsKind elements_kind,
                     MaybeHandle<JSObject> holder);
 
   MaybeHandle<JSObject> holder() const { return holder_; }
   ElementsKind elements_kind() const { return elements_kind_; }
-  Type* receiver_type() const { return receiver_type_; }
+  MapList const& receiver_maps() const { return receiver_maps_; }
   MapTransitionList& transitions() { return transitions_; }
   MapTransitionList const& transitions() const { return transitions_; }
 
  private:
   ElementsKind elements_kind_;
   MaybeHandle<JSObject> holder_;
-  Type* receiver_type_;
+  MapList receiver_maps_;
   MapTransitionList transitions_;
 };
 
-
 // This class encapsulates all information required to access a certain
 // object property, either on the object itself or on the prototype chain.
 class PropertyAccessInfo final {
  public:
   enum Kind { kInvalid, kNotFound, kDataConstant, kDataField };
 
-  static PropertyAccessInfo NotFound(Type* receiver_type,
+  static PropertyAccessInfo NotFound(MapList const& receiver_maps,
                                      MaybeHandle<JSObject> holder);
-  static PropertyAccessInfo DataConstant(Type* receiver_type,
+  static PropertyAccessInfo DataConstant(MapList const& receiver_maps,
                                          Handle<Object> constant,
                                          MaybeHandle<JSObject> holder);
   static PropertyAccessInfo DataField(
-      Type* receiver_type, FieldIndex field_index, Type* field_type,
+      MapList const& receiver_maps, FieldIndex field_index, Type* field_type,
       MaybeHandle<JSObject> holder = MaybeHandle<JSObject>(),
       MaybeHandle<Map> transition_map = MaybeHandle<Map>());
 
   PropertyAccessInfo();
 
+  bool Merge(PropertyAccessInfo const* that) WARN_UNUSED_RESULT;
+
   bool IsNotFound() const { return kind() == kNotFound; }
   bool IsDataConstant() const { return kind() == kDataConstant; }
   bool IsDataField() const { return kind() == kDataField; }
@@ -83,18 +83,19 @@ class PropertyAccessInfo final {
   Handle<Object> constant() const { return constant_; }
   FieldIndex field_index() const { return field_index_; }
   Type* field_type() const { return field_type_; }
-  Type* receiver_type() const { return receiver_type_; }
+  MapList const& receiver_maps() const { return receiver_maps_; }
 
  private:
-  PropertyAccessInfo(MaybeHandle<JSObject> holder, Type* receiver_type);
+  PropertyAccessInfo(MaybeHandle<JSObject> holder,
+                     MapList const& receiver_maps);
   PropertyAccessInfo(MaybeHandle<JSObject> holder, Handle<Object> constant,
-                     Type* receiver_type);
+                     MapList const& receiver_maps);
   PropertyAccessInfo(MaybeHandle<JSObject> holder,
                      MaybeHandle<Map> transition_map, FieldIndex field_index,
-                     Type* field_type, Type* receiver_type);
+                     Type* field_type, MapList const& receiver_maps);
 
   Kind kind_;
-  Type* receiver_type_;
+  MapList receiver_maps_;
   Handle<Object> constant_;
   MaybeHandle<Map> transition_map_;
   MaybeHandle<JSObject> holder_;

src/compiler/js-native-context-specialization.h

@@ -24,9 +24,11 @@ namespace compiler {
 // Forward declarations.
 enum class AccessMode;
 class CommonOperatorBuilder;
+class ElementAccessInfo;
 class JSGraph;
 class JSOperatorBuilder;
 class MachineOperatorBuilder;
+class PropertyAccessInfo;
 class SimplifiedOperatorBuilder;
 
 
@@ -80,9 +82,41 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
 
   Reduction ReduceSoftDeoptimize(Node* node, DeoptimizeReason reason);
 
+  // A triple of nodes that represents a continuation.
+  class ValueEffectControl final {
+   public:
+    ValueEffectControl(Node* value, Node* effect, Node* control)
+        : value_(value), effect_(effect), control_(control) {}
+
+    Node* value() const { return value_; }
+    Node* effect() const { return effect_; }
+    Node* control() const { return control_; }
+
+   private:
+    Node* const value_;
+    Node* const effect_;
+    Node* const control_;
+  };
+
+  // Construct the appropriate subgraph for property access.
+  ValueEffectControl BuildPropertyAccess(Node* receiver, Node* value,
+                                         Node* effect, Node* control,
+                                         Handle<Name> name,
+                                         Handle<Context> native_context,
+                                         PropertyAccessInfo const& access_info,
+                                         AccessMode access_mode);
+
+  // Construct the appropriate subgraph for element access.
+  ValueEffectControl BuildElementAccess(Node* receiver, Node* index,
+                                        Node* value, Node* effect,
+                                        Node* control,
+                                        Handle<Context> native_context,
+                                        ElementAccessInfo const& access_info,
+                                        AccessMode access_mode);
+
   // Adds stability dependencies on all prototypes of every class in
   // {receiver_type} up to (and including) the {holder}.
-  void AssumePrototypesStable(Type* receiver_type,
+  void AssumePrototypesStable(std::vector<Handle<Map>> const& receiver_maps,
                               Handle<Context> native_context,
                               Handle<JSObject> holder);
 

src/compiler/load-elimination.cc

@@ -312,10 +312,10 @@ Reduction LoadElimination::ReduceLoadField(Node* node) {
     if (Node* const replacement = state->LookupField(object, field_index)) {
       // Make sure the {replacement} has at least as good type
       // as the original {node}.
-      if (NodeProperties::GetType(replacement)
+      if (!replacement->IsDead() &&
+          NodeProperties::GetType(replacement)
               ->Is(NodeProperties::GetType(node))) {
         ReplaceWithValue(node, replacement, effect);
-        DCHECK(!replacement->IsDead());
         return Replace(replacement);
       }
     }
@@ -357,10 +357,10 @@ Reduction LoadElimination::ReduceLoadElement(Node* node) {
   if (Node* const replacement = state->LookupElement(object, index)) {
     // Make sure the {replacement} has at least as good type
     // as the original {node}.
-    if (NodeProperties::GetType(replacement)
+    if (!replacement->IsDead() &&
+        NodeProperties::GetType(replacement)
             ->Is(NodeProperties::GetType(node))) {
       ReplaceWithValue(node, replacement, effect);
-      DCHECK(!replacement->IsDead());
       return Replace(replacement);
     }
   }

src/field-index.h

@@ -76,6 +76,11 @@ class FieldIndex final {
         (IsInObjectBits::kMask | IsDoubleBits::kMask | IndexBits::kMask);
   }
 
+  bool operator==(FieldIndex const& other) const {
+    return bit_field_ == other.bit_field_;
+  }
+  bool operator!=(FieldIndex const& other) const { return !(*this == other); }
+
   // For GetLoadByFieldOffset.
   class FieldOffsetIsInobject : public BitField<bool, 1, 1> {};
   class FieldOffsetIsDouble : public BitField<bool, 2, 1> {};

src/handles.h

@@ -226,6 +226,10 @@ class MaybeHandle final {
     }
   }
 
+  // Returns the raw address where this handle is stored. This should only be
+  // used for hashing handles; do not ever try to dereference it.
+  V8_INLINE Address address() const { return bit_cast<Address>(location_); }
+
   bool is_null() const { return location_ == nullptr; }
 
  protected: