[turbofan] Brokerize reduction of RegExp.prototype.test

Bug: v8:7790
Change-Id: If2a8123e5657f0ea9a007b5f1a82e9d1a91c80f9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1679493Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62763}

src/compiler/access-info.cc

@@ -332,9 +332,9 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
   MaybeHandle<Map> field_map;
   MapRef map_ref(broker(), map);
   ZoneVector<CompilationDependency const*> unrecorded_dependencies(zone());
+  map_ref.SerializeOwnDescriptor(descriptor);
   if (details_representation.IsSmi()) {
     field_type = Type::SignedSmall();
-    map_ref.SerializeOwnDescriptor(descriptor);
     unrecorded_dependencies.push_back(
         dependencies()->FieldRepresentationDependencyOffTheRecord(map_ref,
                                                                   descriptor));
@@ -354,19 +354,23 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
       // The field type was cleared by the GC, so we don't know anything
       // about the contents now.
     }
-    map_ref.SerializeOwnDescriptor(descriptor);
     unrecorded_dependencies.push_back(
         dependencies()->FieldRepresentationDependencyOffTheRecord(map_ref,
                                                                   descriptor));
     if (descriptors_field_type->IsClass()) {
-      unrecorded_dependencies.push_back(
-          dependencies()->FieldTypeDependencyOffTheRecord(map_ref, descriptor));
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map(descriptors_field_type->AsClass(), isolate());
       field_type = Type::For(MapRef(broker(), map));
       field_map = MaybeHandle<Map>(map);
     }
+  } else {
+    CHECK(details_representation.IsTagged());
   }
+  // TODO(turbofan): We may want to do this only depending on the use
+  // of the access info.
+  unrecorded_dependencies.push_back(
+      dependencies()->FieldTypeDependencyOffTheRecord(map_ref, descriptor));
+
   PropertyConstness constness;
   if (details.IsReadOnly() && !details.IsConfigurable()) {
     constness = PropertyConstness::kConst;

src/compiler/access-info.h

@@ -112,9 +112,9 @@ class PropertyAccessInfo final {
 
   Kind kind() const { return kind_; }
   MaybeHandle<JSObject> holder() const {
-    // This CHECK tries to protect against using the access info without
-    // recording its dependencies first.
-    CHECK(unrecorded_dependencies_.empty());
+    // TODO(neis): There was a CHECK here that tries to protect against
+    // using the access info without recording its dependencies first.
+    // Find a more suitable place for it.
     return holder_;
   }
   MaybeHandle<Map> transition_map() const { return transition_map_; }

src/compiler/heap-refs.h

@@ -231,6 +231,12 @@ class JSObjectRef : public HeapObjectRef {
   double RawFastDoublePropertyAt(FieldIndex index) const;
   ObjectRef RawFastPropertyAt(FieldIndex index) const;
 
+  // Return the value of the property identified by the field {index}
+  // if {index} is known to be an own data property of the object.
+  base::Optional<ObjectRef> GetOwnProperty(Representation field_representation,
+                                           FieldIndex index,
+                                           bool serialize = false) const;
+
   FixedArrayBaseRef elements() const;
   void SerializeElements();
   void EnsureElementsTenured();
@@ -376,6 +382,7 @@ class ContextRef : public HeapObjectRef {
   V(Map, map_key_iterator_map)                   \
   V(Map, map_key_value_iterator_map)             \
   V(Map, map_value_iterator_map)                 \
+  V(JSFunction, regexp_exec_function)            \
   V(Map, set_key_value_iterator_map)             \
   V(Map, set_value_iterator_map)
 

src/compiler/js-call-reducer.cc

@@ -7096,8 +7096,11 @@ Reduction JSCallReducer::ReduceNumberParseInt(Node* node) {
 }
 
 Reduction JSCallReducer::ReduceRegExpPrototypeTest(Node* node) {
+  DisallowHeapAccessIf disallow_heap_access(FLAG_concurrent_inlining);
+
   if (FLAG_force_slow_path) return NoChange();
   if (node->op()->ValueInputCount() < 3) return NoChange();
+
   CallParameters const& p = CallParametersOf(node->op());
   if (p.speculation_mode() == SpeculationMode::kDisallowSpeculation) {
     return NoChange();
@@ -7114,13 +7117,24 @@ Reduction JSCallReducer::ReduceRegExpPrototypeTest(Node* node) {
   }
   MapHandles const& regexp_maps = inference.GetMaps();
 
-  // Compute property access info for "exec" on {resolution}.
   ZoneVector<PropertyAccessInfo> access_infos(graph()->zone());
   AccessInfoFactory access_info_factory(broker(), dependencies(),
                                         graph()->zone());
-  access_info_factory.ComputePropertyAccessInfos(
-      MapHandles(regexp_maps.begin(), regexp_maps.end()),
-      factory()->exec_string(), AccessMode::kLoad, &access_infos);
+  if (!FLAG_concurrent_inlining) {
+    // Compute property access info for "exec" on {resolution}.
+    access_info_factory.ComputePropertyAccessInfos(
+        MapHandles(regexp_maps.begin(), regexp_maps.end()),
+        factory()->exec_string(), AccessMode::kLoad, &access_infos);
+  } else {
+    // Obtain precomputed access infos from the broker.
+    for (auto map : regexp_maps) {
+      MapRef map_ref(broker(), map);
+      PropertyAccessInfo access_info =
+          broker()->GetAccessInfoForLoadingExec(map_ref);
+      access_infos.push_back(access_info);
+    }
+  }
+
   PropertyAccessInfo ai_exec =
       access_info_factory.FinalizePropertyAccessInfosAsOne(access_infos,
                                                            AccessMode::kLoad);
@@ -7132,34 +7146,24 @@ Reduction JSCallReducer::ReduceRegExpPrototypeTest(Node* node) {
     // Do not reduce if the exec method is not on the prototype chain.
     if (!ai_exec.holder().ToHandle(&holder)) return inference.NoChange();
 
+    JSObjectRef holder_ref(broker(), holder);
+
     // Bail out if the exec method is not the original one.
-    Handle<Object> constant = JSObject::FastPropertyAt(
-        holder, ai_exec.field_representation(), ai_exec.field_index());
-    if (!constant.is_identical_to(isolate()->regexp_exec_function())) {
+    base::Optional<ObjectRef> constant = holder_ref.GetOwnProperty(
+        ai_exec.field_representation(), ai_exec.field_index());
+    if (!constant.has_value() ||
+        !constant->equals(native_context().regexp_exec_function())) {
       return inference.NoChange();
     }
 
-    // Protect the exec method change in the holder.
-    Handle<Object> exec_on_proto;
-    MapRef holder_map(broker(), handle(holder->map(), isolate()));
-    Handle<DescriptorArray> descriptors(
-        holder_map.object()->instance_descriptors(), isolate());
-    int descriptor_index =
-        descriptors->Search(*(factory()->exec_string()), *holder_map.object());
-    CHECK_NE(descriptor_index, DescriptorArray::kNotFound);
-    holder_map.SerializeOwnDescriptors();
-    dependencies()->DependOnFieldType(holder_map, descriptor_index);
-  } else {
-    return inference.NoChange();
-  }
-
-  // Add proper dependencies on the {regexp}s [[Prototype]]s.
-  Handle<JSObject> holder;
-  if (ai_exec.holder().ToHandle(&holder)) {
+    // Add proper dependencies on the {regexp}s [[Prototype]]s.
     dependencies()->DependOnStablePrototypeChains(
         ai_exec.receiver_maps(), kStartAtPrototype,
         JSObjectRef(broker(), holder));
+  } else {
+    return inference.NoChange();
   }
+
   inference.RelyOnMapsPreferStability(dependencies(), jsgraph(), &effect,
                                       control, p.feedback());
 

src/compiler/js-heap-broker.cc

@@ -259,6 +259,12 @@ class JSObjectField {
   uint64_t number_bits_ = 0;
 };
 
+struct FieldIndexHasher {
+  size_t operator()(FieldIndex field_index) const {
+    return field_index.index();
+  }
+};
+
 class JSObjectData : public HeapObjectData {
  public:
   JSObjectData(JSHeapBroker* broker, ObjectData** storage,
@@ -281,6 +287,9 @@ class JSObjectData : public HeapObjectData {
 
   ObjectData* GetOwnConstantElement(JSHeapBroker* broker, uint32_t index,
                                     bool serialize);
+  ObjectData* GetOwnProperty(JSHeapBroker* broker,
+                             Representation representation,
+                             FieldIndex field_index, bool serialize);
 
   // This method is only used to assert our invariants.
   bool cow_or_empty_elements_tenured() const;
@@ -305,6 +314,12 @@ class JSObjectData : public HeapObjectData {
   // non-configurable, or (2) are known not to (possibly they don't exist at
   // all). In case (2), the second pair component is nullptr.
   ZoneVector<std::pair<uint32_t, ObjectData*>> own_constant_elements_;
+  // Properties that either:
+  // (1) are known to exist directly on the object, or
+  // (2) are known not to (possibly they don't exist at all).
+  // In case (2), the second pair component is nullptr.
+  // For simplicity, this may in theory overlap with inobject_fields_.
+  ZoneUnorderedMap<FieldIndex, ObjectData*, FieldIndexHasher> own_properties_;
 };
 
 void JSObjectData::SerializeObjectCreateMap(JSHeapBroker* broker) {
@@ -340,6 +355,15 @@ base::Optional<ObjectRef> GetOwnElementFromHeap(JSHeapBroker* broker,
   }
   return base::nullopt;
 }
+
+ObjectRef GetOwnPropertyFromHeap(JSHeapBroker* broker,
+                                 Handle<JSObject> receiver,
+                                 Representation representation,
+                                 FieldIndex field_index) {
+  Handle<Object> constant =
+      JSObject::FastPropertyAt(receiver, representation, field_index);
+  return ObjectRef(broker, constant);
+}
 }  // namespace
 
 ObjectData* JSObjectData::GetOwnConstantElement(JSHeapBroker* broker,
@@ -361,6 +385,27 @@ ObjectData* JSObjectData::GetOwnConstantElement(JSHeapBroker* broker,
   return result;
 }
 
+ObjectData* JSObjectData::GetOwnProperty(JSHeapBroker* broker,
+                                         Representation representation,
+                                         FieldIndex field_index,
+                                         bool serialize) {
+  auto p = own_properties_.find(field_index);
+  if (p != own_properties_.end()) return p->second;
+
+  if (!serialize) {
+    TRACE_MISSING(broker, "knowledge about property with index "
+                              << field_index.property_index() << " on "
+                              << this);
+    return nullptr;
+  }
+
+  ObjectRef property = GetOwnPropertyFromHeap(
+      broker, Handle<JSObject>::cast(object()), representation, field_index);
+  ObjectData* result(property.data());
+  own_properties_.insert(std::make_pair(field_index, result));
+  return result;
+}
+
 class JSTypedArrayData : public JSObjectData {
  public:
   JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage,
@@ -1259,7 +1304,8 @@ JSObjectData::JSObjectData(JSHeapBroker* broker, ObjectData** storage,
                            Handle<JSObject> object)
     : HeapObjectData(broker, storage, object),
       inobject_fields_(broker->zone()),
-      own_constant_elements_(broker->zone()) {}
+      own_constant_elements_(broker->zone()),
+      own_properties_(broker->zone()) {}
 
 FixedArrayData::FixedArrayData(JSHeapBroker* broker, ObjectData** storage,
                                Handle<FixedArray> object)
@@ -2067,7 +2113,8 @@ JSHeapBroker::JSHeapBroker(Isolate* isolate, Zone* broker_zone,
       tracing_enabled_(tracing_enabled),
       feedback_(zone()),
       bytecode_analyses_(zone()),
-      ais_for_loading_then_(zone()) {
+      ais_for_loading_then_(zone()),
+      ais_for_loading_exec_(zone()) {
   // Note that this initialization of the refs_ pointer with the minimal
   // initial capacity is redundant in the normal use case (concurrent
   // compilation enabled, standard objects to be serialized), as the map
@@ -3259,6 +3306,19 @@ base::Optional<ObjectRef> ObjectRef::GetOwnConstantElement(
   return ObjectRef(broker(), element);
 }
 
+base::Optional<ObjectRef> JSObjectRef::GetOwnProperty(
+    Representation field_representation, FieldIndex index,
+    bool serialize) const {
+  if (broker()->mode() == JSHeapBroker::kDisabled) {
+    return GetOwnPropertyFromHeap(broker(), Handle<JSObject>::cast(object()),
+                                  field_representation, index);
+  }
+  ObjectData* property = data()->AsJSObject()->GetOwnProperty(
+      broker(), field_representation, index, serialize);
+  if (property == nullptr) return base::nullopt;
+  return ObjectRef(broker(), property);
+}
+
 base::Optional<ObjectRef> JSArrayRef::GetOwnCowElement(uint32_t index,
                                                        bool serialize) const {
   if (broker()->mode() == JSHeapBroker::kDisabled) {
@@ -4040,6 +4100,32 @@ void JSHeapBroker::CreateAccessInfoForLoadingThen(
   }
 }
 
+PropertyAccessInfo JSHeapBroker::GetAccessInfoForLoadingExec(MapRef map) {
+  auto access_info = ais_for_loading_exec_.find(map);
+  if (access_info == ais_for_loading_exec_.end()) {
+    TRACE_BROKER_MISSING(this,
+                         "access info for property 'exec' on map " << map);
+    return PropertyAccessInfo::Invalid(zone());
+  }
+  return access_info->second;
+}
+
+PropertyAccessInfo const& JSHeapBroker::CreateAccessInfoForLoadingExec(
+    MapRef map, CompilationDependencies* dependencies) {
+  auto access_info = ais_for_loading_exec_.find(map);
+  if (access_info != ais_for_loading_exec_.end()) {
+    return access_info->second;
+  }
+
+  ZoneVector<PropertyAccessInfo> access_infos(zone());
+  AccessInfoFactory access_info_factory(this, dependencies, zone());
+  PropertyAccessInfo ai_exec = access_info_factory.ComputePropertyAccessInfo(
+      map.object(), isolate()->factory()->exec_string(), AccessMode::kLoad);
+
+  auto inserted_ai = ais_for_loading_exec_.insert(std::make_pair(map, ai_exec));
+  return inserted_ai.first->second;
+}
+
 ElementAccessFeedback const* ProcessedFeedback::AsElementAccess() const {
   CHECK_EQ(kElementAccess, kind());
   return static_cast<ElementAccessFeedback const*>(this);

src/compiler/js-heap-broker.h

@@ -120,6 +120,9 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   PropertyAccessInfo GetAccessInfoForLoadingThen(MapRef map);
   void CreateAccessInfoForLoadingThen(MapRef map,
                                       CompilationDependencies* dependencies);
+  PropertyAccessInfo GetAccessInfoForLoadingExec(MapRef map);
+  PropertyAccessInfo const& CreateAccessInfoForLoadingExec(
+      MapRef map, CompilationDependencies* dependencies);
 
   std::ostream& Trace();
   void IncrementTracingIndentation();
@@ -154,6 +157,7 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
                            ObjectRef::Equal>
       MapToAccessInfos;
   MapToAccessInfos ais_for_loading_then_;
+  MapToAccessInfos ais_for_loading_exec_;
 
   static const size_t kMinimalRefsBucketCount = 8;     // must be power of 2
   static const size_t kInitialRefsBucketCount = 1024;  // must be power of 2

src/compiler/serializer-for-background-compilation.cc

@@ -7,6 +7,7 @@
 #include <sstream>
 
 #include "src/compiler/bytecode-analysis.h"
+#include "src/compiler/compilation-dependencies.h"
 #include "src/compiler/js-heap-broker.h"
 #include "src/compiler/vector-slot-pair.h"
 #include "src/handles/handles-inl.h"
@@ -14,6 +15,7 @@
 #include "src/interpreter/bytecode-array-iterator.h"
 #include "src/objects/code.h"
 #include "src/objects/js-array-inl.h"
+#include "src/objects/js-regexp-inl.h"
 #include "src/objects/shared-function-info-inl.h"
 #include "src/zone/zone.h"
 
@@ -1229,6 +1231,14 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
         ProcessHintsForPromiseResolve(resolution_hints);
       }
       break;
+    case Builtins::kRegExpPrototypeTest: {
+      // For JSCallReducer::ReduceRegExpPrototypeTest.
+      if (arguments.size() >= 1) {
+        Hints const& regexp_hints = arguments[0];
+        ProcessHintsForRegExpTest(regexp_hints);
+      }
+      break;
+    }
     default:
       break;
   }
@@ -1266,6 +1276,44 @@ void SerializerForBackgroundCompilation::ProcessMapHintsForPromises(
   }
 }
 
+PropertyAccessInfo SerializerForBackgroundCompilation::ProcessMapForRegExpTest(
+    MapRef map) {
+  PropertyAccessInfo ai_exec =
+      broker()->CreateAccessInfoForLoadingExec(map, dependencies());
+
+  Handle<JSObject> holder;
+  if (ai_exec.IsDataConstant() && ai_exec.holder().ToHandle(&holder)) {
+    // The property is on the prototype chain.
+    JSObjectRef holder_ref(broker(), holder);
+    holder_ref.GetOwnProperty(ai_exec.field_representation(),
+                              ai_exec.field_index(), true);
+  }
+  return ai_exec;
+}
+
+void SerializerForBackgroundCompilation::ProcessHintsForRegExpTest(
+    Hints const& regexp_hints) {
+  for (auto hint : regexp_hints.constants()) {
+    if (!hint->IsJSRegExp()) continue;
+    Handle<JSRegExp> regexp(Handle<JSRegExp>::cast(hint));
+    Handle<Map> regexp_map(regexp->map(), broker()->isolate());
+    PropertyAccessInfo ai_exec =
+        ProcessMapForRegExpTest(MapRef(broker(), regexp_map));
+    Handle<JSObject> holder;
+    if (ai_exec.IsDataConstant() && !ai_exec.holder().ToHandle(&holder)) {
+      // The property is on the object itself.
+      JSObjectRef holder_ref(broker(), regexp);
+      holder_ref.GetOwnProperty(ai_exec.field_representation(),
+                                ai_exec.field_index(), true);
+    }
+  }
+
+  for (auto map : regexp_hints.maps()) {
+    if (!map->IsJSRegExpMap()) continue;
+    ProcessMapForRegExpTest(MapRef(broker(), map));
+  }
+}
+
 void SerializerForBackgroundCompilation::ContributeToJumpTargetEnvironment(
     int target_offset) {
   auto it = jump_target_environments_.find(target_offset);

src/compiler/serializer-for-background-compilation.h

@@ -387,6 +387,8 @@ class SerializerForBackgroundCompilation {
                                   FeedbackSlot slot, AccessMode mode);
   void ProcessMapHintsForPromises(Hints const& receiver_hints);
   void ProcessHintsForPromiseResolve(Hints const& resolution_hints);
+  void ProcessHintsForRegExpTest(Hints const& regexp_hints);
+  PropertyAccessInfo ProcessMapForRegExpTest(MapRef map);
 
   GlobalAccessFeedback const* ProcessFeedbackForGlobalAccess(FeedbackSlot slot);
   NamedAccessFeedback const* ProcessFeedbackMapsForNamedAccess(

test/mjsunit/harmony/regexp-overriden-exec.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --no-always-opt --opt
+
+const r = RegExp('bla');
+
+function foo() {
+  r.test('string');
+}
+
+%PrepareFunctionForOptimization(foo);
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();
+assertOptimized(foo);
+
+r.__proto__.exec = function() {
+  return null;
+}
+Object.freeze(r.__proto__);
+
+foo();
+assertUnoptimized(foo);