[turbofan] Canonicalize uses of DependOnProtector

This merges the check if a protector is intact with the recording of
the dependency on it, at least in many cases.

Also introduce convenience functions to avoid the heap broker clutter.

Change-Id: I35508c4685a2f0df77819bf81075dd14a30e7e4f
Reviewed-on: https://chromium-review.googlesource.com/c/1487491
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59924}

src/compiler/compilation-dependencies.cc

@@ -12,8 +12,9 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-CompilationDependencies::CompilationDependencies(Isolate* isolate, Zone* zone)
-    : zone_(zone), dependencies_(zone), isolate_(isolate) {}
+CompilationDependencies::CompilationDependencies(JSHeapBroker* broker,
+                                                 Zone* zone)
+    : zone_(zone), broker_(broker), dependencies_(zone) {}
 
 class CompilationDependencies::Dependency : public ZoneObject {
  public:
@@ -423,8 +424,46 @@ void CompilationDependencies::DependOnGlobalProperty(
                                GlobalPropertyDependency(cell, type, read_only));
 }
 
-void CompilationDependencies::DependOnProtector(const PropertyCellRef& cell) {
+bool CompilationDependencies::DependOnProtector(const PropertyCellRef& cell) {
+  if (cell.value().AsSmi() != Isolate::kProtectorValid) return false;
   dependencies_.push_front(new (zone_) ProtectorDependency(cell));
+  return true;
+}
+
+bool CompilationDependencies::DependOnArrayBufferDetachingProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_,
+      broker_->isolate()->factory()->array_buffer_detaching_protector()));
+}
+
+bool CompilationDependencies::DependOnArrayIteratorProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->array_iterator_protector()));
+}
+
+bool CompilationDependencies::DependOnArraySpeciesProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->array_species_protector()));
+}
+
+bool CompilationDependencies::DependOnNoElementsProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->no_elements_protector()));
+}
+
+bool CompilationDependencies::DependOnPromiseHookProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->promise_hook_protector()));
+}
+
+bool CompilationDependencies::DependOnPromiseSpeciesProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->promise_species_protector()));
+}
+
+bool CompilationDependencies::DependOnPromiseThenProtector() {
+  return DependOnProtector(PropertyCellRef(
+      broker_, broker_->isolate()->factory()->promise_then_protector()));
 }
 
 void CompilationDependencies::DependOnElementsKind(
@@ -474,7 +513,7 @@ bool CompilationDependencies::Commit(Handle<Code> code) {
   // these cases, because once the code gets executed it will do a stack check
   // that triggers its deoptimization.
   if (FLAG_stress_gc_during_compilation) {
-    isolate_->heap()->PreciseCollectAllGarbage(
+    broker_->isolate()->heap()->PreciseCollectAllGarbage(
         Heap::kNoGCFlags, GarbageCollectionReason::kTesting,
         kGCCallbackFlagForced);
   }
@@ -490,8 +529,7 @@ bool CompilationDependencies::Commit(Handle<Code> code) {
 
 namespace {
 // This function expects to never see a JSProxy.
-void DependOnStablePrototypeChain(JSHeapBroker* broker,
-                                  CompilationDependencies* deps, MapRef map,
+void DependOnStablePrototypeChain(CompilationDependencies* deps, MapRef map,
                                   const JSObjectRef& last_prototype) {
   while (true) {
     map.SerializePrototype();
@@ -504,19 +542,18 @@ void DependOnStablePrototypeChain(JSHeapBroker* broker,
 }  // namespace
 
 void CompilationDependencies::DependOnStablePrototypeChains(
-    JSHeapBroker* broker, std::vector<Handle<Map>> const& receiver_maps,
-    const JSObjectRef& holder) {
+    std::vector<Handle<Map>> const& receiver_maps, const JSObjectRef& holder) {
   // Determine actual holder and perform prototype chain checks.
   for (auto map : receiver_maps) {
-    MapRef receiver_map(broker, map);
+    MapRef receiver_map(broker_, map);
     if (receiver_map.IsPrimitiveMap()) {
       // Perform the implicit ToObject for primitives here.
       // Implemented according to ES6 section 7.3.2 GetV (V, P).
       base::Optional<JSFunctionRef> constructor =
-          broker->native_context().GetConstructorFunction(receiver_map);
+          broker_->native_context().GetConstructorFunction(receiver_map);
       if (constructor.has_value()) receiver_map = constructor->initial_map();
     }
-    DependOnStablePrototypeChain(broker, this, receiver_map, holder);
+    DependOnStablePrototypeChain(this, receiver_map, holder);
   }
 }
 

src/compiler/compilation-dependencies.h

@@ -28,7 +28,7 @@ class SlackTrackingPrediction {
 // Collects and installs dependencies of the code that is being generated.
 class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
  public:
-  CompilationDependencies(Isolate* isolate, Zone* zone);
+  CompilationDependencies(JSHeapBroker* broker, Zone* zone);
 
   V8_WARN_UNUSED_RESULT bool Commit(Handle<Code> code);
 
@@ -68,8 +68,18 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // {IsReadOnly()} flag of {cell}'s {PropertyDetails}.
   void DependOnGlobalProperty(const PropertyCellRef& cell);
 
-  // Record the assumption that the protector remains valid.
-  void DependOnProtector(const PropertyCellRef& cell);
+  // Return the validity of the given protector and, if true, record the
+  // assumption that the protector remains valid.
+  bool DependOnProtector(const PropertyCellRef& cell);
+
+  // Convenience wrappers around {DependOnProtector}.
+  bool DependOnArrayBufferDetachingProtector();
+  bool DependOnArrayIteratorProtector();
+  bool DependOnArraySpeciesProtector();
+  bool DependOnNoElementsProtector();
+  bool DependOnPromiseHookProtector();
+  bool DependOnPromiseSpeciesProtector();
+  bool DependOnPromiseThenProtector();
 
   // Record the assumption that {site}'s {ElementsKind} doesn't change.
   void DependOnElementsKind(const AllocationSiteRef& site);
@@ -77,8 +87,7 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // Depend on the stability of (the maps of) all prototypes of every class in
   // {receiver_type} up to (and including) the {holder}.
   void DependOnStablePrototypeChains(
-      JSHeapBroker* broker, std::vector<Handle<Map>> const& receiver_maps,
-      const JSObjectRef& holder);
+      std::vector<Handle<Map>> const& receiver_maps, const JSObjectRef& holder);
 
   // Like DependOnElementsKind but also applies to all nested allocation sites.
   void DependOnElementsKinds(const AllocationSiteRef& site);
@@ -98,9 +107,9 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   class Dependency;
 
  private:
-  Zone* zone_;
+  Zone* const zone_;
+  JSHeapBroker* const broker_;
   ZoneForwardList<Dependency*> dependencies_;
-  Isolate* isolate_;
 };
 
 }  // namespace compiler

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

@@ -219,11 +219,8 @@ Reduction JSNativeContextSpecialization::ReduceJSAsyncFunctionEnter(
   Node* frame_state = NodeProperties::GetFrameStateInput(node);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
-  if (!isolate()->IsPromiseHookProtectorIntact()) return NoChange();
 
-  // Install a code dependency on the promise hook protector cell.
-  dependencies()->DependOnProtector(
-      PropertyCellRef(broker(), factory()->promise_hook_protector()));
+  if (!dependencies()->DependOnPromiseHookProtector()) return NoChange();
 
   // Create the promise for the async function.
   Node* promise = effect =
@@ -252,11 +249,8 @@ Reduction JSNativeContextSpecialization::ReduceJSAsyncFunctionReject(
   Node* frame_state = NodeProperties::GetFrameStateInput(node);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
-  if (!isolate()->IsPromiseHookProtectorIntact()) return NoChange();
 
-  // Install a code dependency on the promise hook protector cell.
-  dependencies()->DependOnProtector(
-      PropertyCellRef(broker(), factory()->promise_hook_protector()));
+  if (!dependencies()->DependOnPromiseHookProtector()) return NoChange();
 
   // Load the promise from the {async_function_object}.
   Node* promise = effect = graph()->NewNode(
@@ -291,11 +285,8 @@ Reduction JSNativeContextSpecialization::ReduceJSAsyncFunctionResolve(
   Node* frame_state = NodeProperties::GetFrameStateInput(node);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
-  if (!isolate()->IsPromiseHookProtectorIntact()) return NoChange();
 
-  // Install a code dependency on the promise hook protector cell.
-  dependencies()->DependOnProtector(
-      PropertyCellRef(broker(), factory()->promise_hook_protector()));
+  if (!dependencies()->DependOnPromiseHookProtector()) return NoChange();
 
   // Load the promise from the {async_function_object}.
   Node* promise = effect = graph()->NewNode(
@@ -426,7 +417,7 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
     Handle<JSObject> holder;
     if (access_info.holder().ToHandle(&holder)) {
       dependencies()->DependOnStablePrototypeChains(
-          broker(), access_info.receiver_maps(), JSObjectRef(broker(), holder));
+          access_info.receiver_maps(), JSObjectRef(broker(), holder));
     }
 
     // Monomorphic property access.
@@ -481,7 +472,7 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
 
     if (found_on_proto) {
       dependencies()->DependOnStablePrototypeChains(
-          broker(), access_info.receiver_maps(), JSObjectRef(broker(), holder));
+          access_info.receiver_maps(), JSObjectRef(broker(), holder));
     }
 
     DCHECK(constant->IsCallable());
@@ -665,10 +656,6 @@ Reduction JSNativeContextSpecialization::ReduceJSPromiseResolve(Node* node) {
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
-  if (!isolate()->IsPromiseHookProtectorIntact()) {
-    return NoChange();
-  }
-
   // Check if the {constructor} is the %Promise% function.
   HeapObjectMatcher m(constructor);
   if (!m.HasValue() ||
@@ -688,9 +675,7 @@ Reduction JSNativeContextSpecialization::ReduceJSPromiseResolve(Node* node) {
     if (value_map->IsJSPromiseMap()) return NoChange();
   }
 
-  // Install a code dependency on the promise hook protector cell.
-  dependencies()->DependOnProtector(
-      PropertyCellRef(broker(), factory()->promise_hook_protector()));
+  if (!dependencies()->DependOnPromiseHookProtector()) return NoChange();
 
   // Create a %Promise% instance and resolve it with {value}.
   Node* promise = effect =
@@ -745,7 +730,7 @@ Reduction JSNativeContextSpecialization::ReduceJSResolvePromise(Node* node) {
   Handle<JSObject> holder;
   if (access_info.holder().ToHandle(&holder)) {
     dependencies()->DependOnStablePrototypeChains(
-        broker(), access_info.receiver_maps(), JSObjectRef(broker(), holder));
+        access_info.receiver_maps(), JSObjectRef(broker(), holder));
   }
 
   // Add stability dependencies on the {resolution_maps}.
@@ -2159,7 +2144,7 @@ JSNativeContextSpecialization::BuildPropertyLoad(
   PropertyAccessBuilder access_builder(jsgraph(), broker(), dependencies());
   if (access_info.holder().ToHandle(&holder)) {
     dependencies()->DependOnStablePrototypeChains(
-        broker(), access_info.receiver_maps(), JSObjectRef(broker(), holder));
+        access_info.receiver_maps(), JSObjectRef(broker(), holder));
   }
 
   // Generate the actual property access.
@@ -2218,7 +2203,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
   if (access_info.holder().ToHandle(&holder)) {
     DCHECK_NE(AccessMode::kStoreInLiteral, access_mode);
     dependencies()->DependOnStablePrototypeChains(
-        broker(), access_info.receiver_maps(), JSObjectRef(broker(), holder));
+        access_info.receiver_maps(), JSObjectRef(broker(), holder));
   }
 
   DCHECK(!access_info.IsNotFound());
@@ -2636,12 +2621,7 @@ JSNativeContextSpecialization::BuildElementAccess(
     }
 
     // See if we can skip the detaching check.
-    if (isolate()->IsArrayBufferDetachingIntact()) {
-      // Add a code dependency so we are deoptimized in case an ArrayBuffer
-      // gets detached.
-      dependencies()->DependOnProtector(PropertyCellRef(
-          broker(), factory()->array_buffer_detaching_protector()));
-    } else {
+    if (!dependencies()->DependOnArrayBufferDetachingProtector()) {
       // Deopt if the {buffer} was detached.
       // Note: A detached buffer leads to megamorphic feedback.
       Node* buffer_bit_field = effect = graph()->NewNode(
@@ -3050,10 +3030,7 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
     Node* receiver, Node* index, Node* length, Node** effect, Node** control,
     KeyedAccessLoadMode load_mode) {
   if (load_mode == LOAD_IGNORE_OUT_OF_BOUNDS &&
-      isolate()->IsNoElementsProtectorIntact()) {
-    dependencies()->DependOnProtector(
-        PropertyCellRef(broker(), factory()->no_elements_protector()));
-
+      dependencies()->DependOnNoElementsProtector()) {
     // Ensure that the {index} is a valid String length.
     index = *effect = graph()->NewNode(
         simplified()->CheckBounds(VectorSlotPair()), index,
@@ -3200,11 +3177,7 @@ bool JSNativeContextSpecialization::CanTreatHoleAsUndefined(
   }
 
   // Check if the array prototype chain is intact.
-  if (!isolate()->IsNoElementsProtectorIntact()) return false;
-
-  dependencies()->DependOnProtector(
-      PropertyCellRef(broker(), factory()->no_elements_protector()));
-  return true;
+  return dependencies()->DependOnNoElementsProtector();
 }
 
 namespace {

src/compiler/pipeline.cc

@@ -136,7 +136,7 @@ class PipelineData {
         JSGraph(isolate_, graph_, common_, javascript_, simplified_, machine_);
     broker_ = new (info_->zone()) JSHeapBroker(isolate_, info_->zone());
     dependencies_ =
-        new (info_->zone()) CompilationDependencies(isolate_, info_->zone());
+        new (info_->zone()) CompilationDependencies(broker_, info_->zone());
   }
 
   // For WebAssembly compile entry point.

test/cctest/test-field-type-tracking.cc

@@ -664,7 +664,7 @@ static void TestGeneralizeField(int detach_property_at_index,
   // Create new maps by generalizing representation of propX field.
   CanonicalHandleScope canonical(isolate);
   JSHeapBroker broker(isolate, &zone);
-  CompilationDependencies dependencies(isolate, &zone);
+  CompilationDependencies dependencies(&broker, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
   dependencies.DependOnFieldType(map_ref, property_index);
@@ -1041,7 +1041,7 @@ static void TestReconfigureDataFieldAttribute_GeneralizeField(
   Zone zone(isolate->allocator(), ZONE_NAME);
   CanonicalHandleScope canonical(isolate);
   JSHeapBroker broker(isolate, &zone);
-  CompilationDependencies dependencies(isolate, &zone);
+  CompilationDependencies dependencies(&broker, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
   dependencies.DependOnFieldType(map_ref, kSplitProp);
@@ -1128,7 +1128,7 @@ static void TestReconfigureDataFieldAttribute_GeneralizeFieldTrivial(
   Zone zone(isolate->allocator(), ZONE_NAME);
   CanonicalHandleScope canonical(isolate);
   JSHeapBroker broker(isolate, &zone);
-  CompilationDependencies dependencies(isolate, &zone);
+  CompilationDependencies dependencies(&broker, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
   dependencies.DependOnFieldType(map_ref, kSplitProp);
@@ -1813,7 +1813,7 @@ static void TestReconfigureElementsKind_GeneralizeField(
   Zone zone(isolate->allocator(), ZONE_NAME);
   CanonicalHandleScope canonical(isolate);
   JSHeapBroker broker(isolate, &zone);
-  CompilationDependencies dependencies(isolate, &zone);
+  CompilationDependencies dependencies(&broker, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
   dependencies.DependOnFieldType(map_ref, kDiffProp);
@@ -1911,7 +1911,7 @@ static void TestReconfigureElementsKind_GeneralizeFieldTrivial(
   Zone zone(isolate->allocator(), ZONE_NAME);
   CanonicalHandleScope canonical(isolate);
   JSHeapBroker broker(isolate, &zone);
-  CompilationDependencies dependencies(isolate, &zone);
+  CompilationDependencies dependencies(&broker, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
 

test/unittests/compiler/constant-folding-reducer-unittest.cc

@@ -65,7 +65,7 @@ class ConstantFoldingReducerTest : public TypedGraphTest {
       : TypedGraphTest(3),
         broker_(isolate(), zone()),
         simplified_(zone()),
-        deps_(isolate(), zone()) {}
+        deps_(&broker_, zone()) {}
   ~ConstantFoldingReducerTest() override = default;
 
  protected:

test/unittests/compiler/js-call-reducer-unittest.cc

@@ -21,7 +21,7 @@ namespace compiler {
 class JSCallReducerTest : public TypedGraphTest {
  public:
   JSCallReducerTest()
-      : TypedGraphTest(3), javascript_(zone()), deps_(isolate(), zone()) {
+      : TypedGraphTest(3), javascript_(zone()), deps_(broker(), zone()) {
     broker()->SerializeStandardObjects();
   }
   ~JSCallReducerTest() override = default;

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

@@ -32,9 +32,8 @@ class JSCreateLoweringTest : public TypedGraphTest {
   JSCreateLoweringTest()
       : TypedGraphTest(3),
         javascript_(zone()),
-        deps_(isolate(), zone()),
-        handle_scope_(isolate()) {
-  }
+        deps_(broker(), zone()),
+        handle_scope_(isolate()) {}
   ~JSCreateLoweringTest() override = default;
 
  protected:

test/unittests/compiler/typed-optimization-unittest.cc

@@ -27,7 +27,7 @@ namespace typed_optimization_unittest {
 class TypedOptimizationTest : public TypedGraphTest {
  public:
   TypedOptimizationTest()
-      : TypedGraphTest(3), simplified_(zone()), deps_(isolate(), zone()) {}
+      : TypedGraphTest(3), simplified_(zone()), deps_(broker(), zone()) {}
   ~TypedOptimizationTest() override = default;
 
  protected: