From 84f5850fcd8076e24e96ae210537f611e069e201 Mon Sep 17 00:00:00 2001 From: Kannan Vijayan Date: Tue, 23 Oct 2012 22:18:11 -0400 Subject: [PATCH] Bug 795801 - IC StrictPropertyOp setters in IonMonkey. (r=dvander) --- js/src/ion/IonCaches.cpp | 281 +++++++++++++++++++++++++++++++++++---- js/src/ion/IonCaches.h | 4 +- 2 files changed, 257 insertions(+), 28 deletions(-) diff --git a/js/src/ion/IonCaches.cpp b/js/src/ion/IonCaches.cpp index 3b4feebb6fba..763ac328bd36 100644 --- a/js/src/ion/IonCaches.cpp +++ b/js/src/ion/IonCaches.cpp @@ -401,10 +401,7 @@ struct GetNativePropertyStub // WARNING: if the IonCode object ever moved, since we'd be rooting a nonsense // WARNING: value here. // WARNING: - stubCodePatchOffset = masm.pushWithPatch(ImmWord(uintptr_t(-1))); - // Manually adjust framePushed to account for this push which is not otherwise - // accounted for. - masm.setFramePushed(masm.framePushed() + sizeof(uintptr_t)); + stubCodePatchOffset = masm.PushWithPatch(ImmWord(uintptr_t(-1))); if (callNative) { JS_ASSERT(shape->hasGetterValue() && shape->getterValue().isObject() && @@ -629,10 +626,11 @@ IonCacheGetProperty::attachCallGetter(JSContext *cx, IonScript *ion, JSObject *o } static bool -TryAttachNativeStub(JSContext *cx, IonScript *ion, - IonCacheGetProperty &cache, HandleObject obj, - HandlePropertyName name, const SafepointIndex *safepointIndex, - void *returnAddr, bool *isCacheable) +TryAttachNativeGetPropStub(JSContext *cx, IonScript *ion, + IonCacheGetProperty &cache, HandleObject obj, + HandlePropertyName name, + const SafepointIndex *safepointIndex, + void *returnAddr, bool *isCacheable) { JS_ASSERT(!*isCacheable); @@ -732,8 +730,12 @@ js::ion::GetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Mu // limit. Once we can make calls from within generated stubs, a new call // stub will be generated instead and the previous stubs unlinked. bool isCacheable = false; - if (!TryAttachNativeStub(cx, ion, cache, obj, name, safepointIndex, returnAddr, &isCacheable)) + if (!TryAttachNativeGetPropStub(cx, ion, cache, obj, name, + safepointIndex, returnAddr, + &isCacheable)) + { return false; + } if (cache.idempotent() && !isCacheable) { // Invalidate the cache if the property was not found, or was found on @@ -800,7 +802,8 @@ IonCache::reset() } bool -IonCacheSetProperty::attachNativeExisting(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *shape) +IonCacheSetProperty::attachNativeExisting(JSContext *cx, IonScript *ion, + HandleObject obj, HandleShape shape) { MacroAssembler masm; @@ -859,6 +862,194 @@ IonCacheSetProperty::attachNativeExisting(JSContext *cx, IonScript *ion, JSObjec return true; } +bool +IonCacheSetProperty::attachSetterCall(JSContext *cx, IonScript *ion, + HandleObject obj, HandleObject holder, HandleShape shape, + void *returnAddr) +{ + MacroAssembler masm; + + // Need to set correct framePushed on the masm so that exit frame descriptors are + // properly constructed. + masm.setFramePushed(ion->frameSize()); + + Label failure; + masm.branchPtr(Assembler::NotEqual, + Address(object(), JSObject::offsetOfShape()), + ImmGCPtr(obj->lastProperty()), + &failure); + + // Generate prototype guards if needed. + // Take a scratch register for use, save on stack. + { + RegisterSet regSet(RegisterSet::All()); + regSet.take(AnyRegister(object())); + if (!value().constant()) + regSet.maybeTake(value().reg()); + Register scratchReg = regSet.takeGeneral(); + masm.push(scratchReg); + + Label protoFailure; + Label protoSuccess; + + // Generate prototype/shape guards. + if (obj != holder) + GeneratePrototypeGuards(cx, masm, obj, holder, object(), scratchReg, &protoFailure); + + masm.movePtr(ImmGCPtr(holder), scratchReg); + masm.branchPtr(Assembler::NotEqual, + Address(scratchReg, JSObject::offsetOfShape()), + ImmGCPtr(holder->lastProperty()), + &protoFailure); + + masm.jump(&protoSuccess); + + masm.bind(&protoFailure); + masm.pop(scratchReg); + masm.jump(&failure); + + masm.bind(&protoSuccess); + masm.pop(scratchReg); + } + + // Good to go for invoking setter. + + // saveLive() + masm.PushRegsInMask(liveRegs); + + // Remaining registers should basically be free, but we need to use |object| still + // so leave it alone. + RegisterSet regSet(RegisterSet::All()); + regSet.take(AnyRegister(object())); + + // This is a slower stub path, and we're going to be doing a call anyway. Don't need + // to try so hard to not use the stack. Scratch regs are just taken from the register + // set not including the input, current value saved on the stack, and restored when + // we're done with it. + Register scratchReg = regSet.takeGeneral(); + Register argJSContextReg = regSet.takeGeneral(); + Register argObjReg = regSet.takeGeneral(); + Register argIdReg = regSet.takeGeneral(); + Register argStrictReg = regSet.takeGeneral(); + Register argVpReg = regSet.takeGeneral(); + + // Ensure stack is aligned. + DebugOnly initialStack = masm.framePushed(); + masm.checkStackAlignment(); + + Label success, exception; + + // Push the IonCode pointer for the stub we're generating. + // WARNING: + // WARNING: If IonCode ever becomes relocatable, the following code is incorrect. + // WARNING: Note that we're not marking the pointer being pushed as an ImmGCPtr. + // WARNING: This is not a marking issue since the stub IonCode won't be collected + // WARNING: between the time it's called and when we get here, but it would fail + // WARNING: if the IonCode object ever moved, since we'd be rooting a nonsense + // WARNING: value here. + // WARNING: + CodeOffsetLabel stubCodePatchOffset = masm.PushWithPatch(ImmWord(uintptr_t(-1))); + + StrictPropertyOp target = shape->setterOp(); + JS_ASSERT(target); + // JSStrictPropertyOp: JSBool fn(JSContext *cx, JSHandleObject obj, + // JSHandleId id, JSBool strict, JSMutableHandleValue vp); + + // Push args on stack first so we can take pointers to make handles. + if (value().constant()) + masm.Push(value().value()); + else + masm.Push(value().reg()); + masm.movePtr(StackPointer, argVpReg); + + masm.move32(Imm32(strict() ? 1 : 0), argStrictReg); + + // push canonical jsid from shape instead of propertyname. + jsid propId; + if (!shape->getUserId(cx, &propId)) + return false; + masm.Push(propId, argIdReg); + masm.movePtr(StackPointer, argIdReg); + + masm.Push(object()); + masm.movePtr(StackPointer, argObjReg); + + masm.loadJSContext(argJSContextReg); + + if (!masm.buildOOLFakeExitFrame(returnAddr)) + return false; + masm.enterFakeExitFrame(ION_FRAME_OOL_PROPERTY_OP); + + // Make the call. + masm.setupUnalignedABICall(5, scratchReg); + masm.passABIArg(argJSContextReg); + masm.passABIArg(argObjReg); + masm.passABIArg(argIdReg); + masm.passABIArg(argStrictReg); + masm.passABIArg(argVpReg); + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, target)); + + // Test for failure. + masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, &exception); + + masm.jump(&success); + + // Handle exception case. + masm.bind(&exception); + masm.handleException(); + + // Handle success case. + masm.bind(&success); + + // The next instruction is removing the footer of the exit frame, so there + // is no need for leaveFakeExitFrame. + + // Move the StackPointer back to its original location, unwinding the exit frame. + masm.adjustStack(IonOOLPropertyOpExitFrameLayout::Size()); + JS_ASSERT(masm.framePushed() == initialStack); + + // restoreLive() + masm.PopRegsInMask(liveRegs); + + // Rejoin jump. + RepatchLabel rejoin; + CodeOffsetJump rejoinOffset = masm.jumpWithPatch(&rejoin); + masm.bind(&rejoin); + + // Exit jump. + masm.bind(&failure); + RepatchLabel exit; + CodeOffsetJump exitOffset = masm.jumpWithPatch(&exit); + masm.bind(&exit); + + Linker linker(masm); + IonCode *code = linker.newCode(cx); + if (!code) + return false; + + rejoinOffset.fixup(&masm); + exitOffset.fixup(&masm); + stubCodePatchOffset.fixup(&masm); + + if (ion->invalidated()) + return true; + + Assembler::patchDataWithValueCheck(CodeLocationLabel(code, stubCodePatchOffset), + ImmWord(uintptr_t(code)), ImmWord(uintptr_t(-1))); + + CodeLocationJump rejoinJump(code, rejoinOffset); + CodeLocationJump exitJump(code, exitOffset); + CodeLocationJump lastJump_ = lastJump(); + PatchJump(lastJump_, CodeLocationLabel(code)); + PatchJump(rejoinJump, rejoinLabel()); + PatchJump(exitJump, cacheLabel()); + updateLastJump(exitJump); + + IonSpew(IonSpew_InlineCaches, "Generated SETPROP calling case stub at %p", code->raw()); + + return true; +} + bool IonCacheSetProperty::attachNativeAdding(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *oldShape, const Shape *newShape, @@ -968,13 +1159,9 @@ IsPropertyInlineable(JSObject *obj, IonCacheSetProperty &cache) } static bool -IsPropertySetInlineable(JSContext *cx, HandleObject obj, JSAtom *atom, jsid *pId, const Shape **pShape) +IsPropertySetInlineable(JSContext *cx, HandleObject obj, HandleId id, MutableHandleShape pshape) { - jsid id = AtomToId(atom); - *pId = id; - - const Shape *shape = obj->nativeLookup(cx, id); - *pShape = shape; + Shape *shape = obj->nativeLookup(cx, id); if (!shape) return false; @@ -988,18 +1175,41 @@ IsPropertySetInlineable(JSContext *cx, HandleObject obj, JSAtom *atom, jsid *pId if (!shape->writable()) return false; + pshape.set(shape); + + return true; +} + +static bool +IsPropertySetterCallInlineable(JSContext *cx, HandleObject obj, HandleObject holder, + jsid id, HandleShape shape) +{ + if (!shape) + return false; + + if (shape->hasSlot()) + return false; + + if (shape->hasDefaultSetter()) + return false; + + // We only handle propertyOps for now, so fail if we have SetterValue + // (which implies JSNative setter). + if (shape->hasSetterValue()) + return false; + return true; } static bool IsPropertyAddInlineable(JSContext *cx, HandleObject obj, jsid id, uint32_t oldSlots, - const Shape **pShape) + MutableHandleShape pShape) { // This is not a Add, the property exists. - if (*pShape) + if (pShape.get()) return false; - const Shape *shape = obj->nativeLookup(cx, id); + Shape *shape = obj->nativeLookup(cx, id); if (!shape || shape->inDictionary() || !shape->hasSlot() || !shape->hasDefaultSetter()) return false; @@ -1032,7 +1242,7 @@ IsPropertyAddInlineable(JSContext *cx, HandleObject obj, jsid id, uint32_t oldSl if (obj->numDynamicSlots() != oldSlots) return false; - *pShape = shape; + pShape.set(shape); return true; } @@ -1042,17 +1252,34 @@ js::ion::SetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Ha { AutoFlushCache afc ("SetPropertyCache"); - IonScript *ion = GetTopIonJSScript(cx)->ion; + void *returnAddr; + const SafepointIndex *safepointIndex; + JSScript *script = GetTopIonJSScript(cx, &safepointIndex, &returnAddr); + IonScript *ion = script->ion; IonCacheSetProperty &cache = ion->getCache(cacheIndex).toSetProperty(); RootedPropertyName name(cx, cache.name()); - jsid id; - const Shape *shape = NULL; + RootedId id(cx, AtomToId(name)); + RootedShape shape(cx); + RootedObject holder(cx); bool inlinable = IsPropertyInlineable(obj, cache); - if (inlinable && IsPropertySetInlineable(cx, obj, name, &id, &shape)) { - cache.incrementStubCount(); - if (!cache.attachNativeExisting(cx, ion, obj, shape)) - return false; + if (inlinable) { + RootedShape shape(cx); + if (IsPropertySetInlineable(cx, obj, id, &shape)) { + cache.incrementStubCount(); + if (!cache.attachNativeExisting(cx, ion, obj, shape)) + return false; + } else { + RootedObject holder(cx); + if (!JSObject::lookupProperty(cx, obj, name, &holder, &shape)) + return false; + + if (IsPropertySetterCallInlineable(cx, obj, holder, id, shape)) { + cache.incrementStubCount(); + if (!cache.attachSetterCall(cx, ion, obj, holder, shape, returnAddr)) + return false; + } + } } uint32_t oldSlots = obj->numDynamicSlots(); diff --git a/js/src/ion/IonCaches.h b/js/src/ion/IonCaches.h index 411e6c8c2e89..76bd29f175c8 100644 --- a/js/src/ion/IonCaches.h +++ b/js/src/ion/IonCaches.h @@ -302,7 +302,9 @@ class IonCacheSetProperty : public IonCache ConstantOrRegister value() const { return u.setprop.value.data(); } bool strict() const { return u.setprop.strict; } - bool attachNativeExisting(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *shape); + bool attachNativeExisting(JSContext *cx, IonScript *ion, HandleObject obj, HandleShape shape); + bool attachSetterCall(JSContext *cx, IonScript *ion, HandleObject obj, + HandleObject holder, HandleShape shape, void *returnAddr); bool attachNativeAdding(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *oldshape, const Shape *newshape, const Shape *propshape); };