Bug 861251 - Inline string concatenation in IonMonkey. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 17 Apr 2013 12:23:17 +0200
changeset 129041 2c8d7231bd14e6a8f4e80df624bfc440f16c4375
parent 129040 b619d1769047994e46a33dba31ed92b78fcf946a
child 129042 929fcbfd5618dd5f5ef3355f9e86190d75344da1
push id26637
push userjandemooij@gmail.com
push dateWed, 17 Apr 2013 10:25:23 +0000
treeherdermozilla-inbound@2c8d7231bd14 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs861251
milestone23.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 861251 - Inline string concatenation in IonMonkey. r=luke
js/src/ion/CodeGenerator.cpp
js/src/ion/IonMacroAssembler.cpp
js/src/ion/IonMacroAssembler.h
js/src/ion/LIR-Common.h
js/src/ion/Lowering.cpp
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/ion/CodeGenerator.cpp
+++ b/js/src/ion/CodeGenerator.cpp
@@ -3465,20 +3465,65 @@ CodeGenerator::visitEmulatesUndefinedAnd
 
     testObjectTruthy(objreg, unequal, equal, ToRegister(lir->temp()), ool);
     return true;
 }
 
 bool
 CodeGenerator::visitConcat(LConcat *lir)
 {
-    pushArg(ToRegister(lir->rhs()));
-    pushArg(ToRegister(lir->lhs()));
-    if (!callVM(ConcatStringsInfo, lir))
+    Register lhs = ToRegister(lir->lhs());
+    Register rhs = ToRegister(lir->rhs());
+
+    Register output = ToRegister(lir->output());
+    Register temp = ToRegister(lir->temp());
+
+    OutOfLineCode *ool = oolCallVM(ConcatStringsInfo, lir, (ArgList(), lhs, rhs),
+                                   StoreRegisterTo(output));
+    if (!ool)
         return false;
+
+    Label done;
+
+    // If lhs is empty, return rhs.
+    Label leftEmpty;
+    masm.loadStringLength(lhs, temp);
+    masm.branchTest32(Assembler::Zero, temp, temp, &leftEmpty);
+
+    // If rhs is empty, return lhs.
+    Label rightEmpty;
+    masm.loadStringLength(rhs, output);
+    masm.branchTest32(Assembler::Zero, output, output, &rightEmpty);
+
+    // Ensure total length <= JSString::MAX_LENGTH.
+    masm.add32(output, temp);
+    masm.branch32(Assembler::Above, temp, Imm32(JSString::MAX_LENGTH), ool->entry());
+
+    // Allocate a new rope.
+    masm.newGCString(output, ool->entry());
+
+    // Store lengthAndFlags.
+    JS_STATIC_ASSERT(JSString::ROPE_FLAGS == 0);
+    masm.lshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp);
+    masm.storePtr(temp, Address(output, JSString::offsetOfLengthAndFlags()));
+
+    // Store left and right nodes.
+    masm.storePtr(lhs, Address(output, JSRope::offsetOfLeft()));
+    masm.storePtr(rhs, Address(output, JSRope::offsetOfRight()));
+    masm.jump(&done);
+
+    masm.bind(&leftEmpty);
+    masm.mov(rhs, output);
+    masm.jump(&done);
+
+    masm.bind(&rightEmpty);
+    masm.mov(lhs, output);
+
+    masm.bind(&done);
+    masm.bind(ool->rejoin());
     return true;
 }
 
 typedef bool (*CharCodeAtFn)(JSContext *, HandleString, int32_t, uint32_t *);
 static const VMFunction CharCodeAtInfo = FunctionInfo<CharCodeAtFn>(ion::CharCodeAt);
 
 bool
 CodeGenerator::visitCharCodeAt(LCharCodeAt *lir)
--- a/js/src/ion/IonMacroAssembler.cpp
+++ b/js/src/ion/IonMacroAssembler.cpp
@@ -413,26 +413,21 @@ MacroAssembler::clampDoubleToUint8(Float
         move32(Imm32(255), output);
     }
 
     bind(&done);
 #endif
 }
 
 void
-MacroAssembler::newGCThing(const Register &result,
-                           JSObject *templateObject, Label *fail)
+MacroAssembler::newGCThing(const Register &result, gc::AllocKind allocKind, Label *fail)
 {
     // Inlined equivalent of js::gc::NewGCThing() without failure case handling.
 
-    gc::AllocKind allocKind = templateObject->tenuredGetAllocKind();
-    JS_ASSERT(allocKind >= gc::FINALIZE_OBJECT0 && allocKind <= gc::FINALIZE_OBJECT_LAST);
-    int thingSize = (int)gc::Arena::thingSize(allocKind);
-
-    JS_ASSERT(!templateObject->hasDynamicElements());
+    int thingSize = int(gc::Arena::thingSize(allocKind));
 
     Zone *zone = GetIonContext()->compartment->zone();
 
 #ifdef JS_GC_ZEAL
     // Don't execute the inline path if gcZeal is active.
     movePtr(ImmWord(zone->rt), result);
     loadPtr(Address(result, offsetof(JSRuntime, gcZeal_)), result);
     branch32(Assembler::NotEqual, result, Imm32(0), fail);
@@ -448,16 +443,32 @@ MacroAssembler::newGCThing(const Registe
     branchPtr(Assembler::BelowOrEqual, AbsoluteAddress(&list->last), result, fail);
 
     addPtr(Imm32(thingSize), result);
     storePtr(result, AbsoluteAddress(&list->first));
     subPtr(Imm32(thingSize), result);
 }
 
 void
+MacroAssembler::newGCThing(const Register &result, JSObject *templateObject, Label *fail)
+{
+    gc::AllocKind allocKind = templateObject->tenuredGetAllocKind();
+    JS_ASSERT(allocKind >= gc::FINALIZE_OBJECT0 && allocKind <= gc::FINALIZE_OBJECT_LAST);
+    JS_ASSERT(!templateObject->hasDynamicElements());
+
+    newGCThing(result, allocKind, fail);
+}
+
+void
+MacroAssembler::newGCString(const Register &result, Label *fail)
+{
+    newGCThing(result, js::gc::FINALIZE_STRING, fail);
+}
+
+void
 MacroAssembler::parNewGCThing(const Register &result,
                               const Register &threadContextReg,
                               const Register &tempReg1,
                               const Register &tempReg2,
                               JSObject *templateObject,
                               Label *fail)
 {
     // Similar to ::newGCThing(), except that it allocates from a
--- a/js/src/ion/IonMacroAssembler.h
+++ b/js/src/ion/IonMacroAssembler.h
@@ -548,17 +548,20 @@ class MacroAssembler : public MacroAssem
 
         bind(&isDouble);
         unboxDouble(source, dest);
 
         bind(&done);
     }
 
     // Inline allocation.
+    void newGCThing(const Register &result, gc::AllocKind allocKind, Label *fail);
     void newGCThing(const Register &result, JSObject *templateObject, Label *fail);
+    void newGCString(const Register &result, Label *fail);
+
     void parNewGCThing(const Register &result,
                        const Register &threadContextReg,
                        const Register &tempReg1,
                        const Register &tempReg2,
                        JSObject *templateObject,
                        Label *fail);
     void initGCThing(const Register &obj, JSObject *templateObject);
 
--- a/js/src/ion/LIR-Common.h
+++ b/js/src/ion/LIR-Common.h
@@ -2153,32 +2153,36 @@ class LBinaryV : public LCallInstruction
         return jsop_;
     }
 
     static const size_t LhsInput = 0;
     static const size_t RhsInput = BOX_PIECES;
 };
 
 // Adds two string, returning a string.
-class LConcat : public LCallInstructionHelper<1, 2, 0>
+class LConcat : public LInstructionHelper<1, 2, 1>
 {
   public:
     LIR_HEADER(Concat)
 
-    LConcat(const LAllocation &lhs, const LAllocation &rhs) {
+    LConcat(const LAllocation &lhs, const LAllocation &rhs, const LDefinition &temp) {
         setOperand(0, lhs);
         setOperand(1, rhs);
+        setTemp(0, temp);
     }
 
     const LAllocation *lhs() {
         return this->getOperand(0);
     }
     const LAllocation *rhs() {
         return this->getOperand(1);
     }
+    const LDefinition *temp() {
+        return this->getTemp(0);
+    }
 };
 
 // Get uint16 character code from a string.
 class LCharCodeAt : public LInstructionHelper<1, 2, 0>
 {
   public:
     LIR_HEADER(CharCodeAt)
 
--- a/js/src/ion/Lowering.cpp
+++ b/js/src/ion/Lowering.cpp
@@ -1260,19 +1260,20 @@ LIRGenerator::lowerBinaryV(JSOp op, MBin
 bool
 LIRGenerator::visitConcat(MConcat *ins)
 {
     MDefinition *lhs = ins->getOperand(0);
     MDefinition *rhs = ins->getOperand(1);
 
     JS_ASSERT(lhs->type() == MIRType_String);
     JS_ASSERT(rhs->type() == MIRType_String);
-
-    LConcat *lir = new LConcat(useRegisterAtStart(lhs), useRegisterAtStart(rhs));
-    if (!defineReturn(lir, ins))
+    JS_ASSERT(ins->type() == MIRType_String);
+
+    LConcat *lir = new LConcat(useRegister(lhs), useRegister(rhs), temp());
+    if (!define(lir, ins))
         return false;
     return assignSafepoint(lir, ins);
 }
 
 bool
 LIRGenerator::visitCharCodeAt(MCharCodeAt *ins)
 {
     MDefinition *str = ins->getOperand(0);
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -314,34 +314,16 @@ js::ConcatStrings(JSContext *cx,
     if (rightLen == 0)
         return left;
 
     size_t wholeLength = leftLen + rightLen;
     JSContext *cxIfCanGC = allowGC ? cx : NULL;
     if (!JSString::validateLength(cxIfCanGC, wholeLength))
         return NULL;
 
-    if (JSShortString::lengthFits(wholeLength)) {
-        JSShortString *str = js_NewGCShortString<allowGC>(cx);
-        if (!str)
-            return NULL;
-        const jschar *leftChars = left->getChars(cx);
-        if (!leftChars)
-            return NULL;
-        const jschar *rightChars = right->getChars(cx);
-        if (!rightChars)
-            return NULL;
-
-        jschar *buf = str->init(wholeLength);
-        PodCopy(buf, leftChars, leftLen);
-        PodCopy(buf + leftLen, rightChars, rightLen);
-        buf[wholeLength] = 0;
-        return str;
-    }
-
     return JSRope::new_<allowGC>(cx, left, right, wholeLength);
 }
 
 template JSString *
 js::ConcatStrings<CanGC>(JSContext *cx, HandleString left, HandleString right);
 
 template JSString *
 js::ConcatStrings<NoGC>(JSContext *cx, JSString *left, JSString *right);
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -460,16 +460,23 @@ class JSRope : public JSString
     }
 
     inline JSString *rightChild() const {
         JS_ASSERT(isRope());
         return d.s.u2.right;
     }
 
     inline void markChildren(JSTracer *trc);
+
+    inline static size_t offsetOfLeft() {
+        return offsetof(JSRope, d.u1.left);
+    }
+    inline static size_t offsetOfRight() {
+        return offsetof(JSRope, d.s.u2.right);
+    }
 };
 
 JS_STATIC_ASSERT(sizeof(JSRope) == sizeof(JSString));
 
 class JSLinearString : public JSString
 {
     friend class JSString;