Bug 806793: disable hoisting shape guards after bailing because of a shape guard, r=jandem

js/src/ion/Bailouts.cpp

@@ -313,8 +313,8 @@ ConvertFrames(JSContext *cx, IonActivation *activation, IonBailoutIterator &it)
         return BAILOUT_RETURN_RECOMPILE_CHECK;
       case Bailout_BoundsCheck:
         return BAILOUT_RETURN_BOUNDS_CHECK;
-      case Bailout_Invalidate:
-        return BAILOUT_RETURN_INVALIDATE;
+      case Bailout_ShapeGuard:
+        return BAILOUT_RETURN_SHAPE_GUARD;
       case Bailout_CachedShapeGuard:
         return BAILOUT_RETURN_CACHED_SHAPE_GUARD;
 
@@ -552,7 +552,7 @@ ion::BoundsCheckFailure()
 }
 
 uint32
-ion::ForceInvalidation()
+ion::ShapeGuardFailure()
 {
     JSContext *cx = GetIonContext()->cx;
     JSScript *script = GetBailedJSScript(cx);
@@ -560,7 +560,9 @@ ion::ForceInvalidation()
     JS_ASSERT(script->hasIonScript());
     JS_ASSERT(!script->ion->invalidated());
 
-    IonSpew(IonSpew_Invalidate, "Forced invalidation bailout");
+    script->failedShapeGuard = true;
+
+    IonSpew(IonSpew_Invalidate, "Invalidating due to shape guard failure");
 
     return Invalidate(cx, script);
 }
@@ -574,6 +576,8 @@ ion::CachedShapeGuardFailure()
     JS_ASSERT(script->hasIonScript());
     JS_ASSERT(!script->ion->invalidated());
 
+    script->failedShapeGuard = true;
+
     // Purge JM caches in the script and all inlined script, to avoid baking in
     // the same shape guard next time.
     for (size_t i = 0; i < script->ion->scriptEntries(); i++)

js/src/ion/Bailouts.h

@@ -106,7 +106,7 @@ static const uint32 BAILOUT_RETURN_TYPE_BARRIER = 3;
 static const uint32 BAILOUT_RETURN_MONITOR = 4;
 static const uint32 BAILOUT_RETURN_RECOMPILE_CHECK = 5;
 static const uint32 BAILOUT_RETURN_BOUNDS_CHECK = 6;
-static const uint32 BAILOUT_RETURN_INVALIDATE = 7;
+static const uint32 BAILOUT_RETURN_SHAPE_GUARD = 7;
 static const uint32 BAILOUT_RETURN_OVERRECURSED = 8;
 static const uint32 BAILOUT_RETURN_CACHED_SHAPE_GUARD = 9;
 
@@ -222,7 +222,7 @@ uint32 RecompileForInlining();
 
 uint32 BoundsCheckFailure();
 
-uint32 ForceInvalidation();
+uint32 ShapeGuardFailure();
 
 uint32 CachedShapeGuardFailure();
 

js/src/ion/IonBuilder.cpp

@@ -35,6 +35,7 @@ IonBuilder::IonBuilder(JSContext *cx, TempAllocator *temp, MIRGraph *graph,
     oracle(oracle),
     inliningDepth(inliningDepth),
     failedBoundsCheck_(info->script()->failedBoundsCheck),
+    failedShapeGuard_(info->script()->failedShapeGuard),
     lazyArguments_(NULL)
 {
     script_.init(info->script());
@@ -406,6 +407,9 @@ IonBuilder::buildInline(IonBuilder *callerBuilder, MResumePoint *callerResumePoi
     if (callerBuilder->failedBoundsCheck_)
         failedBoundsCheck_ = true;
 
+    if (callerBuilder->failedShapeGuard_)
+        failedShapeGuard_ = true;
+
     // Generate single entrance block.
     current = newBlock(pc);
     if (!current)
@@ -4617,10 +4621,8 @@ IonBuilder::jsop_getgname(HandlePropertyName name)
 
     // 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(), Bailout_Invalidate);
-        current->add(guard);
-    }
+    if (!propertyTypes && shape->configurable())
+        global = addShapeGuard(global, globalObj->lastProperty(), Bailout_ShapeGuard);
 
     JS_ASSERT(shape->slot() >= globalObj->numFixedSlots());
 
@@ -4670,10 +4672,8 @@ IonBuilder::jsop_setgname(HandlePropertyName name)
     // 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(), Bailout_Invalidate);
-        current->add(guard);
-    }
+    if (!propertyTypes)
+        global = addShapeGuard(global, globalObj->lastProperty(), Bailout_ShapeGuard);
 
     JS_ASSERT(shape->slot() >= globalObj->numFixedSlots());
 
@@ -5436,8 +5436,7 @@ IonBuilder::TestCommonPropFunc(JSContext *cx, types::StackTypeSet *types, Handle
     // are no lookup hooks for this property.
     MInstruction *wrapper = MConstant::New(ObjectValue(*foundProto));
     current->add(wrapper);
-    MGuardShape *guard = MGuardShape::New(wrapper, foundProto->lastProperty(), Bailout_Invalidate);
-    current->add(guard);
+    wrapper = addShapeGuard(wrapper, foundProto->lastProperty(), Bailout_ShapeGuard);
 
     // Now we have to freeze all the property typesets to ensure there isn't a
     // lower shadowing getter or setter installed in the future.
@@ -5961,8 +5960,7 @@ IonBuilder::getPropTryMonomorphic(bool *emitted, HandleId id, types::StackTypeSe
     // the shape is not in dictionary made. 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, Bailout_CachedShapeGuard);
-    current->add(guard);
+    obj = addShapeGuard(obj, objShape, Bailout_CachedShapeGuard);
 
     spew("Inlining monomorphic GETPROP");
     Shape *shape = objShape->search(cx, id);
@@ -6112,8 +6110,7 @@ IonBuilder::jsop_setprop(HandlePropertyName name)
             // 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, Bailout_CachedShapeGuard);
-            current->add(guard);
+            obj = addShapeGuard(obj, objShape, Bailout_CachedShapeGuard);
 
             Shape *shape = objShape->search(cx, NameToId(name));
             JS_ASSERT(shape);
@@ -6441,3 +6438,16 @@ IonBuilder::addBoundsCheck(MDefinition *index, MDefinition *length)
 
     return check;
 }
+
+MInstruction *
+IonBuilder::addShapeGuard(MDefinition *obj, const Shape *shape, BailoutKind bailoutKind)
+{
+    MGuardShape *guard = MGuardShape::New(obj, shape, bailoutKind);
+    current->add(guard);
+
+    // If a shape guard failed in the past, don't optimize shape guard.
+    if (failedShapeGuard_)
+        guard->setNotMovable();
+
+    return guard;
+}

js/src/ion/IonBuilder.h

@@ -280,6 +280,7 @@ class IonBuilder : public MIRGenerator
     MDefinition *walkScopeChain(unsigned hops);
 
     MInstruction *addBoundsCheck(MDefinition *index, MDefinition *length);
+    MInstruction *addShapeGuard(MDefinition *obj, const Shape *shape, BailoutKind bailoutKind);
 
     JSObject *getNewArrayTemplateObject(uint32 count);
 
@@ -473,6 +474,10 @@ class IonBuilder : public MIRGenerator
     // an outer script.
     bool failedBoundsCheck_;
 
+    // True if script->failedShapeGuard is set for the current script or
+    // an outer script.
+    bool failedShapeGuard_;
+
     // If this script can use a lazy arguments object, it wil be pre-created
     // here.
     MInstruction *lazyArguments_;

js/src/ion/IonMacroAssembler.cpp

@@ -524,7 +524,7 @@ MacroAssembler::generateBailoutTail(Register scratch)
 
     branch32(Equal, ReturnReg, Imm32(BAILOUT_RETURN_BOUNDS_CHECK), &boundscheck);
     branch32(Equal, ReturnReg, Imm32(BAILOUT_RETURN_OVERRECURSED), &overrecursed);
-    branch32(Equal, ReturnReg, Imm32(BAILOUT_RETURN_INVALIDATE), &invalidate);
+    branch32(Equal, ReturnReg, Imm32(BAILOUT_RETURN_SHAPE_GUARD), &invalidate);
 
     // Fall-through: cached shape guard failure.
     {
@@ -539,7 +539,7 @@ MacroAssembler::generateBailoutTail(Register scratch)
     bind(&invalidate);
     {
         setupUnalignedABICall(0, scratch);
-        callWithABI(JS_FUNC_TO_DATA_PTR(void *, ForceInvalidation));
+        callWithABI(JS_FUNC_TO_DATA_PTR(void *, ShapeGuardFailure));
 
         branchTest32(Zero, ReturnReg, ReturnReg, &exception);
         jump(&interpret);

js/src/ion/IonTypes.h

@@ -52,8 +52,8 @@ enum BailoutKind
     // A bailout triggered by a bounds-check failure.
     Bailout_BoundsCheck,
 
-    // Like Bailout_Normal, but invalidate the current IonScript.
-    Bailout_Invalidate,
+    // A shape guard based on TI information failed.
+    Bailout_ShapeGuard,
 
     // A shape guard based on JM ICs failed.
     Bailout_CachedShapeGuard

js/src/ion/MIR.h

@@ -4484,6 +4484,7 @@ class MGuardShape
     {
         setGuard();
         setMovable();
+        setResultType(MIRType_Object);
     }
 
   public:

js/src/ion/arm/Lowering-arm.cpp

@@ -297,9 +297,15 @@ LIRGeneratorARM::newLTableSwitchV(MTableSwitch *tableswitch)
 bool
 LIRGeneratorARM::visitGuardShape(MGuardShape *ins)
 {
+    JS_ASSERT(ins->obj()->type() == MIRType_Object);
+
     LDefinition tempObj = temp(LDefinition::OBJECT);
     LGuardShape *guard = new LGuardShape(useRegister(ins->obj()), tempObj);
-    return assignSnapshot(guard, ins->bailoutKind()) && add(guard, ins);
+    if (!assignSnapshot(guard, ins->bailoutKind()))
+        return false;
+    if (!add(guard, ins))
+        return false;
+    return redefine(ins, ins->obj());
 }
 
 bool

js/src/ion/shared/Lowering-x86-shared.cpp

@@ -46,8 +46,14 @@ LIRGeneratorX86Shared::visitInterruptCheck(MInterruptCheck *ins)
 bool
 LIRGeneratorX86Shared::visitGuardShape(MGuardShape *ins)
 {
+    JS_ASSERT(ins->obj()->type() == MIRType_Object);
+
     LGuardShape *guard = new LGuardShape(useRegister(ins->obj()));
-    return assignSnapshot(guard, ins->bailoutKind()) && add(guard, ins);
+    if (!assignSnapshot(guard, ins->bailoutKind()))
+        return false;
+    if (!add(guard, ins))
+        return false;
+    return redefine(ins, ins->obj());
 }
 
 bool

js/src/jsscript.h

@@ -472,6 +472,9 @@ struct JSScript : public js::gc::Cell
 #ifdef JS_METHODJIT
     bool            debugMode:1;      /* script was compiled in debug mode */
     bool            failedBoundsCheck:1; /* script has had hoisted bounds checks fail */
+#endif
+#ifdef JS_ION
+    bool            failedShapeGuard:1; /* script has had hoisted shape guard fail */
 #endif
     bool            invalidatedIdempotentCache:1; /* idempotent cache has triggered invalidation */
     bool            isGenerator:1;    /* is a generator */