Bug 1366377 - Unify StringSplitString ObjectGroup and fix Ion MCallOptimize. r=tcampbell

js/src/jit/CacheIR.cpp

@@ -3583,8 +3583,10 @@ bool
 CallIRGenerator::tryAttachStringSplit()
 {
     // Get the object group to use for this location.
-    RootedObjectGroup group(cx_, ObjectGroup::allocationSiteGroup(cx_, script_, pc_,
-                                                                  JSProto_Array, nullptr));
+    RootedObjectGroup group(cx_, ObjectGroupCompartment::getStringSplitStringGroup(cx_));
+    if (!group) {
+        return false;
+    }
 
     AutoAssertNoPendingException aanpe(cx_);
     Int32OperandId argcId(writer.setInputOperandId(0));

js/src/jit/MCallOptimize.cpp

@@ -1584,11 +1584,14 @@ IonBuilder::inlineStringSplitString(CallInfo& callInfo)
     if (resultConstStringSplit != InliningStatus_NotInlined)
         return resultConstStringSplit;
 
-    JSObject* templateObject = inspector->getTemplateObjectForNative(pc, js::intrinsic_StringSplitString);
-    if (!templateObject)
+    JSContext* cx = GetJitContext()->cx;
+    ObjectGroup* group = ObjectGroupCompartment::getStringSplitStringGroup(cx);
+    if (!group)
+        return InliningStatus_NotInlined;
+    if (group->maybePreliminaryObjects())
         return InliningStatus_NotInlined;
 
-    TypeSet::ObjectKey* retKey = TypeSet::ObjectKey::get(templateObject);
+    TypeSet::ObjectKey* retKey = TypeSet::ObjectKey::get(group);
     if (retKey->unknownProperties())
         return InliningStatus_NotInlined;
 
@@ -1602,12 +1605,7 @@ IonBuilder::inlineStringSplitString(CallInfo& callInfo)
     }
 
     callInfo.setImplicitlyUsedUnchecked();
-    MConstant* templateObjectDef = MConstant::New(alloc(), ObjectValue(*templateObject),
-                                                  constraints());
-    current->add(templateObjectDef);
-
-    MStringSplit* ins = MStringSplit::New(alloc(), constraints(), strArg, sepArg,
-                                          templateObjectDef);
+    MStringSplit* ins = MStringSplit::New(alloc(), constraints(), strArg, sepArg, group);
     current->add(ins);
     current->push(ins);
 

js/src/jit/MIR.h

@@ -7491,15 +7491,19 @@ class MSinCos
 };
 
 class MStringSplit
-  : public MTernaryInstruction,
+  : public MBinaryInstruction,
     public MixPolicy<StringPolicy<0>, StringPolicy<1> >::Data
 {
+    CompilerObjectGroup group_;
+
     MStringSplit(CompilerConstraintList* constraints, MDefinition* string, MDefinition* sep,
-                 MConstant* templateObject)
-      : MTernaryInstruction(string, sep, templateObject)
+                 ObjectGroup* group)
+      : MBinaryInstruction(string, sep),
+        group_(group)
     {
         setResultType(MIRType::Object);
-        setResultTypeSet(templateObject->resultTypeSet());
+        TemporaryTypeSet* types = MakeSingletonTypeSet(constraints, group);
+        setResultTypeSet(types);
     }
 
   public:
@@ -7507,11 +7511,8 @@ class MStringSplit
     TRIVIAL_NEW_WRAPPERS
     NAMED_OPERANDS((0, string), (1, separator))
 
-    JSObject* templateObject() const {
-        return &getOperand(2)->toConstant()->toObject();
-    }
     ObjectGroup* group() const {
-        return templateObject()->group();
+        return group_;
     }
     bool possiblyCalls() const override {
         return true;
@@ -7525,6 +7526,9 @@ class MStringSplit
     bool canRecoverOnBailout() const override {
         return true;
     }
+    bool appendRoots(MRootList& roots) const override {
+        return roots.append(group_);
+    }
 };
 
 // Returns the value to use as |this| value. See also ComputeThis and

js/src/jit/Recover.cpp

@@ -1044,7 +1044,10 @@ RStringSplit::recover(JSContext* cx, SnapshotIterator& iter) const
 {
     RootedString str(cx, iter.read().toString());
     RootedString sep(cx, iter.read().toString());
-    RootedObjectGroup group(cx, iter.read().toObject().group());
+    RootedObjectGroup group(cx, ObjectGroupCompartment::getStringSplitStringGroup(cx));
+    if (!group) {
+        return false;
+    }
     RootedValue result(cx);
 
     JSObject* res = str_split_string(cx, group, str, sep, INT32_MAX);

js/src/jit/Recover.h

@@ -485,7 +485,7 @@ class RRandom final : public RInstruction
 class RStringSplit final : public RInstruction
 {
   public:
-    RINSTRUCTION_HEADER_NUM_OP_(StringSplit, 3)
+    RINSTRUCTION_HEADER_NUM_OP_(StringSplit, 2)
 
     MOZ_MUST_USE bool recover(JSContext* cx, SnapshotIterator& iter) const override;
 };

js/src/vm/ObjectGroup.cpp

@@ -1693,6 +1693,7 @@ ObjectGroupCompartment::~ObjectGroupCompartment()
     js_delete(arrayObjectTable);
     js_delete(plainObjectTable);
     js_delete(allocationSiteTable);
+    stringSplitStringGroup = nullptr;
 }
 
 void
@@ -1739,6 +1740,44 @@ ObjectGroupCompartment::makeGroup(JSContext* cx, const Class* clasp,
     return group;
 }
 
+/* static */
+ObjectGroup*
+ObjectGroupCompartment::getStringSplitStringGroup(JSContext* cx)
+{
+    ObjectGroupCompartment& groups = cx->compartment()->objectGroups;
+
+    ObjectGroup* group = groups.stringSplitStringGroup.get();
+    if (group) {
+        return group;
+    }
+
+    // The following code is a specialized version of the code
+    // for ObjectGroup::allocationSiteGroup().
+
+    const Class* clasp = GetClassForProtoKey(JSProto_Array);
+
+    RootedObject proto(cx);
+    if (!GetBuiltinPrototype(cx, JSProto_Array, &proto))
+        return nullptr;
+    Rooted<TaggedProto> tagged(cx, TaggedProto(proto));
+
+    group = makeGroup(cx, clasp, tagged, /* initialFlags = */ 0);
+    if (!group)
+        return nullptr;
+
+    if (cx->options().unboxedArrays()) {
+        PreliminaryObjectArrayWithTemplate* preliminaryObjects =
+            cx->new_<PreliminaryObjectArrayWithTemplate>(nullptr);
+        if (preliminaryObjects)
+            group->setPreliminaryObjects(preliminaryObjects);
+        else
+            cx->recoverFromOutOfMemory();
+    }
+
+    groups.stringSplitStringGroup.set(group);
+    return group;
+}
+
 void
 ObjectGroupCompartment::addSizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                                size_t* allocationSiteTables,
@@ -1820,6 +1859,11 @@ ObjectGroupCompartment::sweep(FreeOp* fop)
         arrayObjectTable->sweep();
     if (plainObjectTable)
         plainObjectTable->sweep();
+    if (stringSplitStringGroup) {
+        if (JS::GCPolicy<ReadBarrieredObjectGroup>::needsSweep(&stringSplitStringGroup)) {
+            stringSplitStringGroup = nullptr;
+        }
+    }
 }
 
 void

js/src/vm/ObjectGroup.h

@@ -623,6 +623,14 @@ class ObjectGroupCompartment
     // Table for referencing types of objects keyed to an allocation site.
     AllocationSiteTable* allocationSiteTable;
 
+    // A single per-compartment ObjectGroup for all calls to StringSplitString.
+    // StringSplitString is always called from self-hosted code, and conceptually
+    // the return object for a string.split(string) operation should have a
+    // unified type.  Having a global group for this also allows us to remove
+    // the hash-table lookup that would be required if we allocated this group
+    // on the basis of call-site pc.
+    ReadBarrieredObjectGroup stringSplitStringGroup;
+
   public:
     struct NewEntry;
 
@@ -640,6 +648,8 @@ class ObjectGroupCompartment
                                   Handle<TaggedProto> proto,
                                   ObjectGroupFlags initialFlags = 0);
 
+    static ObjectGroup* getStringSplitStringGroup(JSContext* cx);
+
     void addSizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                 size_t* allocationSiteTables,
                                 size_t* arrayGroupTables,

js/src/vm/SelfHosting.cpp

@@ -1658,7 +1658,7 @@ js::intrinsic_StringSplitString(JSContext* cx, unsigned argc, Value* vp)
     RootedString string(cx, args[0].toString());
     RootedString sep(cx, args[1].toString());
 
-    RootedObjectGroup group(cx, ObjectGroup::callingAllocationSiteGroup(cx, JSProto_Array));
+    RootedObjectGroup group(cx, ObjectGroupCompartment::getStringSplitStringGroup(cx));
     if (!group)
         return false;
 
@@ -1683,7 +1683,7 @@ intrinsic_StringSplitStringLimit(JSContext* cx, unsigned argc, Value* vp)
     // because of Ion optimization.
     uint32_t limit = uint32_t(args[2].toNumber());
 
-    RootedObjectGroup group(cx, ObjectGroup::callingAllocationSiteGroup(cx, JSProto_Array));
+    RootedObjectGroup group(cx, ObjectGroupCompartment::getStringSplitStringGroup(cx));
     if (!group)
         return false;