From 1095b0f0e41a0eedbebf026fb542818975f6c534 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Fri, 26 Apr 2013 14:08:54 +0200 Subject: [PATCH] Bug 863018 part 2 - Add JSShortString path back to ConcatStrings and LConcat. r=luke --- js/src/ion/CodeGenerator.cpp | 134 +++++++++++++++--- js/src/ion/Ion.cpp | 28 +++- js/src/ion/IonCompartment.h | 15 ++ js/src/ion/IonMacroAssembler.cpp | 6 + js/src/ion/IonMacroAssembler.h | 1 + js/src/ion/LIR-Common.h | 21 ++- js/src/ion/Lowering.cpp | 9 +- js/src/ion/arm/Assembler-arm.h | 2 +- js/src/ion/arm/MacroAssembler-arm.h | 3 + js/src/ion/x64/Assembler-x64.h | 14 ++ js/src/ion/x64/MacroAssembler-x64.h | 4 + js/src/ion/x86/Assembler-x86.h | 1 + js/src/ion/x86/MacroAssembler-x86.h | 4 + .../jit-test/tests/ion/string-concat-short.js | 13 ++ js/src/vm/String.cpp | 18 +++ js/src/vm/String.h | 4 + 16 files changed, 249 insertions(+), 28 deletions(-) create mode 100644 js/src/jit-test/tests/ion/string-concat-short.js diff --git a/js/src/ion/CodeGenerator.cpp b/js/src/ion/CodeGenerator.cpp index cbd2109b12f..43fa2af4fcc 100644 --- a/js/src/ion/CodeGenerator.cpp +++ b/js/src/ion/CodeGenerator.cpp @@ -3590,52 +3590,152 @@ CodeGenerator::visitConcat(LConcat *lir) Register rhs = ToRegister(lir->rhs()); Register output = ToRegister(lir->output()); - Register temp = ToRegister(lir->temp()); + + JS_ASSERT(lhs == CallTempReg0); + JS_ASSERT(rhs == CallTempReg1); + JS_ASSERT(ToRegister(lir->temp1()) == CallTempReg2); + JS_ASSERT(ToRegister(lir->temp2()) == CallTempReg3); + JS_ASSERT(ToRegister(lir->temp3()) == CallTempReg4); + JS_ASSERT(ToRegister(lir->temp4()) == CallTempReg5); + JS_ASSERT(output == CallTempReg6); OutOfLineCode *ool = oolCallVM(ConcatStringsInfo, lir, (ArgList(), lhs, rhs), StoreRegisterTo(output)); if (!ool) return false; - Label done; + IonCode *stringConcatStub = gen->ionCompartment()->stringConcatStub(); + masm.call(stringConcatStub); + masm.branchTestPtr(Assembler::Zero, output, output, ool->entry()); + + masm.bind(ool->rejoin()); + return true; +} + +static void +CopyStringChars(MacroAssembler &masm, Register to, Register from, Register len, Register scratch) +{ + // Copy |len| jschars from |from| to |to|. Assumes len > 0 (checked below in + // debug builds), and when done |to| must point to the next available char. + +#ifdef DEBUG + Label ok; + masm.branch32(Assembler::GreaterThan, len, Imm32(0), &ok); + masm.breakpoint(); + masm.bind(&ok); +#endif + + JS_STATIC_ASSERT(sizeof(jschar) == 2); + + Label start; + masm.bind(&start); + masm.load16ZeroExtend(Address(from, 0), scratch); + masm.store16(scratch, Address(to, 0)); + masm.addPtr(Imm32(2), from); + masm.addPtr(Imm32(2), to); + masm.sub32(Imm32(1), len); + masm.j(Assembler::NonZero, &start); +} + +IonCode * +IonCompartment::generateStringConcatStub(JSContext *cx) +{ + MacroAssembler masm(cx); + + Register lhs = CallTempReg0; + Register rhs = CallTempReg1; + Register temp1 = CallTempReg2; + Register temp2 = CallTempReg3; + Register temp3 = CallTempReg4; + Register temp4 = CallTempReg5; + Register output = CallTempReg6; + + Label failure; // If lhs is empty, return rhs. Label leftEmpty; - masm.loadStringLength(lhs, temp); - masm.branchTest32(Assembler::Zero, temp, temp, &leftEmpty); + masm.loadStringLength(lhs, temp1); + masm.branchTest32(Assembler::Zero, temp1, temp1, &leftEmpty); // If rhs is empty, return lhs. Label rightEmpty; - masm.loadStringLength(rhs, output); - masm.branchTest32(Assembler::Zero, output, output, &rightEmpty); + masm.loadStringLength(rhs, temp2); + masm.branchTest32(Assembler::Zero, temp2, temp2, &rightEmpty); - // Ensure total length <= JSString::MAX_LENGTH. - masm.add32(output, temp); - masm.branch32(Assembler::Above, temp, Imm32(JSString::MAX_LENGTH), ool->entry()); + masm.add32(temp1, temp2); + + // Check if we can use a JSShortString. + Label isShort; + masm.branch32(Assembler::BelowOrEqual, temp2, Imm32(JSShortString::MAX_SHORT_LENGTH), + &isShort); + + // Ensure result length <= JSString::MAX_LENGTH. + masm.branch32(Assembler::Above, temp1, Imm32(JSString::MAX_LENGTH), &failure); // Allocate a new rope. - masm.newGCString(output, ool->entry()); + masm.newGCString(output, &failure); // Store lengthAndFlags. JS_STATIC_ASSERT(JSString::ROPE_FLAGS == 0); - masm.lshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp); - masm.storePtr(temp, Address(output, JSString::offsetOfLengthAndFlags())); + masm.lshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp2); + masm.storePtr(temp2, 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.ret(); masm.bind(&leftEmpty); masm.mov(rhs, output); - masm.jump(&done); + masm.ret(); masm.bind(&rightEmpty); masm.mov(lhs, output); + masm.ret(); - masm.bind(&done); - masm.bind(ool->rejoin()); - return true; + masm.bind(&isShort); + + // State: lhs length in temp1, result length in temp2. + + // Ensure both strings are linear (flags != 0). + JS_STATIC_ASSERT(JSString::ROPE_FLAGS == 0); + masm.branchTestPtr(Assembler::Zero, Address(lhs, JSString::offsetOfLengthAndFlags()), + Imm32(JSString::FLAGS_MASK), &failure); + masm.branchTestPtr(Assembler::Zero, Address(rhs, JSString::offsetOfLengthAndFlags()), + Imm32(JSString::FLAGS_MASK), &failure); + + // Allocate a JSShortString. + masm.newGCShortString(output, &failure); + + // Set lengthAndFlags. + masm.lshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp2); + masm.orPtr(Imm32(JSString::FIXED_FLAGS), temp2); + masm.storePtr(temp2, Address(output, JSString::offsetOfLengthAndFlags())); + + // Set chars pointer, keep in temp2 for copy loop below. + masm.computeEffectiveAddress(Address(output, JSShortString::offsetOfInlineStorage()), temp2); + masm.storePtr(temp2, Address(output, JSShortString::offsetOfChars())); + + // Copy lhs chars. Temp1 still holds the lhs length. Note that this + // advances temp2 to point to the next char. + masm.loadPtr(Address(lhs, JSString::offsetOfChars()), temp3); + CopyStringChars(masm, temp2, temp3, temp1, temp4); + + // Copy rhs chars. + masm.loadPtr(Address(rhs, JSString::offsetOfChars()), temp3); + masm.loadStringLength(rhs, temp1); + CopyStringChars(masm, temp2, temp3, temp1, temp4); + + // Null-terminate. + masm.store16(Imm32(0), Address(temp2, 0)); + masm.ret(); + + masm.bind(&failure); + masm.movePtr(ImmWord((void *)NULL), output); + masm.ret(); + + Linker linker(masm); + return linker.newCode(cx, JSC::OTHER_CODE); } typedef bool (*CharCodeAtFn)(JSContext *, HandleString, int32_t, uint32_t *); diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index 10c68f8e182..6d79bdeadb2 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -183,9 +183,6 @@ IonRuntime::initialize(JSContext *cx) { AutoEnterAtomsCompartment ac(cx); - if (!cx->compartment->ensureIonCompartmentExists(cx)) - return false; - IonContext ictx(cx, NULL); AutoFlushCache afc("IonRuntime::initialize"); @@ -193,6 +190,9 @@ IonRuntime::initialize(JSContext *cx) if (!execAlloc_) return false; + if (!cx->compartment->ensureIonCompartmentExists(cx)) + return false; + functionWrappers_ = cx->new_(cx); if (!functionWrappers_ || !functionWrappers_->init()) return false; @@ -284,7 +284,8 @@ IonRuntime::freeOsrTempData() IonCompartment::IonCompartment(IonRuntime *rt) : rt(rt), stubCodes_(NULL), - baselineCallReturnAddr_(NULL) + baselineCallReturnAddr_(NULL), + stringConcatStub_(NULL) { } @@ -300,6 +301,19 @@ IonCompartment::initialize(JSContext *cx) stubCodes_ = cx->new_(cx); if (!stubCodes_ || !stubCodes_->init()) return false; + + return true; +} + +bool +IonCompartment::ensureIonStubsExist(JSContext *cx) +{ + if (!stringConcatStub_) { + stringConcatStub_ = generateStringConcatStub(cx); + if (!stringConcatStub_) + return false; + } + return true; } @@ -364,6 +378,9 @@ IonCompartment::sweep(FreeOp *fop) // If the sweep removed the ICCall_Fallback stub, NULL the baselineCallReturnAddr_ field. if (!stubCodes_->lookup(static_cast(ICStub::Call_Fallback))) baselineCallReturnAddr_ = NULL; + + if (stringConcatStub_ && !IsIonCodeMarked(stringConcatStub_.unsafeGet())) + stringConcatStub_ = NULL; } IonCode * @@ -1323,6 +1340,9 @@ IonCompile(JSContext *cx, JSScript *script, if (!cx->compartment->ensureIonCompartmentExists(cx)) return AbortReason_Alloc; + if (!cx->compartment->ionCompartment()->ensureIonStubsExist(cx)) + return AbortReason_Alloc; + MIRGraph *graph = alloc->new_(temp); ExecutionMode executionMode = compileContext.executionMode(); CompileInfo *info = alloc->new_(script, script->function(), osrPc, constructing, diff --git a/js/src/ion/IonCompartment.h b/js/src/ion/IonCompartment.h index 472cf55fba5..16ebaaf122f 100644 --- a/js/src/ion/IonCompartment.h +++ b/js/src/ion/IonCompartment.h @@ -197,6 +197,14 @@ class IonCompartment // Allocated space for optimized baseline stubs. OptimizedICStubSpace optimizedStubSpace_; + // Stub to concatenate two strings inline. Note that it can't be + // stored in IonRuntime because masm.newGCString bakes in zone-specific + // pointers. This has to be a weak pointer to avoid keeping the whole + // compartment alive. + ReadBarriered stringConcatStub_; + + IonCode *generateStringConcatStub(JSContext *cx); + public: IonCode *getVMWrapper(const VMFunction &f); @@ -235,6 +243,9 @@ class IonCompartment bool initialize(JSContext *cx); + // Initialize code stubs only used by Ion, not Baseline. + bool ensureIonStubsExist(JSContext *cx); + void mark(JSTracer *trc, JSCompartment *compartment); void sweep(FreeOp *fop); @@ -284,6 +295,10 @@ class IonCompartment return rt->debugTrapHandler(cx); } + IonCode *stringConcatStub() { + return stringConcatStub_; + } + AutoFlushCache *flusher() { return rt->flusher(); } diff --git a/js/src/ion/IonMacroAssembler.cpp b/js/src/ion/IonMacroAssembler.cpp index 4742e816df8..5055193dda5 100644 --- a/js/src/ion/IonMacroAssembler.cpp +++ b/js/src/ion/IonMacroAssembler.cpp @@ -463,6 +463,12 @@ MacroAssembler::newGCString(const Register &result, Label *fail) newGCThing(result, js::gc::FINALIZE_STRING, fail); } +void +MacroAssembler::newGCShortString(const Register &result, Label *fail) +{ + newGCThing(result, js::gc::FINALIZE_SHORT_STRING, fail); +} + void MacroAssembler::parNewGCThing(const Register &result, const Register &threadContextReg, diff --git a/js/src/ion/IonMacroAssembler.h b/js/src/ion/IonMacroAssembler.h index 405760e6b36..cb0c833cc5d 100644 --- a/js/src/ion/IonMacroAssembler.h +++ b/js/src/ion/IonMacroAssembler.h @@ -580,6 +580,7 @@ class MacroAssembler : public MacroAssemblerSpecific 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 newGCShortString(const Register &result, Label *fail); void parNewGCThing(const Register &result, const Register &threadContextReg, diff --git a/js/src/ion/LIR-Common.h b/js/src/ion/LIR-Common.h index 09cd0f54c65..aed8c8bd26e 100644 --- a/js/src/ion/LIR-Common.h +++ b/js/src/ion/LIR-Common.h @@ -2248,15 +2248,19 @@ class LBinaryV : public LCallInstructionHelper }; // Adds two string, returning a string. -class LConcat : public LInstructionHelper<1, 2, 1> +class LConcat : public LInstructionHelper<1, 2, 4> { public: LIR_HEADER(Concat) - LConcat(const LAllocation &lhs, const LAllocation &rhs, const LDefinition &temp) { + LConcat(const LAllocation &lhs, const LAllocation &rhs, const LDefinition &temp1, + const LDefinition &temp2, const LDefinition &temp3, const LDefinition &temp4) { setOperand(0, lhs); setOperand(1, rhs); - setTemp(0, temp); + setTemp(0, temp1); + setTemp(1, temp2); + setTemp(2, temp3); + setTemp(3, temp4); } const LAllocation *lhs() { @@ -2265,9 +2269,18 @@ class LConcat : public LInstructionHelper<1, 2, 1> const LAllocation *rhs() { return this->getOperand(1); } - const LDefinition *temp() { + const LDefinition *temp1() { return this->getTemp(0); } + const LDefinition *temp2() { + return this->getTemp(1); + } + const LDefinition *temp3() { + return this->getTemp(2); + } + const LDefinition *temp4() { + return this->getTemp(3); + } }; // Get uint16 character code from a string. diff --git a/js/src/ion/Lowering.cpp b/js/src/ion/Lowering.cpp index 12c60374b56..6f0360f0c97 100644 --- a/js/src/ion/Lowering.cpp +++ b/js/src/ion/Lowering.cpp @@ -1293,8 +1293,13 @@ LIRGenerator::visitConcat(MConcat *ins) JS_ASSERT(rhs->type() == MIRType_String); JS_ASSERT(ins->type() == MIRType_String); - LConcat *lir = new LConcat(useRegister(lhs), useRegister(rhs), temp()); - if (!define(lir, ins)) + LConcat *lir = new LConcat(useFixed(lhs, CallTempReg0), + useFixed(rhs, CallTempReg1), + tempFixed(CallTempReg2), + tempFixed(CallTempReg3), + tempFixed(CallTempReg4), + tempFixed(CallTempReg5)); + if (!defineFixed(lir, ins, LAllocation(AnyRegister(CallTempReg6)))) return false; return assignSafepoint(lir, ins); } diff --git a/js/src/ion/arm/Assembler-arm.h b/js/src/ion/arm/Assembler-arm.h index 80f1b773725..68acd6338f1 100644 --- a/js/src/ion/arm/Assembler-arm.h +++ b/js/src/ion/arm/Assembler-arm.h @@ -55,7 +55,7 @@ static const Register CallTempReg2 = r7; static const Register CallTempReg3 = r8; static const Register CallTempReg4 = r0; static const Register CallTempReg5 = r1; - +static const Register CallTempReg6 = r2; static const Register IntArgReg0 = r0; static const Register IntArgReg1 = r1; diff --git a/js/src/ion/arm/MacroAssembler-arm.h b/js/src/ion/arm/MacroAssembler-arm.h index cbd46d0c1c8..54e8ea5d552 100644 --- a/js/src/ion/arm/MacroAssembler-arm.h +++ b/js/src/ion/arm/MacroAssembler-arm.h @@ -853,6 +853,9 @@ class MacroAssemblerARMCompat : public MacroAssemblerARM void branchTestPtr(Condition cond, const Register &lhs, const Imm32 rhs, Label *label) { branchTest32(cond, lhs, rhs, label); } + void branchTestPtr(Condition cond, const Address &lhs, Imm32 imm, Label *label) { + branchTest32(cond, lhs, imm, label); + } void branchPtr(Condition cond, Register lhs, Register rhs, Label *label) { branch32(cond, lhs, rhs, label); } diff --git a/js/src/ion/x64/Assembler-x64.h b/js/src/ion/x64/Assembler-x64.h index 37837bcd80a..2621dab8319 100644 --- a/js/src/ion/x64/Assembler-x64.h +++ b/js/src/ion/x64/Assembler-x64.h @@ -82,6 +82,7 @@ static const Register CallTempReg2 = rbx; static const Register CallTempReg3 = rcx; static const Register CallTempReg4 = rsi; static const Register CallTempReg5 = rdx; +static const Register CallTempReg6 = rbp; // Different argument registers for WIN64 #if defined(_WIN64) @@ -626,6 +627,19 @@ class Assembler : public AssemblerX86Shared void testq(const Register &lhs, const Register &rhs) { masm.testq_rr(rhs.code(), lhs.code()); } + void testq(const Operand &lhs, Imm32 rhs) { + switch (lhs.kind()) { + case Operand::REG: + masm.testq_i32r(rhs.value, lhs.reg()); + break; + case Operand::REG_DISP: + masm.testq_i32m(rhs.value, lhs.disp(), lhs.base()); + break; + default: + JS_NOT_REACHED("unexpected operand kind"); + break; + } + } void jmp(void *target, Relocation::Kind reloc = Relocation::HARDCODED) { JmpSrc src = masm.jmp(); diff --git a/js/src/ion/x64/MacroAssembler-x64.h b/js/src/ion/x64/MacroAssembler-x64.h index 8b21e4500f2..4d5a5372826 100644 --- a/js/src/ion/x64/MacroAssembler-x64.h +++ b/js/src/ion/x64/MacroAssembler-x64.h @@ -510,6 +510,10 @@ class MacroAssemblerX64 : public MacroAssemblerX86Shared testq(lhs, imm); j(cond, label); } + void branchTestPtr(Condition cond, const Address &lhs, Imm32 imm, Label *label) { + testq(Operand(lhs), imm); + j(cond, label); + } void decBranchPtr(Condition cond, const Register &lhs, Imm32 imm, Label *label) { subPtr(imm, lhs); j(cond, label); diff --git a/js/src/ion/x86/Assembler-x86.h b/js/src/ion/x86/Assembler-x86.h index 3099f80a9b6..9f4046c2ccc 100644 --- a/js/src/ion/x86/Assembler-x86.h +++ b/js/src/ion/x86/Assembler-x86.h @@ -54,6 +54,7 @@ static const Register CallTempReg2 = ebx; static const Register CallTempReg3 = ecx; static const Register CallTempReg4 = esi; static const Register CallTempReg5 = edx; +static const Register CallTempReg6 = ebp; // We have no arg regs, so our NonArgRegs are just our CallTempReg* static const Register CallTempNonArgRegs[] = { edi, eax, ebx, ecx, esi, edx }; diff --git a/js/src/ion/x86/MacroAssembler-x86.h b/js/src/ion/x86/MacroAssembler-x86.h index 8c2a88d39d6..698c1f83ba1 100644 --- a/js/src/ion/x86/MacroAssembler-x86.h +++ b/js/src/ion/x86/MacroAssembler-x86.h @@ -544,6 +544,10 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared testl(lhs, imm); j(cond, label); } + void branchTestPtr(Condition cond, const Address &lhs, Imm32 imm, Label *label) { + testl(Operand(lhs), imm); + j(cond, label); + } void decBranchPtr(Condition cond, const Register &lhs, Imm32 imm, Label *label) { subPtr(imm, lhs); j(cond, label); diff --git a/js/src/jit-test/tests/ion/string-concat-short.js b/js/src/jit-test/tests/ion/string-concat-short.js new file mode 100644 index 00000000000..a0108df0445 --- /dev/null +++ b/js/src/jit-test/tests/ion/string-concat-short.js @@ -0,0 +1,13 @@ +function f() { + var res = 0; + for (var i=0; i<100; i++) { + var s = "test" + i; + res += s.length; + assertEq(s[0], "t"); + assertEq(s[3], "t"); + if (i > 90) + assertEq(s[4], "9"); + } + return res; +} +assertEq(f(), 590); diff --git a/js/src/vm/String.cpp b/js/src/vm/String.cpp index dc62d96247e..1bb9b86c641 100644 --- a/js/src/vm/String.cpp +++ b/js/src/vm/String.cpp @@ -319,6 +319,24 @@ js::ConcatStrings(JSContext *cx, if (!JSString::validateLength(cxIfCanGC, wholeLength)) return NULL; + if (JSShortString::lengthFits(wholeLength)) { + JSShortString *str = js_NewGCShortString(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_(cx, left, right, wholeLength); } diff --git a/js/src/vm/String.h b/js/src/vm/String.h index a0c8514509c..e02cbc88ba3 100644 --- a/js/src/vm/String.h +++ b/js/src/vm/String.h @@ -677,6 +677,10 @@ class JSInlineString : public JSFlatString static bool lengthFits(size_t length) { return length <= MAX_INLINE_LENGTH; } + + static size_t offsetOfInlineStorage() { + return offsetof(JSInlineString, d.inlineStorage); + } }; JS_STATIC_ASSERT(sizeof(JSInlineString) == sizeof(JSString));