Bug 956051 - Inline str_replace with string, string as arguments. r=nbp, r=jandem

js/src/jit/CodeGenerator.cpp

@@ -789,7 +789,7 @@ CodeGenerator::visitRegExpTest(LRegExpTest *lir)
 }
 
 typedef JSString *(*RegExpReplaceFn)(JSContext *, HandleString, HandleObject, HandleString);
-static const VMFunction RegExpReplaceInfo = FunctionInfo<RegExpReplaceFn>(regexp_replace);
+static const VMFunction RegExpReplaceInfo = FunctionInfo<RegExpReplaceFn>(RegExpReplace);
 
 bool
 CodeGenerator::visitRegExpReplace(LRegExpReplace *lir)
@@ -799,7 +799,7 @@ CodeGenerator::visitRegExpReplace(LRegExpReplace *lir)
     else
         pushArg(ToRegister(lir->replacement()));
 
-    pushArg(ToRegister(lir->regexp()));
+    pushArg(ToRegister(lir->pattern()));
 
     if (lir->string()->isConstant())
         pushArg(ImmGCPtr(lir->string()->toConstant()->toString()));
@@ -809,6 +809,31 @@ CodeGenerator::visitRegExpReplace(LRegExpReplace *lir)
     return callVM(RegExpReplaceInfo, lir);
 }
 
+typedef JSString *(*StringReplaceFn)(JSContext *, HandleString, HandleString, HandleString);
+static const VMFunction StringReplaceInfo = FunctionInfo<StringReplaceFn>(StringReplace);
+
+bool
+CodeGenerator::visitStringReplace(LStringReplace *lir)
+{
+    if (lir->replacement()->isConstant())
+        pushArg(ImmGCPtr(lir->replacement()->toConstant()->toString()));
+    else
+        pushArg(ToRegister(lir->replacement()));
+
+    if (lir->pattern()->isConstant())
+        pushArg(ImmGCPtr(lir->pattern()->toConstant()->toString()));
+    else
+        pushArg(ToRegister(lir->pattern()));
+
+    if (lir->string()->isConstant())
+        pushArg(ImmGCPtr(lir->string()->toConstant()->toString()));
+    else
+        pushArg(ToRegister(lir->string()));
+
+    return callVM(StringReplaceInfo, lir);
+}
+
+
 typedef JSObject *(*LambdaFn)(JSContext *, HandleFunction, HandleObject);
 static const VMFunction LambdaInfo =
     FunctionInfo<LambdaFn>(js::Lambda);

js/src/jit/CodeGenerator.h

@@ -93,6 +93,7 @@ class CodeGenerator : public CodeGeneratorSpecific
     bool visitRegExpExec(LRegExpExec *lir);
     bool visitRegExpTest(LRegExpTest *lir);
     bool visitRegExpReplace(LRegExpReplace *lir);
+    bool visitStringReplace(LStringReplace *lir);
     bool visitLambda(LLambda *lir);
     bool visitLambdaForSingleton(LLambdaForSingleton *lir);
     bool visitLambdaPar(LLambdaPar *lir);

js/src/jit/IonBuilder.h

@@ -629,7 +629,7 @@ class IonBuilder : public MIRGenerator
     InliningStatus inlineStrCharCodeAt(CallInfo &callInfo);
     InliningStatus inlineStrFromCharCode(CallInfo &callInfo);
     InliningStatus inlineStrCharAt(CallInfo &callInfo);
-    InliningStatus inlineStrReplaceRegExp(CallInfo &callInfo);
+    InliningStatus inlineStrReplace(CallInfo &callInfo);
 
     // RegExp natives.
     InliningStatus inlineRegExpExec(CallInfo &callInfo);

js/src/jit/LIR-Common.h

@@ -3246,34 +3246,61 @@ class LRegExpTest : public LCallInstructionHelper<1, 2, 0>
     }
 };
 
-class LRegExpReplace : public LCallInstructionHelper<1, 3, 0>
+
+class LStrReplace : public LCallInstructionHelper<1, 3, 0>
 {
   public:
-    LIR_HEADER(RegExpReplace)
+    LStrReplace(const LAllocation &string, const LAllocation &pattern,
-
-    LRegExpReplace(const LAllocation &string, const LAllocation &regexp,
                    const LAllocation &replacement)
     {
         setOperand(0, string);
-        setOperand(1, regexp);
+        setOperand(1, pattern);
         setOperand(2, replacement);
     }
 
     const LAllocation *string() {
         return getOperand(0);
     }
-    const LAllocation *regexp() {
+    const LAllocation *pattern() {
         return getOperand(1);
     }
     const LAllocation *replacement() {
         return getOperand(2);
     }
+};
+
+class LRegExpReplace: public LStrReplace
+{
+  public:
+    LIR_HEADER(RegExpReplace);
+
+    LRegExpReplace(const LAllocation &string, const LAllocation &pattern,
+                   const LAllocation &replacement)
+      : LStrReplace(string, pattern, replacement)
+    {
+    }
 
     const MRegExpReplace *mir() const {
         return mir_->toRegExpReplace();
     }
 };
 
+class LStringReplace: public LStrReplace
+{
+  public:
+    LIR_HEADER(StringReplace);
+
+    LStringReplace(const LAllocation &string, const LAllocation &pattern,
+                   const LAllocation &replacement)
+      : LStrReplace(string, pattern, replacement)
+    {
+    }
+
+    const MStringReplace *mir() const {
+        return mir_->toStringReplace();
+    }
+};
+
 class LLambdaForSingleton : public LCallInstructionHelper<1, 1, 0>
 {
   public:

js/src/jit/LOpcodes.h

@@ -152,6 +152,7 @@
     _(RegExpExec)                   \
     _(RegExpTest)                   \
     _(RegExpReplace)                \
+    _(StringReplace)                \
     _(Lambda)                       \
     _(LambdaForSingleton)           \
     _(LambdaPar)                    \

js/src/jit/Lowering.cpp

@@ -1939,12 +1939,25 @@ LIRGenerator::visitRegExpTest(MRegExpTest *ins)
 bool
 LIRGenerator::visitRegExpReplace(MRegExpReplace *ins)
 {
-    JS_ASSERT(ins->regexp()->type() == MIRType_Object);
+    JS_ASSERT(ins->pattern()->type() == MIRType_Object);
     JS_ASSERT(ins->string()->type() == MIRType_String);
     JS_ASSERT(ins->replacement()->type() == MIRType_String);
 
     LRegExpReplace *lir = new(alloc()) LRegExpReplace(useRegisterOrConstantAtStart(ins->string()),
-                                                      useRegisterAtStart(ins->regexp()),
+                                                      useRegisterAtStart(ins->pattern()),
+                                                      useRegisterOrConstantAtStart(ins->replacement()));
+    return defineReturn(lir, ins) && assignSafepoint(lir, ins);
+}
+
+bool
+LIRGenerator::visitStringReplace(MStringReplace *ins)
+{
+    JS_ASSERT(ins->pattern()->type() == MIRType_String);
+    JS_ASSERT(ins->string()->type() == MIRType_String);
+    JS_ASSERT(ins->replacement()->type() == MIRType_String);
+
+    LStringReplace *lir = new(alloc()) LStringReplace(useRegisterOrConstantAtStart(ins->string()),
+                                                      useRegisterAtStart(ins->pattern()),
                                                       useRegisterOrConstantAtStart(ins->replacement()));
     return defineReturn(lir, ins) && assignSafepoint(lir, ins);
 }

js/src/jit/Lowering.h

@@ -148,6 +148,7 @@ class LIRGenerator : public LIRGeneratorSpecific
     bool visitRegExpExec(MRegExpExec *ins);
     bool visitRegExpTest(MRegExpTest *ins);
     bool visitRegExpReplace(MRegExpReplace *ins);
+    bool visitStringReplace(MStringReplace *ins);
     bool visitLambda(MLambda *ins);
     bool visitLambdaPar(MLambdaPar *ins);
     bool visitImplicitThis(MImplicitThis *ins);

js/src/jit/MCallOptimize.cpp

@@ -120,7 +120,7 @@ IonBuilder::inlineNativeCall(CallInfo &callInfo, JSNative native)
     if (native == js_str_charAt)
         return inlineStrCharAt(callInfo);
     if (native == str_replace)
-        return inlineStrReplaceRegExp(callInfo);
+        return inlineStrReplace(callInfo);
 
     // RegExp natives.
     if (native == regexp_exec && CallResultEscapes(pc))
@@ -1178,7 +1178,7 @@ IonBuilder::inlineRegExpTest(CallInfo &callInfo)
 }
 
 IonBuilder::InliningStatus
-IonBuilder::inlineStrReplaceRegExp(CallInfo &callInfo)
+IonBuilder::inlineStrReplace(CallInfo &callInfo)
 {
     if (callInfo.argc() != 2 || callInfo.constructing())
         return InliningStatus_NotInlined;
@@ -1194,7 +1194,7 @@ IonBuilder::inlineStrReplaceRegExp(CallInfo &callInfo)
     // Arg 0: RegExp.
     types::TemporaryTypeSet *arg0Type = callInfo.getArg(0)->resultTypeSet();
     const Class *clasp = arg0Type ? arg0Type->getKnownClass() : nullptr;
-    if (clasp != &RegExpObject::class_)
+    if (clasp != &RegExpObject::class_ && callInfo.getArg(0)->type() != MIRType_String)
         return InliningStatus_NotInlined;
 
     // Arg 1: String.
@@ -1203,13 +1203,18 @@ IonBuilder::inlineStrReplaceRegExp(CallInfo &callInfo)
 
     callInfo.setImplicitlyUsedUnchecked();
 
-    MInstruction *cte = MRegExpReplace::New(alloc(), callInfo.thisArg(), callInfo.getArg(0),
+    MInstruction *cte;
-                                            callInfo.getArg(1));
+    if (callInfo.getArg(0)->type() == MIRType_String) {
+        cte = MStringReplace::New(alloc(), callInfo.thisArg(), callInfo.getArg(0),
+                                  callInfo.getArg(1));
+    } else {
+        cte = MRegExpReplace::New(alloc(), callInfo.thisArg(), callInfo.getArg(0),
+                                  callInfo.getArg(1));
+    }
     current->add(cte);
     current->push(cte);
-    if (!resumeAfter(cte))
+    if (cte->isEffectful() && !resumeAfter(cte))
         return InliningStatus_Error;
-
     return InliningStatus_Inlined;
 }
 

js/src/jit/MIR.h

@@ -4885,29 +4885,26 @@ class MRegExpTest
     }
 };
 
-class MRegExpReplace
+template <class Policy1>
+class MStrReplace
   : public MTernaryInstruction,
-    public Mix3Policy<StringPolicy<0>, ObjectPolicy<1>, StringPolicy<2> >
+    public Mix3Policy<StringPolicy<0>, Policy1, StringPolicy<2> >
 {
-  private:
+  protected:
 
-    MRegExpReplace(MDefinition *string, MDefinition *regexp, MDefinition *replacement)
+    MStrReplace(MDefinition *string, MDefinition *pattern, MDefinition *replacement)
-      : MTernaryInstruction(string, regexp, replacement)
+      : MTernaryInstruction(string, pattern, replacement)
     {
+        setMovable();
         setResultType(MIRType_String);
     }
 
   public:
-    INSTRUCTION_HEADER(RegExpReplace)
-
-    static MRegExpReplace *New(TempAllocator &alloc, MDefinition *string, MDefinition *regexp, MDefinition *replacement) {
-        return new(alloc) MRegExpReplace(string, regexp, replacement);
-    }
 
     MDefinition *string() const {
         return getOperand(0);
     }
-    MDefinition *regexp() const {
+    MDefinition *pattern() const {
         return getOperand(1);
     }
     MDefinition *replacement() const {
@@ -4923,6 +4920,46 @@ class MRegExpReplace
     }
 };
 
+class MRegExpReplace
+    : public MStrReplace< ObjectPolicy<1> >
+{
+  private:
+
+    MRegExpReplace(MDefinition *string, MDefinition *pattern, MDefinition *replacement)
+      : MStrReplace< ObjectPolicy<1> >(string, pattern, replacement)
+    {
+    }
+
+  public:
+    INSTRUCTION_HEADER(RegExpReplace);
+
+    static MRegExpReplace *New(TempAllocator &alloc, MDefinition *string, MDefinition *pattern, MDefinition *replacement) {
+        return new(alloc) MRegExpReplace(string, pattern, replacement);
+    }
+};
+
+class MStringReplace
+    : public MStrReplace< StringPolicy<1> >
+{
+  private:
+
+    MStringReplace(MDefinition *string, MDefinition *pattern, MDefinition *replacement)
+      : MStrReplace< StringPolicy<1> >(string, pattern, replacement)
+    {
+    }
+
+  public:
+    INSTRUCTION_HEADER(StringReplace);
+
+    static MStringReplace *New(TempAllocator &alloc, MDefinition *string, MDefinition *pattern, MDefinition *replacement) {
+        return new(alloc) MStringReplace(string, pattern, replacement);
+    }
+
+    AliasSet getAliasSet() const {
+        return AliasSet::None();
+    }
+};
+
 struct LambdaFunctionInfo
 {
     // The functions used in lambdas are the canonical original function in

js/src/jit/MOpcodes.h

@@ -101,6 +101,7 @@ namespace jit {
     _(RegExpExec)                                                           \
     _(RegExpTest)                                                           \
     _(RegExpReplace)                                                        \
+    _(StringReplace)                                                        \
     _(Lambda)                                                               \
     _(ImplicitThis)                                                         \
     _(Slots)                                                                \

js/src/jit/ParallelSafetyAnalysis.cpp

@@ -281,6 +281,7 @@ class ParallelSafetyVisitor : public MInstructionVisitor
     UNSAFE_OP(RegExpTest)
     UNSAFE_OP(RegExpExec)
     UNSAFE_OP(RegExpReplace)
+    UNSAFE_OP(StringReplace)
     UNSAFE_OP(CallInstanceOf)
     UNSAFE_OP(FunctionBoundary)
     UNSAFE_OP(GuardString)

js/src/jit/VMFunctions.cpp

@@ -914,10 +914,10 @@ JSObject *CreateDerivedTypedObj(JSContext *cx, HandleObject type,
 }
 
 JSString *
-regexp_replace(JSContext *cx, HandleString string, HandleObject regexp, HandleString repl)
+RegExpReplace(JSContext *cx, HandleString string, HandleObject regexp, HandleString repl)
 {
-    JS_ASSERT(!!string);
+    JS_ASSERT(string);
-    JS_ASSERT(!!repl);
+    JS_ASSERT(repl);
 
     RootedValue rval(cx);
     if (!str_replace_regexp_raw(cx, string, regexp, repl, &rval))
@@ -926,6 +926,20 @@ regexp_replace(JSContext *cx, HandleString string, HandleObject regexp, HandleSt
     return rval.toString();
 }
 
+JSString *
+StringReplace(JSContext *cx, HandleString string, HandleString pattern, HandleString repl)
+{
+    JS_ASSERT(string);
+    JS_ASSERT(pattern);
+    JS_ASSERT(repl);
+
+    RootedValue rval(cx);
+    if (!str_replace_string_raw(cx, string, pattern, repl, &rval))
+        return nullptr;
+
+    return rval.toString();
+}
+
 bool
 Recompile(JSContext *cx)
 {

js/src/jit/VMFunctions.h

@@ -663,8 +663,10 @@ JSObject *CreateDerivedTypedObj(JSContext *cx, HandleObject type,
                                 HandleObject owner, int32_t offset);
 
 bool Recompile(JSContext *cx);
-JSString *regexp_replace(JSContext *cx, HandleString string, HandleObject regexp,
+JSString *RegExpReplace(JSContext *cx, HandleString string, HandleObject regexp,
-                         HandleString repl);
+                        HandleString repl);
+JSString *StringReplace(JSContext *cx, HandleString string, HandleString pattern,
+                        HandleString repl);
 
 #ifdef DEBUG
 void AssertValidObjectPtr(JSContext *cx, JSObject *obj);

js/src/jsstr.cpp

@@ -1731,6 +1731,13 @@ class MOZ_STACK_CLASS StringRegExpGuard
         return true;
     }
 
+    bool init(JSContext *cx, HandleString pattern) {
+        fm.patstr = AtomizeString(cx, pattern);
+        if (!fm.patstr)
+            return false;
+        return true;
+    }
+
     /*
      * Attempt to match |patstr| to |textstr|. A flags argument, metachars in
      * the pattern string, or a lengthy pattern string can thwart this process.
@@ -2449,7 +2456,7 @@ ReplaceRegExp(JSContext *cx, RegExpStatics *res, ReplaceData &rdata)
 
 static bool
 BuildFlatReplacement(JSContext *cx, HandleString textstr, HandleString repstr,
-                     const FlatMatch &fm, CallArgs *args)
+                     const FlatMatch &fm, MutableHandleValue rval)
 {
     RopeBuilder builder(cx);
     size_t match = fm.match();
@@ -2521,7 +2528,7 @@ BuildFlatReplacement(JSContext *cx, HandleString textstr, HandleString repstr,
         }
     }
 
-    args->rval().setString(builder.result());
+    rval.setString(builder.result());
     return true;
 }
 
@@ -2533,7 +2540,7 @@ BuildFlatReplacement(JSContext *cx, HandleString textstr, HandleString repstr,
  */
 static inline bool
 BuildDollarReplacement(JSContext *cx, JSString *textstrArg, JSLinearString *repstr,
-                       const jschar *firstDollar, const FlatMatch &fm, CallArgs *args)
+                       const jschar *firstDollar, const FlatMatch &fm, MutableHandleValue rval)
 {
     Rooted<JSLinearString*> textstr(cx, textstrArg->ensureLinear(cx));
     if (!textstr)
@@ -2605,7 +2612,7 @@ BuildDollarReplacement(JSContext *cx, JSString *textstrArg, JSLinearString *reps
            builder.append(rightSide));
 #undef ENSURE
 
-    args->rval().setString(builder.result());
+    rval.setString(builder.result());
     return true;
 }
 
@@ -2860,11 +2867,47 @@ js::str_replace_regexp_raw(JSContext *cx, HandleString string, HandleObject rege
     return StrReplaceRegExp(cx, rdata, rval);
 }
 
+static inline bool
+StrReplaceString(JSContext *cx, ReplaceData &rdata, const FlatMatch &fm, MutableHandleValue rval)
+{
+    /*
+     * Note: we could optimize the text.length == pattern.length case if we wanted,
+     * even in the presence of dollar metachars.
+     */
+    if (rdata.dollar)
+        return BuildDollarReplacement(cx, rdata.str, rdata.repstr, rdata.dollar, fm, rval);
+    return BuildFlatReplacement(cx, rdata.str, rdata.repstr, fm, rval);
+}
+
+static const uint32_t ReplaceOptArg = 2;
+
+bool
+js::str_replace_string_raw(JSContext *cx, HandleString string, HandleString pattern,
+                          HandleString replacement, MutableHandleValue rval)
+{
+    ReplaceData rdata(cx);
+
+    rdata.str = string;
+    JSLinearString *repl = replacement->ensureLinear(cx);
+    if (!repl)
+        return false;
+    rdata.setReplacementString(repl);
+
+    if (!rdata.g.init(cx, pattern))
+        return false;
+    const FlatMatch *fm = rdata.g.tryFlatMatch(cx, rdata.str, ReplaceOptArg, ReplaceOptArg, false);
+
+    if (fm->match() < 0) {
+        rval.setString(string);
+        return true;
+    }
+
+    return StrReplaceString(cx, rdata, *fm, rval);
+}
+
 static inline bool
 str_replace_flat_lambda(JSContext *cx, CallArgs outerArgs, ReplaceData &rdata, const FlatMatch &fm)
 {
-    JS_ASSERT(fm.match() >= 0);
-
     RootedString matchStr(cx, js_NewDependentString(cx, rdata.str, fm.match(), fm.patternLength()));
     if (!matchStr)
         return false;
@@ -2911,8 +2954,6 @@ str_replace_flat_lambda(JSContext *cx, CallArgs outerArgs, ReplaceData &rdata, c
     return true;
 }
 
-static const uint32_t ReplaceOptArg = 2;
-
 /*
  * Pattern match the script to check if it is is indexing into a particular
  * object, e.g. 'function(a) { return b[a]; }'. Avoid calling the script in
@@ -3017,6 +3058,7 @@ js::str_replace(JSContext *cx, unsigned argc, Value *vp)
      */
 
     const FlatMatch *fm = rdata.g.tryFlatMatch(cx, rdata.str, ReplaceOptArg, args.length(), false);
+
     if (!fm) {
         if (cx->isExceptionPending())  /* oom in RopeMatch in tryFlatMatch */
             return false;
@@ -3030,15 +3072,7 @@ js::str_replace(JSContext *cx, unsigned argc, Value *vp)
 
     if (rdata.lambda)
         return str_replace_flat_lambda(cx, args, rdata, *fm);
-
+    return StrReplaceString(cx, rdata, *fm, args.rval());
-    /*
-     * Note: we could optimize the text.length == pattern.length case if we wanted,
-     * even in the presence of dollar metachars.
-     */
-    if (rdata.dollar)
-        return BuildDollarReplacement(cx, rdata.str, rdata.repstr, rdata.dollar, *fm, &args);
-
-    return BuildFlatReplacement(cx, rdata.str, rdata.repstr, *fm, &args);
 }
 
 namespace {

js/src/jsstr.h

@@ -370,6 +370,10 @@ bool
 str_replace_regexp_raw(JSContext *cx, HandleString string, HandleObject regexp,
                        HandleString replacement, MutableHandleValue rval);
 
+bool
+str_replace_string_raw(JSContext *cx, HandleString string, HandleString pattern,
+                       HandleString replacement, MutableHandleValue rval);
+
 } /* namespace js */
 
 extern bool