Bug 784127 - Stop inlining monomorphic property accesses if a shape guard fails. r=sstangl
☠☠ backed out by e164663c4b6f ☠ ☠
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 29 Aug 2012 10:56:14 +0200
changeset 105032 d11dafc10fc07049b51f0187be5790553c8104a8
parent 105031 07d466a718a660c156f195a05c15c7cfe5abd99d
child 105033 e164663c4b6f471dbc5adb3d908928262b679b47
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewerssstangl
bugs784127
milestone17.0a1
Bug 784127 - Stop inlining monomorphic property accesses if a shape guard fails. r=sstangl
js/src/ion/Bailouts.cpp
js/src/ion/IonBuilder.cpp
js/src/ion/IonBuilder.h
js/src/ion/IonTypes.h
js/src/ion/MIR.h
js/src/ion/arm/Lowering-arm.cpp
js/src/ion/shared/Lowering-x86-shared.cpp
js/src/jsscript.h
--- a/js/src/ion/Bailouts.cpp
+++ b/js/src/ion/Bailouts.cpp
@@ -303,16 +303,19 @@ ConvertFrames(JSContext *cx, IonActivati
       case Bailout_Monitor:
         return BAILOUT_RETURN_MONITOR;
       case Bailout_RecompileCheck:
         return BAILOUT_RETURN_RECOMPILE_CHECK;
       case Bailout_BoundsCheck:
         return BAILOUT_RETURN_BOUNDS_CHECK;
       case Bailout_Invalidate:
         return BAILOUT_RETURN_INVALIDATE;
+      case Bailout_CachedShapeGuard:
+        it.script()->failedCachedShapeGuard = true;
+        return BAILOUT_RETURN_INVALIDATE;
 
       // When bailing out from an argument check, none of the code of the
       // function has run yet. When profiling, this means that the function
       // hasn't flagged its entry just yet. It has been "entered," however, so
       // we flag it here manually that the entry has happened.
       case Bailout_ArgumentCheck:
         fp->unsetPushedSPSFrame();
         Probes::enterScript(cx, fp->script(), fp->script()->function(), fp);
--- a/js/src/ion/IonBuilder.cpp
+++ b/js/src/ion/IonBuilder.cpp
@@ -31,16 +31,17 @@ IonBuilder::IonBuilder(JSContext *cx, Te
     lir(NULL),
     cx(cx),
     loopDepth_(loopDepth),
     callerResumePoint_(NULL),
     callerBuilder_(NULL),
     oracle(oracle),
     inliningDepth(inliningDepth),
     failedBoundsCheck_(script->failedBoundsCheck),
+    failedCachedShapeGuard_(script->failedCachedShapeGuard),
     lazyArguments_(NULL)
 {
     pc = info->startPC();
 }
 
 void
 IonBuilder::clearForBackEnd()
 {
@@ -390,16 +391,19 @@ IonBuilder::buildInline(IonBuilder *call
             script->filename, script->lineno, (void *)script);
 
     callerBuilder_ = callerBuilder;
     callerResumePoint_ = callerResumePoint;
 
     if (callerBuilder->failedBoundsCheck_)
         failedBoundsCheck_ = true;
 
+    if (callerBuilder->failedCachedShapeGuard_)
+        failedCachedShapeGuard_ = true;
+
     // Generate single entrance block.
     current = newBlock(pc);
     if (!current)
         return false;
 
     current->setCallerResumePoint(callerResumePoint);
 
     // Connect the entrance block to the last block in the caller's graph.
@@ -4598,17 +4602,17 @@ IonBuilder::jsop_getgname(HandleProperty
     }
 
     MInstruction *global = MConstant::New(ObjectValue(*globalObj));
     current->add(global);
 
     // If we have a property typeset, the isOwnProperty call will trigger recompilation if
     // the property is deleted or reconfigured.
     if (!propertyTypes && shape->configurable()) {
-        MGuardShape *guard = MGuardShape::New(global, globalObj->lastProperty());
+        MGuardShape *guard = MGuardShape::New(global, globalObj->lastProperty(), Bailout_Invalidate);
         current->add(guard);
     }
 
     JS_ASSERT(shape->slot() >= globalObj->numFixedSlots());
 
     MSlots *slots = MSlots::New(global);
     current->add(slots);
     MLoadSlot *load = MLoadSlot::New(slots, shape->slot() - globalObj->numFixedSlots());
@@ -4651,17 +4655,17 @@ IonBuilder::jsop_setgname(HandleProperty
 
     MInstruction *global = MConstant::New(ObjectValue(*globalObj));
     current->add(global);
 
     // If we have a property type set, the isOwnProperty call will trigger recompilation
     // if the property is deleted or reconfigured. Without TI, we always need a shape guard
     // to guard against the property being reconfigured as non-writable.
     if (!propertyTypes) {
-        MGuardShape *guard = MGuardShape::New(global, globalObj->lastProperty());
+        MGuardShape *guard = MGuardShape::New(global, globalObj->lastProperty(), Bailout_Invalidate);
         current->add(guard);
     }
 
     JS_ASSERT(shape->slot() >= globalObj->numFixedSlots());
 
     MSlots *slots = MSlots::New(global);
     current->add(slots);
 
@@ -5405,17 +5409,17 @@ IonBuilder::TestCommonPropFunc(JSContext
     types->addFreeze(cx);
 
     // Add a shape guard on the prototype we found the property on. The rest of
     // the prototype chain is guarded by TI freezes. Note that a shape guard is
     // good enough here, even in the proxy case, because we have ensured there
     // are no lookup hooks for this property.
     MInstruction *wrapper = MConstant::New(ObjectValue(*foundProto));
     current->add(wrapper);
-    MGuardShape *guard = MGuardShape::New(wrapper, foundProto->lastProperty());
+    MGuardShape *guard = MGuardShape::New(wrapper, foundProto->lastProperty(), Bailout_Invalidate);
     current->add(guard);
 
     // Now we have to freeze all the property typesets to ensure there isn't a
     // lower shadowing getter or setter installed in the future.
     types::TypeObject *curType;
     for (unsigned i = 0; i < types->getObjectCount(); i++) {
         curType = types->getTypeObject(i);
         JSObject *obj = NULL;
@@ -5830,25 +5834,26 @@ IonBuilder::jsop_getprop(HandlePropertyN
     }
 
     if (unary.ival == MIRType_Object) {
         MIRType rvalType = MIRType_Value;
         if (!barrier && !IsNullOrUndefined(unary.rval))
             rvalType = unary.rval;
 
         Shape *objShape;
-        if ((objShape = mjit::GetPICSingleShape(cx, script, pc, info().constructing())) &&
+        if (!failedCachedShapeGuard_ &&
+            (objShape = mjit::GetPICSingleShape(cx, script, pc, info().constructing())) &&
             !objShape->inDictionary())
         {
             // The JM IC was monomorphic, so we inline the property access as
             // long as the shape is not in dictionary mode. We cannot be sure
             // that the shape is still a lastProperty, and calling
             // Shape::search() on dictionary mode shapes that aren't
             // lastProperty is invalid.
-            MGuardShape *guard = MGuardShape::New(obj, objShape);
+            MGuardShape *guard = MGuardShape::New(obj, objShape, Bailout_CachedShapeGuard);
             current->add(guard);
 
             spew("Inlining monomorphic GETPROP");
 
             Shape *shape = objShape->search(cx, NameToId(name));
             JS_ASSERT(shape);
 
             return loadSlot(obj, shape, rvalType);
@@ -5946,24 +5951,25 @@ IonBuilder::jsop_setprop(HandlePropertyN
 
     oracle->binaryOp(script, pc);
 
     MSetPropertyInstruction *ins;
     if (monitored) {
         ins = MCallSetProperty::New(obj, value, name, script->strictModeCode);
     } else {
         Shape *objShape;
-        if ((objShape = mjit::GetPICSingleShape(cx, script, pc, info().constructing())) &&
+        if (!failedCachedShapeGuard_ &&
+            (objShape = mjit::GetPICSingleShape(cx, script, pc, info().constructing())) &&
             !objShape->inDictionary())
         {
             // The JM IC was monomorphic, so we inline the property access as
             // long as the shape is not in dictionary mode. We cannot be sure
             // that the shape is still a lastProperty, and calling Shape::search
             // on dictionary mode shapes that aren't lastProperty is invalid.
-            MGuardShape *guard = MGuardShape::New(obj, objShape);
+            MGuardShape *guard = MGuardShape::New(obj, objShape, Bailout_CachedShapeGuard);
             current->add(guard);
 
             Shape *shape = objShape->search(cx, NameToId(name));
             JS_ASSERT(shape);
 
             spew("Inlining monomorphic SETPROP");
 
             jsid typeId = types::MakeTypeId(cx, id);
--- a/js/src/ion/IonBuilder.h
+++ b/js/src/ion/IonBuilder.h
@@ -451,16 +451,20 @@ class IonBuilder : public MIRGenerator
     Vector<MInstruction *, 2, IonAllocPolicy> iterators_;
     TypeOracle *oracle;
     size_t inliningDepth;
 
     // True if script->failedBoundsCheck is set for the current script or
     // an outer script.
     bool failedBoundsCheck_;
 
+    // True if script->failedCachedShapeGuard is set for the current script or
+    // an outer script.
+    bool failedCachedShapeGuard_;
+
     // If this script can use a lazy arguments object, it wil be pre-created
     // here.
     MInstruction *lazyArguments_;
 };
 
 } // namespace ion
 } // namespace js
 
--- a/js/src/ion/IonTypes.h
+++ b/js/src/ion/IonTypes.h
@@ -48,17 +48,20 @@ enum BailoutKind
 
     // A bailout to trigger recompilation to inline calls when the script is hot.
     Bailout_RecompileCheck,
 
     // A bailout triggered by a bounds-check failure.
     Bailout_BoundsCheck,
 
     // Like Bailout_Normal, but invalidate the current IonScript.
-    Bailout_Invalidate
+    Bailout_Invalidate,
+
+    // A shape guard based on JM ICs failed.
+    Bailout_CachedShapeGuard
 };
 
 #ifdef DEBUG
 // Track the pipeline of opcodes which has produced a snapshot.
 #define TRACK_SNAPSHOTS 1
 #endif
 
 } // namespace ion
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -4351,46 +4351,53 @@ class MBindNameCache
 };
 
 // Guard on an object's shape.
 class MGuardShape
   : public MUnaryInstruction,
     public SingleObjectPolicy
 {
     const Shape *shape_;
-
-    MGuardShape(MDefinition *obj, const Shape *shape)
+    BailoutKind bailoutKind_;
+
+    MGuardShape(MDefinition *obj, const Shape *shape, BailoutKind bailoutKind)
       : MUnaryInstruction(obj),
-        shape_(shape)
+        shape_(shape),
+        bailoutKind_(bailoutKind)
     {
         setGuard();
         setMovable();
     }
 
   public:
     INSTRUCTION_HEADER(GuardShape);
 
-    static MGuardShape *New(MDefinition *obj, const Shape *shape) {
-        return new MGuardShape(obj, shape);
+    static MGuardShape *New(MDefinition *obj, const Shape *shape, BailoutKind bailoutKind) {
+        return new MGuardShape(obj, shape, bailoutKind);
     }
 
     TypePolicy *typePolicy() {
         return this;
     }
     MDefinition *obj() const {
         return getOperand(0);
     }
     const Shape *shape() const {
         return shape_;
     }
+    BailoutKind bailoutKind() const {
+        return bailoutKind_;
+    }
     bool congruentTo(MDefinition * const &ins) const {
         if (!ins->isGuardShape())
             return false;
         if (shape() != ins->toGuardShape()->shape())
             return false;
+        if (bailoutKind() != ins->toGuardShape()->bailoutKind())
+            return false;
         return congruentIfOperandsEqual(ins);
     }
     AliasSet getAliasSet() const {
         return AliasSet::Load(AliasSet::ObjectFields);
     }
 };
 
 // Guard on an object's class.
--- a/js/src/ion/arm/Lowering-arm.cpp
+++ b/js/src/ion/arm/Lowering-arm.cpp
@@ -317,17 +317,17 @@ LIRGeneratorARM::visitTableSwitch(MTable
     return add(new LTableSwitch(index, tempInt, tableswitch));
 }
 
 bool
 LIRGeneratorARM::visitGuardShape(MGuardShape *ins)
 {
     LDefinition tempObj = temp(LDefinition::OBJECT);
     LGuardShape *guard = new LGuardShape(useRegister(ins->obj()), tempObj);
-    return assignSnapshot(guard, Bailout_Invalidate) && add(guard, ins);
+    return assignSnapshot(guard, ins->bailoutKind()) && add(guard, ins);
 }
 
 bool
 LIRGeneratorARM::visitRecompileCheck(MRecompileCheck *ins)
 {
     LRecompileCheck *lir = new LRecompileCheck(temp(LDefinition::GENERAL));
     return assignSnapshot(lir, Bailout_RecompileCheck) && add(lir, ins);
 }
--- a/js/src/ion/shared/Lowering-x86-shared.cpp
+++ b/js/src/ion/shared/Lowering-x86-shared.cpp
@@ -59,17 +59,17 @@ LIRGeneratorX86Shared::visitInterruptChe
         return false;
     return true;
 }
 
 bool
 LIRGeneratorX86Shared::visitGuardShape(MGuardShape *ins)
 {
     LGuardShape *guard = new LGuardShape(useRegister(ins->obj()));
-    return assignSnapshot(guard, Bailout_Invalidate) && add(guard, ins);
+    return assignSnapshot(guard, ins->bailoutKind()) && add(guard, ins);
 }
 
 bool
 LIRGeneratorX86Shared::visitPowHalf(MPowHalf *ins)
 {
     MDefinition *input = ins->input();
     JS_ASSERT(input->type() == MIRType_Double);
     LPowHalfD *lir = new LPowHalfD(useRegisterAtStart(input), temp());
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -471,16 +471,17 @@ struct JSScript : public js::gc::Cell
     bool            isActiveEval:1;   /* script came from eval(), and is still active */
     bool            isCachedEval:1;   /* script came from eval(), and is in eval cache */
     bool            uninlineable:1;   /* script is considered uninlineable by analysis */
 #ifdef JS_METHODJIT
     bool            debugMode:1;      /* script was compiled in debug mode */
     bool            failedBoundsCheck:1; /* script has had hoisted bounds checks fail */
 #endif
     bool            invalidatedIdempotentCache:1; /* idempotent cache has triggered invalidation */
+    bool            failedCachedShapeGuard:1; /* Ion shape guard based on JM IC triggered invalidation */
     bool            isGenerator:1;    /* is a generator */
     bool            isGeneratorExp:1; /* is a generator expression */
     bool            hasScriptCounts:1;/* script has an entry in
                                          JSCompartment::scriptCountsMap */
     bool            hasDebugScript:1; /* script has an entry in
                                          JSCompartment::debugScriptMap */
   private:
     /* See comments below. */