[turbofan] Get rid of HeapObjectType-related heap reads.

This removes the last unconditional read accesses to the heap, but
required a significant refactoring.

- Remove HeapObjectRef::type().
- Change HeapObjectData::Is* testers to look at the instance type
  in HeapObjectData::map().

- Remove ObjectRef::oddball_type()
- Add MapRef::oddball_type()
- Add MapRef::is_undetectable().
- Add MapRef::is_callable().

- Remove JSHeapBroker::HeapObjectTypeFromMap()
- Remove Type::For(JSHeapBroker*, Handle<Map>)
- Add BitsetType::Lub(MapRef).
- Add Type::For(MapRef).
- Add Type::For(HeapObjectType).

- Add HeapObjectRef::GetHeapObjectType(). THIS IS TEMPORARY.

As the last item suggests, I couldn't actually remove the
HeapObjectType class yet. See the explanation in the code.

Bug: v8:7790
Change-Id: I508e4bd5337277b0050f2204392fc36f41032fe9
Reviewed-on: https://chromium-review.googlesource.com/1228033Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55994}

src/compiler/access-info.cc

@@ -409,7 +409,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
               dependencies()->DependOnFieldType(map_ref, number);
               // Remember the field map, and try to infer a useful type.
               Handle<Map> map(descriptors_field_type->AsClass(), isolate());
-              field_type = Type::For(js_heap_broker(), map);
+              field_type = Type::For(MapRef(js_heap_broker(), map));
               field_map = MaybeHandle<Map>(map);
             }
           }
@@ -714,7 +714,7 @@ bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
       dependencies()->DependOnFieldType(transition_map_ref, number);
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map(descriptors_field_type->AsClass(), isolate());
-      field_type = Type::For(js_heap_broker(), map);
+      field_type = Type::For(MapRef(js_heap_broker(), map));
       field_map = MaybeHandle<Map>(map);
     }
   }

src/compiler/js-context-specialization.cc

@@ -165,7 +165,7 @@ Reduction JSContextSpecialization::ReduceJSLoadContext(Node* node) {
     // We must be conservative and check if the value in the slot is currently
     // the hole or undefined. Only if it is neither of these, can we be sure
     // that it won't change anymore.
-    OddballType oddball_type = maybe_value->oddball_type();
+    OddballType oddball_type = maybe_value->AsHeapObject().map().oddball_type();
     if (oddball_type == OddballType::kUndefined ||
         oddball_type == OddballType::kHole) {
       maybe_value.reset();

src/compiler/js-create-lowering.cc

@@ -1352,7 +1352,7 @@ base::Optional<MapRef> GetObjectCreateMap(JSHeapBroker* broker,
   if (prototype.equals(standard_map.prototype())) {
     return standard_map;
   }
-  if (prototype.oddball_type() == OddballType::kNull) {
+  if (prototype.map().oddball_type() == OddballType::kNull) {
     return broker->native_context().slow_object_with_null_prototype_map();
   }
   if (prototype.IsJSObject()) {
@@ -1378,7 +1378,7 @@ Reduction JSCreateLowering::ReduceJSCreateObject(Node* node) {
 
   Node* properties = jsgraph()->EmptyFixedArrayConstant();
   if (instance_map.is_dictionary_map()) {
-    DCHECK_EQ(prototype_const.type().oddball_type(), OddballType::kNull);
+    DCHECK_EQ(prototype_const.map().oddball_type(), OddballType::kNull);
     // Allocate an empty NameDictionary as backing store for the properties.
     Handle<Map> map = isolate()->factory()->name_dictionary_map();
     int capacity =
@@ -1689,7 +1689,11 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
         value = effect = builder.Finish();
       } else if (property_details.representation().IsSmi()) {
         // Ensure that value is stored as smi.
-        value = boilerplate_value.oddball_type() == OddballType::kUninitialized
+        bool is_uninitialized =
+            boilerplate_value.IsHeapObject() &&
+            boilerplate_value.AsHeapObject().map().oddball_type() ==
+                OddballType::kUninitialized;
+        value = is_uninitialized
                     ? jsgraph()->ZeroConstant()
                     : jsgraph()->Constant(boilerplate_value.AsSmi());
       } else {
@@ -1717,7 +1721,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
   // Actually allocate and initialize the object.
   AllocationBuilder builder(jsgraph(), effect, control);
   builder.Allocate(boilerplate_map.instance_size(), pretenure,
-                   Type::For(js_heap_broker(), boilerplate_map.object<Map>()));
+                   Type::For(boilerplate_map));
   builder.Store(AccessBuilder::ForMap(), boilerplate_map);
   builder.Store(AccessBuilder::ForJSObjectPropertiesOrHash(), properties);
   builder.Store(AccessBuilder::ForJSObjectElements(), elements);
@@ -1805,8 +1809,7 @@ Node* JSCreateLowering::AllocateLiteralRegExp(Node* effect, Node* control,
       JSRegExp::kSize + JSRegExp::kInObjectFieldCount * kPointerSize;
 
   AllocationBuilder builder(jsgraph(), effect, control);
-  builder.Allocate(size, pretenure,
-                   Type::For(js_heap_broker(), boilerplate_map.object<Map>()));
+  builder.Allocate(size, pretenure, Type::For(boilerplate_map));
   builder.Store(AccessBuilder::ForMap(), boilerplate_map);
   builder.Store(AccessBuilder::ForJSObjectPropertiesOrHash(),
                 boilerplate.raw_properties_or_hash());

src/compiler/js-graph.cc

@@ -68,7 +68,8 @@ Node* JSGraph::Constant(Handle<Object> value) {
 
 Node* JSGraph::Constant(const ObjectRef& ref) {
   if (ref.IsSmi()) return Constant(ref.AsSmi());
-  OddballType oddball_type = ref.oddball_type();
+  OddballType oddball_type =
+      ref.AsHeapObject().GetHeapObjectType().oddball_type();
   if (ref.IsHeapNumber()) {
     return Constant(ref.AsHeapNumber().value());
   } else if (oddball_type == OddballType::kUndefined) {

src/compiler/js-heap-broker.h

@@ -25,35 +25,6 @@ enum class OddballType : uint8_t {
   kOther  // Oddball, but none of the above.
 };
 
-// TODO(neis): Get rid of the HeapObjectType class.
-class HeapObjectType {
- public:
-  enum Flag : uint8_t { kUndetectable = 1 << 0, kCallable = 1 << 1 };
-
-  typedef base::Flags<Flag> Flags;
-
-  HeapObjectType(InstanceType instance_type, Flags flags,
-                 OddballType oddball_type)
-      : instance_type_(instance_type),
-        oddball_type_(oddball_type),
-        flags_(flags) {
-    DCHECK_EQ(instance_type == ODDBALL_TYPE,
-              oddball_type != OddballType::kNone);
-  }
-
-  OddballType oddball_type() const { return oddball_type_; }
-  InstanceType instance_type() const { return instance_type_; }
-  Flags flags() const { return flags_; }
-
-  bool is_callable() const { return flags_ & kCallable; }
-  bool is_undetectable() const { return flags_ & kUndetectable; }
-
- private:
-  InstanceType const instance_type_;
-  OddballType const oddball_type_;
-  Flags const flags_;
-};
-
 // This list is sorted such that subtypes appear before their supertypes.
 // DO NOT VIOLATE THIS PROPERTY!
 #define HEAP_BROKER_OBJECT_LIST(V) \
@@ -114,8 +85,6 @@ class ObjectRef {
     return Handle<T>::cast(object());
   }
 
-  OddballType oddball_type() const;
-
   bool IsSmi() const;
   int AsSmi() const;
 
@@ -140,12 +109,49 @@ class ObjectRef {
   ObjectData* data_;
 };
 
+// Temporary class that carries information from a Map. We'd like to remove
+// this class and use MapRef instead, but we can't as long as we support the
+// kDisabled broker mode. That's because obtaining the MapRef via
+// HeapObjectRef::map() requires a HandleScope when the broker is disabled.
+// During OptimizeGraph we generally don't have a HandleScope, however. There
+// are two places where we therefore use GetHeapObjectType() instead. Both that
+// function and this class should eventually be removed.
+class HeapObjectType {
+ public:
+  enum Flag : uint8_t { kUndetectable = 1 << 0, kCallable = 1 << 1 };
+
+  typedef base::Flags<Flag> Flags;
+
+  HeapObjectType(InstanceType instance_type, Flags flags,
+                 OddballType oddball_type)
+      : instance_type_(instance_type),
+        oddball_type_(oddball_type),
+        flags_(flags) {
+    DCHECK_EQ(instance_type == ODDBALL_TYPE,
+              oddball_type != OddballType::kNone);
+  }
+
+  OddballType oddball_type() const { return oddball_type_; }
+  InstanceType instance_type() const { return instance_type_; }
+  Flags flags() const { return flags_; }
+
+  bool is_callable() const { return flags_ & kCallable; }
+  bool is_undetectable() const { return flags_ & kUndetectable; }
+
+ private:
+  InstanceType const instance_type_;
+  OddballType const oddball_type_;
+  Flags const flags_;
+};
+
 class HeapObjectRef : public ObjectRef {
  public:
   using ObjectRef::ObjectRef;
 
-  HeapObjectType type() const;
   MapRef map() const;
+
+  // See the comment on the HeapObjectType class.
+  HeapObjectType GetHeapObjectType() const;
 };
 
 class PropertyCellRef : public HeapObjectRef {
@@ -327,10 +333,14 @@ class MapRef : public HeapObjectRef {
   bool IsInobjectSlackTrackingInProgress() const;
   bool is_dictionary_map() const;
   bool IsFixedCowArrayMap() const;
+  bool is_undetectable() const;
+  bool is_callable() const;
 
   ObjectRef constructor_or_backpointer() const;
   ObjectRef prototype() const;
 
+  OddballType oddball_type() const;
+
   base::Optional<MapRef> AsElementsKind(ElementsKind kind) const;
 
   // Concerning the underlying instance_descriptors:
@@ -456,11 +466,6 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   JSHeapBroker(Isolate* isolate, Zone* zone);
   void SerializeStandardObjects();
 
-  HeapObjectType HeapObjectTypeFromMap(Handle<Map> map) const {
-    AllowHandleDereference handle_dereference;
-    return HeapObjectTypeFromMap(*map);
-  }
-
   Isolate* isolate() const { return isolate_; }
   Zone* zone() const { return zone_; }
 
@@ -488,9 +493,6 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   friend class ObjectRef;
   friend class ObjectData;
 
-  // TODO(neis): Remove eventually.
-  HeapObjectType HeapObjectTypeFromMap(Map* map) const;
-
   void AddData(Handle<Object> object, ObjectData* data);
 
   Isolate* const isolate_;

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

@@ -664,20 +664,19 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
             property_cell_value_type = Type::Number();
             representation = MachineRepresentation::kTaggedPointer;
           } else {
-            Handle<Map> property_cell_value_map(
-                Handle<HeapObject>::cast(property_cell_value)->map(),
-                isolate());
-            property_cell_value_type =
-                Type::For(js_heap_broker(), property_cell_value_map);
+            MapRef property_cell_value_map(
+                js_heap_broker(),
+                handle(HeapObject::cast(*property_cell_value)->map(),
+                       isolate()));
+            property_cell_value_type = Type::For(property_cell_value_map);
             representation = MachineRepresentation::kTaggedPointer;
 
             // We can only use the property cell value map for map check
             // elimination if it's stable, i.e. the HeapObject wasn't
             // mutated without the cell state being updated.
-            if (property_cell_value_map->is_stable()) {
-              dependencies()->DependOnStableMap(
-                  MapRef(js_heap_broker(), property_cell_value_map));
-              map = property_cell_value_map;
+            if (property_cell_value_map.is_stable()) {
+              dependencies()->DependOnStableMap(property_cell_value_map);
+              map = property_cell_value_map.object<Map>();
             }
           }
         }
@@ -780,8 +779,8 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadGlobal(Node* node) {
       native_context().script_context_table().lookup(name);
   if (result) {
     ObjectRef contents = result->context.get(result->index);
-    OddballType oddball_type = contents.oddball_type();
-    if (oddball_type == OddballType::kHole) {
+    if (contents.IsHeapObject() &&
+        contents.AsHeapObject().map().oddball_type() == OddballType::kHole) {
       return NoChange();
     }
     Node* context = jsgraph()->Constant(result->context);
@@ -809,8 +808,9 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreGlobal(Node* node) {
       native_context().script_context_table().lookup(name);
   if (result) {
     ObjectRef contents = result->context.get(result->index);
-    OddballType oddball_type = contents.oddball_type();
-    if (oddball_type == OddballType::kHole || result->immutable) {
+    if ((contents.IsHeapObject() &&
+         contents.AsHeapObject().map().oddball_type() == OddballType::kHole) ||
+        result->immutable) {
       return NoChange();
     }
     Node* context = jsgraph()->Constant(result->context);

src/compiler/js-typed-lowering.cc

@@ -952,8 +952,8 @@ Reduction JSTypedLowering::ReduceJSToNumberInput(Node* input) {
     }
   }
   if (input_type.IsHeapConstant()) {
-    ObjectRef input_value = input_type.AsHeapConstant()->Ref();
-    if (input_value.oddball_type() != OddballType::kNone) {
+    HeapObjectRef input_value = input_type.AsHeapConstant()->Ref();
+    if (input_value.map().oddball_type() != OddballType::kNone) {
       return Replace(jsgraph()->Constant(input_value.OddballToNumber()));
     }
   }

src/compiler/types.cc

@@ -132,8 +132,11 @@ Type::bitset Type::BitsetLub() const {
   UNREACHABLE();
 }
 
-Type::bitset BitsetType::Lub(HeapObjectType const& type) {
-  switch (type.instance_type()) {
+// TODO(neis): Once the broker mode kDisabled is gone, change the input type to
+// MapRef and get rid of the HeapObjectType class.
+template <typename MapRefLike>
+Type::bitset BitsetType::Lub(const MapRefLike& map) {
+  switch (map.instance_type()) {
     case CONS_STRING_TYPE:
     case CONS_ONE_BYTE_STRING_TYPE:
     case THIN_STRING_TYPE:
@@ -163,7 +166,7 @@ Type::bitset BitsetType::Lub(HeapObjectType const& type) {
     case BIGINT_TYPE:
       return kBigInt;
     case ODDBALL_TYPE:
-      switch (type.oddball_type()) {
+      switch (map.oddball_type()) {
         case OddballType::kNone:
           break;
         case OddballType::kHole:
@@ -189,15 +192,15 @@ Type::bitset BitsetType::Lub(HeapObjectType const& type) {
     case JS_GLOBAL_PROXY_TYPE:
     case JS_API_OBJECT_TYPE:
     case JS_SPECIAL_API_OBJECT_TYPE:
-      if (type.is_undetectable()) {
+      if (map.is_undetectable()) {
         // Currently we assume that every undetectable receiver is also
         // callable, which is what we need to support document.all.  We
         // could add another Type bit to support other use cases in the
         // future if necessary.
-        DCHECK(type.is_callable());
+        DCHECK(map.is_callable());
         return kOtherUndetectable;
       }
-      if (type.is_callable()) {
+      if (map.is_callable()) {
         return kOtherCallable;
       }
       return kOtherObject;
@@ -244,18 +247,18 @@ Type::bitset BitsetType::Lub(HeapObjectType const& type) {
     case WASM_MEMORY_TYPE:
     case WASM_MODULE_TYPE:
     case WASM_TABLE_TYPE:
-      DCHECK(!type.is_callable());
-      DCHECK(!type.is_undetectable());
+      DCHECK(!map.is_callable());
+      DCHECK(!map.is_undetectable());
       return kOtherObject;
     case JS_BOUND_FUNCTION_TYPE:
-      DCHECK(!type.is_undetectable());
+      DCHECK(!map.is_undetectable());
       return kBoundFunction;
     case JS_FUNCTION_TYPE:
-      DCHECK(!type.is_undetectable());
+      DCHECK(!map.is_undetectable());
       return kFunction;
     case JS_PROXY_TYPE:
-      DCHECK(!type.is_undetectable());
-      if (type.is_callable()) return kCallableProxy;
+      DCHECK(!map.is_undetectable());
+      if (map.is_callable()) return kCallableProxy;
       return kOtherProxy;
     case MAP_TYPE:
     case ALLOCATION_SITE_TYPE:
@@ -352,6 +355,9 @@ Type::bitset BitsetType::Lub(HeapObjectType const& type) {
   UNREACHABLE();
 }
 
+// Explicit instantiation.
+template Type::bitset BitsetType::Lub<MapRef>(const MapRef& map);
+
 Type::bitset BitsetType::Lub(double value) {
   DisallowHeapAllocation no_allocation;
   if (IsMinusZero(value)) return kMinusZero;

src/compiler/types.h

@@ -253,7 +253,10 @@ class V8_EXPORT_PRIVATE BitsetType {
   static double Max(bitset);
 
   static bitset Glb(double min, double max);
-  static bitset Lub(HeapObjectType const& type);
+  static bitset Lub(HeapObjectType const& type) {
+    return Lub<HeapObjectType>(type);
+  }
+  static bitset Lub(MapRef const& map) { return Lub<MapRef>(map); }
   static bitset Lub(double value);
   static bitset Lub(double min, double max);
   static bitset ExpandInternals(bitset bits);
@@ -275,6 +278,9 @@ class V8_EXPORT_PRIVATE BitsetType {
   static const Boundary BoundariesArray[];
   static inline const Boundary* Boundaries();
   static inline size_t BoundariesSize();
+
+  template <typename MapRefLike>
+  static bitset Lub(MapRefLike const& map);
 };
 
 // -----------------------------------------------------------------------------
@@ -379,8 +385,10 @@ class V8_EXPORT_PRIVATE Type {
   static Type Union(Type type1, Type type2, Zone* zone);
   static Type Intersect(Type type1, Type type2, Zone* zone);
 
-  static Type For(JSHeapBroker* js_heap_broker, Handle<i::Map> map) {
-    HeapObjectType type = js_heap_broker->HeapObjectTypeFromMap(map);
+  static Type For(HeapObjectType const& type) {
+    return NewBitset(BitsetType::ExpandInternals(BitsetType::Lub(type)));
+  }
+  static Type For(MapRef const& type) {
     return NewBitset(BitsetType::ExpandInternals(BitsetType::Lub(type)));
   }
 
@@ -547,7 +555,7 @@ class V8_EXPORT_PRIVATE HeapConstantType : public NON_EXPORTED_BASE(TypeBase) {
   static HeapConstantType* New(const HeapObjectRef& heap_ref, Zone* zone) {
     DCHECK(!heap_ref.IsHeapNumber());
     DCHECK_IMPLIES(heap_ref.IsString(), heap_ref.IsInternalizedString());
-    BitsetType::bitset bitset = BitsetType::Lub(heap_ref.type());
+    BitsetType::bitset bitset = BitsetType::Lub(heap_ref.GetHeapObjectType());
     return new (zone->New(sizeof(HeapConstantType)))
         HeapConstantType(bitset, heap_ref);
   }