[turbofan] Brokerize JSCreateLowering::ReduceJSCreate

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

src/compiler/js-create-lowering.cc

@@ -41,11 +41,14 @@ Node* GetArgumentsFrameState(Node* frame_state) {
 
 // Checks whether allocation using the given target and new.target can be
 // inlined.
-bool IsAllocationInlineable(Handle<JSFunction> target,
-                            Handle<JSFunction> new_target) {
-  return new_target->has_initial_map() &&
-         !new_target->initial_map()->is_dictionary_map() &&
-         new_target->initial_map()->constructor_or_backpointer() == *target;
+bool IsAllocationInlineable(const JSFunctionRef& target,
+                            const JSFunctionRef& new_target,
+                            const JSHeapBroker* broker) {
+  return new_target.has_initial_map() &&
+         !new_target.initial_map(broker).is_dictionary_map() &&
+         new_target.initial_map(broker)
+             .constructor_or_backpointer(broker)
+             .equals(target);
 }
 
 // When initializing arrays, we'll unfold the loop if the number of
@@ -110,6 +113,7 @@ Reduction JSCreateLowering::Reduce(Node* node) {
 }
 
 Reduction JSCreateLowering::ReduceJSCreate(Node* node) {
+  DisallowHandleDereference disallow_handle_dereference;
   DCHECK_EQ(IrOpcode::kJSCreate, node->opcode());
   Node* const target = NodeProperties::GetValueInput(node, 0);
   Type const target_type = NodeProperties::GetType(target);
@@ -118,48 +122,52 @@ Reduction JSCreateLowering::ReduceJSCreate(Node* node) {
   Node* const effect = NodeProperties::GetEffectInput(node);
   Node* const control = NodeProperties::GetControlInput(node);
   // Extract constructor and original constructor function.
-  if (target_type.IsHeapConstant() && new_target_type.IsHeapConstant() &&
-      target_type.AsHeapConstant()->Value()->IsJSFunction() &&
-      new_target_type.AsHeapConstant()->Value()->IsJSFunction()) {
-    Handle<JSFunction> constructor =
-        Handle<JSFunction>::cast(target_type.AsHeapConstant()->Value());
-    if (!constructor->IsConstructor()) return NoChange();
-    Handle<JSFunction> original_constructor =
-        Handle<JSFunction>::cast(new_target_type.AsHeapConstant()->Value());
-    if (!original_constructor->IsConstructor()) return NoChange();
+  if (!target_type.IsHeapConstant() || !new_target_type.IsHeapConstant() ||
+      !target_type.AsHeapConstant()->Ref().IsJSFunction() ||
+      !new_target_type.AsHeapConstant()->Ref().IsJSFunction()) {
+    return NoChange();
+  }
 
-    // Check if we can inline the allocation.
-    if (IsAllocationInlineable(constructor, original_constructor)) {
-      // Force completion of inobject slack tracking before
-      // generating code to finalize the instance size.
-      original_constructor->CompleteInobjectSlackTrackingIfActive();
+  JSFunctionRef constructor =
+      target_type.AsHeapConstant()->Ref().AsJSFunction();
+  if (!constructor.IsConstructor()) return NoChange();
+  JSFunctionRef original_constructor =
+      new_target_type.AsHeapConstant()->Ref().AsJSFunction();
+  if (!original_constructor.IsConstructor()) return NoChange();
 
-      // Add a dependency on the {initial_map} to make sure that this code is
-      // deoptimized whenever the {initial_map} changes.
-      Handle<Map> initial_map =
-          dependencies()->DependOnInitialMap(original_constructor);
-      int const instance_size = initial_map->instance_size();
+  // Check if we can inline the allocation.
+  if (!IsAllocationInlineable(constructor, original_constructor,
+                              js_heap_broker())) {
+    return NoChange();
+  }
 
-      // Emit code to allocate the JSObject instance for the
-      // {original_constructor}.
-      AllocationBuilder a(jsgraph(), js_heap_broker(), effect, control);
-      a.Allocate(instance_size);
-      a.Store(AccessBuilder::ForMap(), initial_map);
-      a.Store(AccessBuilder::ForJSObjectPropertiesOrHash(),
-              jsgraph()->EmptyFixedArrayConstant());
-      a.Store(AccessBuilder::ForJSObjectElements(),
-              jsgraph()->EmptyFixedArrayConstant());
-      for (int i = 0; i < initial_map->GetInObjectProperties(); ++i) {
-        a.Store(
-            AccessBuilder::ForJSObjectInObjectProperty(MapRef(initial_map), i),
+  // Add a dependency on the {initial_map} to make sure that this code is
+  // deoptimized whenever the {initial_map} changes.
+  MapRef initial_map =
+      original_constructor.DependOnInitialMap(js_heap_broker(), dependencies());
+
+  // Force completion of inobject slack tracking before
+  // generating code to finalize the instance size.
+  int const instance_size =
+      original_constructor.GetInstanceSizeWithFinishedSlackTracking();
+
+  // Emit code to allocate the JSObject instance for the
+  // {original_constructor}.
+  AllocationBuilder a(jsgraph(), js_heap_broker(), effect, control);
+  a.Allocate(instance_size);
+  a.Store(AccessBuilder::ForMap(), initial_map);
+  a.Store(AccessBuilder::ForJSObjectPropertiesOrHash(),
+          jsgraph()->EmptyFixedArrayConstant());
+  a.Store(AccessBuilder::ForJSObjectElements(),
+          jsgraph()->EmptyFixedArrayConstant());
+  for (int i = 0; i < initial_map.GetInObjectProperties(); ++i) {
+    a.Store(AccessBuilder::ForJSObjectInObjectProperty(initial_map, i),
             jsgraph()->UndefinedConstant());
-      }
-      RelaxControls(node);
-      a.FinishAndChange(node);
-      return Changed(node);
-    }
   }
-  return NoChange();
+
+  RelaxControls(node);
+  a.FinishAndChange(node);
+  return Changed(node);
 }
 
 Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
@@ -722,7 +730,9 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
     DCHECK(original_constructor->IsConstructor());
 
     // Check if we can inline the allocation.
-    if (IsAllocationInlineable(constructor, original_constructor)) {
+    if (IsAllocationInlineable(JSFunctionRef(constructor),
+                               JSFunctionRef(original_constructor),
+                               js_heap_broker())) {
       // Force completion of inobject slack tracking before
       // generating code to finalize the instance size.
       original_constructor->CompleteInobjectSlackTrackingIfActive();

src/compiler/js-heap-broker.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/compiler/js-heap-broker.h"
+#include "src/compiler/compilation-dependencies.h"
 #include "src/objects-inl.h"
 #include "src/objects/js-regexp-inl.h"
 
@@ -165,6 +166,35 @@ BuiltinFunctionId JSFunctionRef::GetBuiltinFunctionId() const {
   return object<JSFunction>()->shared()->builtin_function_id();
 }
 
+bool JSFunctionRef::IsConstructor() const {
+  AllowHandleDereference allow_handle_dereference;
+  return object<JSFunction>()->IsConstructor();
+}
+
+MapRef JSFunctionRef::DependOnInitialMap(
+    const JSHeapBroker* broker, CompilationDependencies* dependencies) const {
+  AllowHandleDereference allow_handle_dereference;
+  Handle<Map> initial_map =
+      dependencies->DependOnInitialMap(object<JSFunction>());
+  return MapRef(initial_map);
+}
+
+int JSFunctionRef::GetInstanceSizeWithFinishedSlackTracking() const {
+  AllowHandleDereference allow_handle_dereference;
+  object<JSFunction>()->CompleteInobjectSlackTrackingIfActive();
+  return object<JSFunction>()->initial_map()->instance_size();
+}
+
+bool JSFunctionRef::has_initial_map() const {
+  AllowHandleDereference allow_handle_dereference;
+  return object<JSFunction>()->has_initial_map();
+}
+
+MapRef JSFunctionRef::initial_map(const JSHeapBroker* broker) const {
+  AllowHandleDereference allow_handle_dereference;
+  return MapRef(handle(object<JSFunction>()->initial_map(), broker->isolate()));
+}
+
 NameRef::NameRef(Handle<Object> object) : HeapObjectRef(object) {
   AllowHandleDereference handle_dereference;
   DCHECK(object->IsName());
@@ -373,6 +403,22 @@ FieldIndex MapRef::GetFieldIndexFor(int i) const {
   return FieldIndex::ForDescriptor(*object<Map>(), i);
 }
 
+int MapRef::GetInObjectPropertyOffset(int i) const {
+  AllowHandleDereference allow_handle_dereference;
+  return object<Map>()->GetInObjectPropertyOffset(i);
+}
+
+bool MapRef::is_dictionary_map() const {
+  AllowHandleDereference allow_handle_dereference;
+  return object<Map>()->is_dictionary_map();
+}
+
+ObjectRef MapRef::constructor_or_backpointer(const JSHeapBroker* broker) const {
+  AllowHandleDereference allow_handle_dereference;
+  return ObjectRef(
+      handle(object<Map>()->constructor_or_backpointer(), broker->isolate()));
+}
+
 int MapRef::instance_size() const {
   AllowHandleDereference allow_handle_dereference;
   return object<Map>()->instance_size();
@@ -404,22 +450,12 @@ bool MapRef::IsInobjectSlackTrackingInProgress() const {
   return object<Map>()->IsInobjectSlackTrackingInProgress();
 }
 
-bool MapRef::is_dictionary_map() const {
-  AllowHandleDereference allow_handle_dereference;
-  return object<Map>()->is_dictionary_map();
-}
-
 bool MapRef::IsFixedCowArrayMap(const JSHeapBroker* broker) const {
   AllowHandleDereference allow_handle_dereference;
   return *object<Map>() ==
          ReadOnlyRoots(broker->isolate()).fixed_cow_array_map();
 }
 
-int MapRef::GetInObjectPropertyOffset(int index) const {
-  AllowHandleDereference allow_handle_dereference;
-  return object<Map>()->GetInObjectPropertyOffset(index);
-}
-
 bool MapRef::has_prototype_slot() const {
   AllowHandleDereference allow_handle_dereference;
   return object<Map>()->has_prototype_slot();

src/compiler/js-heap-broker.h

@@ -14,6 +14,8 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+class CompilationDependencies;
+
 enum class OddballType : uint8_t {
   kNone,     // Not an Oddball.
   kBoolean,  // True or False.
@@ -163,8 +165,16 @@ class JSObjectRef : public HeapObjectRef {
 class JSFunctionRef : public JSObjectRef {
  public:
   explicit JSFunctionRef(Handle<Object> object);
+
   bool HasBuiltinFunctionId() const;
   BuiltinFunctionId GetBuiltinFunctionId() const;
+  bool IsConstructor() const;
+  bool has_initial_map() const;
+  MapRef initial_map(const JSHeapBroker* broker) const;
+
+  MapRef DependOnInitialMap(const JSHeapBroker* broker,
+                            CompilationDependencies* dependencies) const;
+  int GetInstanceSizeWithFinishedSlackTracking() const;
 };
 
 class JSRegExpRef : public JSObjectRef {
@@ -260,15 +270,17 @@ class MapRef : public HeapObjectRef {
   InstanceType instance_type() const;
   int GetInObjectProperties() const;
   int NumberOfOwnDescriptors() const;
+
   PropertyDetails GetPropertyDetails(int i) const;
   NameRef GetPropertyKey(const JSHeapBroker* broker, int i) const;
   FieldIndex GetFieldIndexFor(int i) const;
-
   int GetInObjectPropertyOffset(int index) const;
 
+  bool is_dictionary_map() const;
+  ObjectRef constructor_or_backpointer(const JSHeapBroker* broker) const;
+
   bool IsJSArrayMap() const;
   bool IsFixedCowArrayMap(const JSHeapBroker* broker) const;
-  bool is_dictionary_map() const;
 
   bool has_prototype_slot() const;
 

test/unittests/compiler/js-create-lowering-unittest.cc

@@ -30,7 +30,10 @@ namespace compiler {
 class JSCreateLoweringTest : public TypedGraphTest {
  public:
   JSCreateLoweringTest()
-      : TypedGraphTest(3), javascript_(zone()), deps_(isolate(), zone()) {}
+      : TypedGraphTest(3),
+        javascript_(zone()),
+        deps_(isolate(), zone()),
+        handle_scope_(isolate()) {}
   ~JSCreateLoweringTest() override {}
 
  protected:
@@ -63,6 +66,7 @@ class JSCreateLoweringTest : public TypedGraphTest {
  private:
   JSOperatorBuilder javascript_;
   CompilationDependencies deps_;
+  CanonicalHandleScope handle_scope_;
 };
 
 // -----------------------------------------------------------------------------