Bug 1533890: Remove StringSplit call IC r=mgaudet
authorIain Ireland <iireland@mozilla.com>
Wed, 01 May 2019 20:32:15 +0000
changeset 531357 5d03ed4eeeb06b1bd28306e47ec0b0327690de25
parent 531356 2ee2a6c2095d3dd7e56a87f44742cb2f63c950b9
child 531358 cbf988d7f823f40af06d3124273626c6b63ee1f2
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1533890, 1366377
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1533890: Remove StringSplit call IC r=mgaudet I started changing StringSplit to attach outside of self-hosted code, to match ConstStringSplit. Upon closer examination, the StringSplit IC doesn't actually add any value, and we're better off deleting it. The generated code calls StringSplitHelper, which ends up doing almost exactly the same thing as the call to intrinsic_StringSplitString it replaces. When we first wrote the patch (bug 1366377), the advantage was that we got to skip a lookup to determine the group of the resulting object. However, a subsequent patch created a single group for every StringSplitString result, which is basically free to look up. I couldn't write a microbenchmark where the StringSplit IC made any difference, so let's just delete it and simplify our codebase. Depends on D29534 Differential Revision: https://phabricator.services.mozilla.com/D29535
js/src/jit/BaselineCacheIRCompiler.cpp
js/src/jit/CacheIR.cpp
js/src/jit/CacheIR.h
js/src/jit/IonCacheIRCompiler.cpp
js/src/jit/VMFunctionList-inl.h
js/src/jit/VMFunctions.cpp
js/src/jit/VMFunctions.h
--- a/js/src/jit/BaselineCacheIRCompiler.cpp
+++ b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -883,44 +883,16 @@ bool BaselineCacheIRCompiler::emitLoadSt
   AutoOutputRegister output(*this);
   AutoScratchRegisterMaybeOutput scratch(allocator, masm, output);
 
   masm.loadPtr(stubAddress(reader.stubOffset()), scratch);
   masm.tagValue(JSVAL_TYPE_STRING, scratch, output.valueReg());
   return true;
 }
 
-bool BaselineCacheIRCompiler::emitCallStringSplitResult() {
-  JitSpew(JitSpew_Codegen, __FUNCTION__);
-  Register str = allocator.useRegister(masm, reader.stringOperandId());
-  Register sep = allocator.useRegister(masm, reader.stringOperandId());
-  Address groupAddr(stubAddress(reader.stubOffset()));
-
-  AutoScratchRegister scratch(allocator, masm);
-  allocator.discardStack(masm);
-
-  AutoStubFrame stubFrame(*this);
-  stubFrame.enter(masm, scratch);
-
-  // Load the group in the scratch register.
-  masm.loadPtr(groupAddr, scratch);
-
-  masm.Push(Imm32(INT32_MAX));
-  masm.Push(scratch);
-  masm.Push(sep);
-  masm.Push(str);
-
-  using Fn = bool (*)(JSContext*, HandleString, HandleString, HandleObjectGroup,
-                      uint32_t limit, MutableHandleValue);
-  callVM<Fn, StringSplitHelper>(masm);
-
-  stubFrame.leave(masm);
-  return true;
-}
-
 bool BaselineCacheIRCompiler::emitCallConstStringSplitResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   Address resultTemplateAddr(stubAddress(reader.stubOffset()));
 
   AutoScratchRegister scratch(allocator, masm);
   allocator.discardStack(masm);
 
   AutoStubFrame stubFrame(*this);
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -4578,64 +4578,16 @@ CallIRGenerator::CallIRGenerator(JSConte
       callee_(callee),
       thisval_(thisval),
       newTarget_(newTarget),
       args_(args),
       typeCheckInfo_(cx, /* needsTypeBarrier = */ true),
       cacheIRStubKind_(BaselineCacheIRStubKind::Regular),
       isFirstStub_(isFirstStub) {}
 
-AttachDecision CallIRGenerator::tryAttachStringSplit() {
-  // Only optimize StringSplitString(str, str)
-  if (argc_ != 2 || !args_[0].isString() || !args_[1].isString()) {
-    return AttachDecision::NoAction;
-  }
-
-  // Get the object group to use for this location.
-  RootedObjectGroup group(cx_,
-                          ObjectGroupRealm::getStringSplitStringGroup(cx_));
-  if (!group) {
-    cx_->clearPendingException();
-    return AttachDecision::NoAction;
-  }
-
-  Int32OperandId argcId(writer.setInputOperandId(0));
-
-  // Ensure argc == 2.
-  writer.guardSpecificInt32Immediate(argcId, 2);
-
-  // Ensure callee is the |String_split| native function.
-  ValOperandId calleeValId =
-      writer.loadArgumentFixedSlot(ArgumentKind::Callee, argc_);
-  ObjOperandId calleeObjId = writer.guardIsObject(calleeValId);
-  writer.guardSpecificNativeFunction(calleeObjId,
-                                     js::intrinsic_StringSplitString);
-
-  // Ensure arg0 is a string.
-  ValOperandId arg0ValId =
-      writer.loadArgumentFixedSlot(ArgumentKind::Arg0, argc_);
-  StringOperandId arg0StrId = writer.guardIsString(arg0ValId);
-
-  // Ensure arg1 is a string.
-  ValOperandId arg1ValId =
-      writer.loadArgumentFixedSlot(ArgumentKind::Arg1, argc_);
-  StringOperandId arg1StrId = writer.guardIsString(arg1ValId);
-
-  // Call custom string splitter VM-function.
-  writer.callStringSplitResult(arg0StrId, arg1StrId, group);
-  writer.typeMonitorResult();
-
-  cacheIRStubKind_ = BaselineCacheIRStubKind::Monitored;
-  trackAttached("StringSplitString");
-
-  TypeScript::Monitor(cx_, script_, pc_, TypeSet::ObjectType(group));
-
-  return AttachDecision::Attach;
-}
-
 AttachDecision CallIRGenerator::tryAttachArrayPush() {
   // Only optimize on obj.push(val);
   if (argc_ != 1 || !thisval_.isObject()) {
     return AttachDecision::NoAction;
   }
 
   // Where |obj| is a native array.
   RootedObject thisobj(cx_, &thisval_.toObject());
@@ -4964,19 +4916,16 @@ AttachDecision CallIRGenerator::tryAttac
   }
 
   // Other functions are only optimized for normal calls.
   if (op_ != JSOP_CALL && op_ != JSOP_CALL_IGNORES_RV) {
     return AttachDecision::NoAction;
   }
 
   // Check for special-cased native functions.
-  if (callee->native() == js::intrinsic_StringSplitString) {
-    return tryAttachStringSplit();
-  }
   if (callee->native() == js::array_push) {
     return tryAttachArrayPush();
   }
   if (callee->native() == js::array_join) {
     return tryAttachArrayJoin();
   }
   if (callee->native() == intrinsic_IsSuspendedGenerator) {
     return tryAttachIsSuspendedGenerator();
--- a/js/src/jit/CacheIR.h
+++ b/js/src/jit/CacheIR.h
@@ -363,17 +363,16 @@ extern const uint32_t ArgLengths[];
   _(DoubleDecResult, Id)                                                       \
   _(LoadInt32TruthyResult, Id)                                                 \
   _(LoadDoubleTruthyResult, Id)                                                \
   _(LoadStringTruthyResult, Id)                                                \
   _(LoadObjectTruthyResult, Id)                                                \
   _(LoadValueResult, Field)                                                    \
   _(LoadNewObjectFromTemplateResult, Field, UInt32, UInt32)                    \
                                                                                \
-  _(CallStringSplitResult, Id, Id, Field)                                      \
   _(CallConstStringSplitResult, Field)                                         \
   _(CallStringConcatResult, Id, Id)                                            \
   _(CallStringObjectConcatResult, Id, Id)                                      \
   _(CallIsSuspendedGeneratorResult, Id)                                        \
                                                                                \
   _(CompareStringResult, Id, Id, Byte)                                         \
   _(CompareObjectResult, Id, Id, Byte)                                         \
   _(CompareSymbolResult, Id, Id, Byte)                                         \
@@ -1728,22 +1727,16 @@ class MOZ_RAII CacheIRWriter : public JS
   void callStringConcatResult(StringOperandId lhs, StringOperandId rhs) {
     writeOpWithOperandId(CacheOp::CallStringConcatResult, lhs);
     writeOperandId(rhs);
   }
   void callStringObjectConcatResult(ValOperandId lhs, ValOperandId rhs) {
     writeOpWithOperandId(CacheOp::CallStringObjectConcatResult, lhs);
     writeOperandId(rhs);
   }
-  void callStringSplitResult(StringOperandId str, StringOperandId sep,
-                             ObjectGroup* group) {
-    writeOpWithOperandId(CacheOp::CallStringSplitResult, str);
-    writeOperandId(sep);
-    addStubField(uintptr_t(group), StubField::Type::ObjectGroup);
-  }
   FieldOffset callConstStringSplitResult(ArrayObject* resultTemplate) {
     writeOp(CacheOp::CallConstStringSplitResult);
     return addStubField(uintptr_t(resultTemplate), StubField::Type::JSObject);
   }
 
   void compareStringResult(uint32_t op, StringOperandId lhs,
                            StringOperandId rhs) {
     writeOpWithOperandId(CacheOp::CompareStringResult, lhs);
@@ -2386,17 +2379,16 @@ class MOZ_RAII CallIRGenerator : public 
                                     MutableHandleObject result,
                                     bool* skipAttach);
   bool getTemplateObjectForNative(HandleFunction calleeFunc,
                                   MutableHandleObject result);
   bool getTemplateObjectForClassHook(HandleObject calleeObj,
                                      MutableHandleObject result);
 
   // Regular stubs
-  AttachDecision tryAttachStringSplit();
   AttachDecision tryAttachArrayPush();
   AttachDecision tryAttachArrayJoin();
   AttachDecision tryAttachIsSuspendedGenerator();
   AttachDecision tryAttachFunCall();
   AttachDecision tryAttachFunApply();
   AttachDecision tryAttachSelfHosted(HandleFunction calleeFunc);
   AttachDecision tryAttachCallScripted(HandleFunction calleeFunc);
   AttachDecision tryAttachSpecialCaseCallNative(HandleFunction calleeFunc);
--- a/js/src/jit/IonCacheIRCompiler.cpp
+++ b/js/src/jit/IonCacheIRCompiler.cpp
@@ -1272,21 +1272,16 @@ bool IonCacheIRCompiler::emitLoadEnviron
   return true;
 }
 
 bool IonCacheIRCompiler::emitLoadStringResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   MOZ_CRASH("not used in ion");
 }
 
-bool IonCacheIRCompiler::emitCallStringSplitResult() {
-  JitSpew(JitSpew_Codegen, __FUNCTION__);
-  MOZ_CRASH("not used in ion");
-}
-
 bool IonCacheIRCompiler::emitCallConstStringSplitResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   MOZ_CRASH("not used in ion");
 }
 
 bool IonCacheIRCompiler::emitCompareStringResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoSaveLiveRegisters save(*this);
--- a/js/src/jit/VMFunctionList-inl.h
+++ b/js/src/jit/VMFunctionList-inl.h
@@ -222,17 +222,16 @@ namespace jit {
   _(SingletonObjectLiteralOperation, js::SingletonObjectLiteralOperation)      \
   _(StartDynamicModuleImport, js::StartDynamicModuleImport)                    \
   _(StrictlyEqual, js::jit::StrictlyEqual<js::jit::EqualityKind::Equal>)       \
   _(StrictlyNotEqual, js::jit::StrictlyEqual<js::jit::EqualityKind::NotEqual>) \
   _(StringFlatReplaceString, js::StringFlatReplaceString)                      \
   _(StringFromCharCode, js::jit::StringFromCharCode)                           \
   _(StringFromCodePoint, js::jit::StringFromCodePoint)                         \
   _(StringReplace, js::jit::StringReplace)                                     \
-  _(StringSplitHelper, js::jit::StringSplitHelper)                             \
   _(StringSplitString, js::StringSplitString)                                  \
   _(StringToLowerCase, js::StringToLowerCase)                                  \
   _(StringToNumber, js::StringToNumber)                                        \
   _(StringToUpperCase, js::StringToUpperCase)                                  \
   _(StringsCompareGreaterThanOrEquals,                                         \
     js::jit::StringsCompare<ComparisonKind::GreaterThanOrEqual>)               \
   _(StringsCompareLessThan, js::jit::StringsCompare<ComparisonKind::LessThan>) \
   _(StringsEqual, js::jit::StringsEqual<js::jit::EqualityKind::Equal>)         \
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -440,28 +440,16 @@ bool StringsCompare(JSContext* cx, Handl
 
 template bool StringsCompare<ComparisonKind::LessThan>(JSContext* cx,
                                                        HandleString lhs,
                                                        HandleString rhs,
                                                        bool* res);
 template bool StringsCompare<ComparisonKind::GreaterThanOrEqual>(
     JSContext* cx, HandleString lhs, HandleString rhs, bool* res);
 
-bool StringSplitHelper(JSContext* cx, HandleString str, HandleString sep,
-                       HandleObjectGroup group, uint32_t limit,
-                       MutableHandleValue result) {
-  JSObject* resultObj = StringSplitString(cx, group, str, sep, limit);
-  if (!resultObj) {
-    return false;
-  }
-
-  result.setObject(*resultObj);
-  return true;
-}
-
 bool ArrayPopDense(JSContext* cx, HandleObject obj, MutableHandleValue rval) {
   MOZ_ASSERT(obj->is<ArrayObject>());
 
   AutoDetectInvalidation adi(cx, rval);
 
   JS::AutoValueArray<2> argv(cx);
   argv[0].setUndefined();
   argv[1].setObject(*obj);
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -861,20 +861,16 @@ template <EqualityKind Kind>
 bool StringsEqual(JSContext* cx, HandleString lhs, HandleString rhs, bool* res);
 
 enum class ComparisonKind : bool { GreaterThanOrEqual, LessThan };
 
 template <ComparisonKind Kind>
 bool StringsCompare(JSContext* cx, HandleString lhs, HandleString rhs,
                     bool* res);
 
-MOZ_MUST_USE bool StringSplitHelper(JSContext* cx, HandleString str,
-                                    HandleString sep, HandleObjectGroup group,
-                                    uint32_t limit, MutableHandleValue result);
-
 MOZ_MUST_USE bool ArrayPopDense(JSContext* cx, HandleObject obj,
                                 MutableHandleValue rval);
 MOZ_MUST_USE bool ArrayPushDense(JSContext* cx, HandleArrayObject arr,
                                  HandleValue v, uint32_t* length);
 MOZ_MUST_USE bool ArrayShiftDense(JSContext* cx, HandleObject obj,
                                   MutableHandleValue rval);
 JSString* ArrayJoin(JSContext* cx, HandleObject array, HandleString sep);
 MOZ_MUST_USE bool SetArrayLength(JSContext* cx, HandleObject obj,