Bug 1128535 - Inline getters/setters in Ion even if type information is bad. r=efaust

js/src/jit/BaselineInspector.cpp

@@ -533,81 +533,127 @@ static Shape *GlobalShapeForGetPropFunction(ICStub *stub)
     return nullptr;
 }
 
-JSObject *
-BaselineInspector::commonGetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonGetter,
-                                         Shape **globalShape)
+static bool
+AddReceiverShape(BaselineInspector::ShapeVector &shapes, Shape *shape)
+{
+    MOZ_ASSERT(shape);
+
+    for (size_t i = 0; i < shapes.length(); i++) {
+        if (shapes[i] == shape)
+            return true;
+    }
+
+    return shapes.append(shape);
+}
+
+static bool
+AddReceiverShapeForGetPropFunction(BaselineInspector::ShapeVector &shapes, ICStub *stub)
+{
+    if (stub->isGetProp_CallNative())
+        return true;
+
+    Shape *shape = nullptr;
+    if (stub->isGetProp_CallScripted())
+        shape = stub->toGetProp_CallScripted()->receiverShape();
+    else
+        shape = stub->toGetProp_CallNativePrototype()->receiverShape();
+
+    return AddReceiverShape(shapes, shape);
+}
+
+bool
+BaselineInspector::commonGetPropFunction(jsbytecode *pc, JSObject **holder, Shape **holderShape,
+                                         JSFunction **commonGetter, Shape **globalShape,
+                                         bool *isOwnProperty, ShapeVector &receiverShapes)
 {
     if (!hasBaselineScript())
-        return nullptr;
+        return false;
 
+    MOZ_ASSERT(receiverShapes.empty());
+
+    *holder = nullptr;
     const ICEntry &entry = icEntryFromPC(pc);
-    JSObject* holder = nullptr;
-    Shape *holderShape = nullptr;
-    JSFunction *getter = nullptr;
-    Shape *global = nullptr;
+
     for (ICStub *stub = entry.firstStub(); stub; stub = stub->next()) {
         if (stub->isGetProp_CallScripted()  ||
             stub->isGetProp_CallNative()    ||
             stub->isGetProp_CallNativePrototype())
         {
             ICGetPropCallGetter *nstub = static_cast<ICGetPropCallGetter *>(stub);
-            if (!holder) {
-                holder = nstub->holder();
-                holderShape = nstub->holderShape();
-                getter = nstub->getter();
-                global = GlobalShapeForGetPropFunction(nstub);
-            } else if (nstub->holderShape() != holderShape ||
-                       GlobalShapeForGetPropFunction(nstub) != global)
+            bool isOwn = stub->isGetProp_CallNative();
+            if (!AddReceiverShapeForGetPropFunction(receiverShapes, nstub))
+                return false;
+
+            if (!*holder) {
+                *holder = nstub->holder();
+                *holderShape = nstub->holderShape();
+                *commonGetter = nstub->getter();
+                *globalShape = GlobalShapeForGetPropFunction(nstub);
+                *isOwnProperty = isOwn;
+            } else if (nstub->holderShape() != *holderShape ||
+                       GlobalShapeForGetPropFunction(nstub) != *globalShape ||
+                       isOwn != *isOwnProperty)
             {
-                return nullptr;
+                return false;
             } else {
-                MOZ_ASSERT(getter == nstub->getter());
+                MOZ_ASSERT(*commonGetter == nstub->getter());
             }
-        } else if (stub->isGetProp_Fallback() &&
+        } else if (!stub->isGetProp_Fallback() ||
                    stub->toGetProp_Fallback()->hadUnoptimizableAccess())
         {
             // We have an unoptimizable access, so don't try to optimize.
-            return nullptr;
+            return false;
         }
     }
-    *lastProperty = holderShape;
-    *commonGetter = getter;
-    *globalShape = global;
-    return holder;
+
+    if (!*holder)
+        return false;
+
+    MOZ_ASSERT(*isOwnProperty == receiverShapes.empty());
+    return true;
 }
 
-JSObject *
-BaselineInspector::commonSetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonSetter)
+bool
+BaselineInspector::commonSetPropFunction(jsbytecode *pc, JSObject **holder, Shape **holderShape,
+                                         JSFunction **commonSetter, bool *isOwnProperty,
+                                         ShapeVector &receiverShapes)
 {
     if (!hasBaselineScript())
-        return nullptr;
+        return false;
 
+    MOZ_ASSERT(receiverShapes.empty());
+
+    *holder = nullptr;
     const ICEntry &entry = icEntryFromPC(pc);
-    JSObject *holder = nullptr;
-    Shape *holderShape = nullptr;
-    JSFunction *setter = nullptr;
+
     for (ICStub *stub = entry.firstStub(); stub; stub = stub->next()) {
         if (stub->isSetProp_CallScripted() || stub->isSetProp_CallNative()) {
             ICSetPropCallSetter *nstub = static_cast<ICSetPropCallSetter *>(stub);
-            if (!holder) {
-                holder = nstub->holder();
-                holderShape = nstub->holderShape();
-                setter = nstub->setter();
-            } else if (nstub->holderShape() != holderShape) {
-                return nullptr;
+            if (!AddReceiverShape(receiverShapes, nstub->shape()))
+                return false;
+
+            if (!*holder) {
+                *holder = nstub->holder();
+                *holderShape = nstub->holderShape();
+                *commonSetter = nstub->setter();
+                *isOwnProperty = false;
+            } else if (nstub->holderShape() != *holderShape) {
+                return false;
             } else {
-                MOZ_ASSERT(setter == nstub->setter());
+                MOZ_ASSERT(*commonSetter == nstub->setter());
             }
-        } else if (stub->isSetProp_Fallback() &&
+        } else if (!stub->isSetProp_Fallback() ||
                    stub->toSetProp_Fallback()->hadUnoptimizableAccess())
         {
             // We have an unoptimizable access, so don't try to optimize.
-            return nullptr;
+            return false;
         }
     }
-    *lastProperty = holderShape;
-    *commonSetter = setter;
-    return holder;
+
+    if (!*holder)
+        return false;
+
+    return true;
 }
 
 bool

js/src/jit/BaselineInspector.h

@@ -119,9 +119,12 @@ class BaselineInspector
     DeclEnvObject *templateDeclEnvObject();
     CallObject *templateCallObject();
 
-    JSObject *commonGetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonGetter,
-                                    Shape **globalShape);
-    JSObject *commonSetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonSetter);
+    bool commonGetPropFunction(jsbytecode *pc, JSObject **holder, Shape **holderShape,
+                               JSFunction **commonGetter, Shape **globalShape, bool *isOwnProperty,
+                               ShapeVector &receiverShapes);
+    bool commonSetPropFunction(jsbytecode *pc, JSObject **holder, Shape **holderShape,
+                               JSFunction **commonSetter, bool *isOwnProperty,
+                               ShapeVector &receiverShapes);
 
     bool instanceOfData(jsbytecode *pc, Shape **shape, uint32_t *slot, JSObject **prototypeObject);
 };

js/src/jit/IonBuilder.cpp

@@ -10166,6 +10166,25 @@ IonBuilder::getPropTryUnboxed(bool *emitted, MDefinition *obj, PropertyName *nam
     return true;
 }
 
+MDefinition *
+IonBuilder::addShapeGuardsForGetterSetter(MDefinition *obj, JSObject *holder, Shape *holderShape,
+                                          const BaselineInspector::ShapeVector &receiverShapes,
+                                          bool isOwnProperty)
+{
+    MOZ_ASSERT(holder);
+    MOZ_ASSERT(holderShape);
+
+    if (isOwnProperty) {
+        MOZ_ASSERT(receiverShapes.empty());
+        return addShapeGuard(obj, holderShape, Bailout_ShapeGuard);
+    }
+
+    MDefinition *holderDef = constantMaybeNursery(holder);
+    addShapeGuard(holderDef, holderShape, Bailout_ShapeGuard);
+
+    return addShapeGuardPolymorphic(obj, receiverShapes);
+}
+
 bool
 IonBuilder::getPropTryCommonGetter(bool *emitted, MDefinition *obj, PropertyName *name,
                                    types::TemporaryTypeSet *types)
@@ -10175,21 +10194,32 @@ IonBuilder::getPropTryCommonGetter(bool *emitted, MDefinition *obj, PropertyName
     Shape *lastProperty = nullptr;
     JSFunction *commonGetter = nullptr;
     Shape *globalShape = nullptr;
-    JSObject *foundProto = inspector->commonGetPropFunction(pc, &lastProperty, &commonGetter, &globalShape);
-    if (!foundProto)
+    JSObject *foundProto = nullptr;
+    bool isOwnProperty = false;
+    BaselineInspector::ShapeVector receiverShapes(alloc());
+    if (!inspector->commonGetPropFunction(pc, &foundProto, &lastProperty, &commonGetter,
+                                          &globalShape, &isOwnProperty, receiverShapes))
+    {
         return true;
+    }
 
     types::TemporaryTypeSet *objTypes = obj->resultTypeSet();
     MDefinition *guard = nullptr;
     MDefinition *globalGuard = nullptr;
-    bool canUseCommonGetter =
+    bool canUseTIForGetter =
         testCommonGetterSetter(objTypes, name, /* isGetter = */ true,
                                foundProto, lastProperty, &guard, globalShape,
                                &globalGuard);
-    if (!canUseCommonGetter)
-        return true;
+    if (!canUseTIForGetter) {
+        // If type information is bad, we can still optimize the getter if we
+        // shape guard.
+        obj = addShapeGuardsForGetterSetter(obj, foundProto, lastProperty, receiverShapes,
+                                            isOwnProperty);
+        if (!obj)
+            return false;
+    }
 
-    bool isDOM = objTypes->isDOMClass(constraints());
+    bool isDOM = objTypes && objTypes->isDOMClass(constraints());
 
     if (isDOM && testShouldDOMCall(objTypes, commonGetter, JSJitInfo::Getter)) {
         const JSJitInfo *jitinfo = commonGetter->jitInfo();
@@ -10229,7 +10259,7 @@ IonBuilder::getPropTryCommonGetter(bool *emitted, MDefinition *obj, PropertyName
     }
 
     // Don't call the getter with a primitive value.
-    if (objTypes->getKnownMIRType() != MIRType_Object) {
+    if (obj->type() != MIRType_Object) {
         MGuardObject *guardObj = MGuardObject::New(alloc(), obj);
         current->add(guardObj);
         obj = guardObj;
@@ -10420,17 +10450,9 @@ IonBuilder::getPropTryInlineAccess(bool *emitted, MDefinition *obj, PropertyName
         return false;
 
     if (sameSlot && unboxedGroups.empty()) {
-        MGuardShapePolymorphic *guard = MGuardShapePolymorphic::New(alloc(), obj);
-        current->add(guard);
-        obj = guard;
-
-        if (failedShapeGuard_)
-            guard->setNotMovable();
-
-        for (size_t i = 0; i < nativeShapes.length(); i++) {
-            if (!guard->addShape(nativeShapes[i]))
-                return false;
-        }
+        obj = addShapeGuardPolymorphic(obj, nativeShapes);
+        if (!obj)
+            return false;
 
         if (!loadSlot(obj, propShapes[0], rvalType, barrier, types))
             return false;
@@ -10682,21 +10704,29 @@ IonBuilder::setPropTryCommonSetter(bool *emitted, MDefinition *obj,
 
     Shape *lastProperty = nullptr;
     JSFunction *commonSetter = nullptr;
-    JSObject *foundProto = inspector->commonSetPropFunction(pc, &lastProperty, &commonSetter);
-    if (!foundProto) {
+    JSObject *foundProto = nullptr;
+    bool isOwnProperty;
+    BaselineInspector::ShapeVector receiverShapes(alloc());
+    if (!inspector->commonSetPropFunction(pc, &foundProto, &lastProperty, &commonSetter, &isOwnProperty,
+                                          receiverShapes))
+    {
         trackOptimizationOutcome(TrackedOutcome::NoProtoFound);
         return true;
     }
 
     types::TemporaryTypeSet *objTypes = obj->resultTypeSet();
     MDefinition *guard = nullptr;
-    bool canUseCommonSetter =
+    bool canUseTIForSetter =
         testCommonGetterSetter(objTypes, name, /* isGetter = */ false,
                                foundProto, lastProperty, &guard);
-    if (!canUseCommonSetter)
-        return true;
-
-    bool isDOM = objTypes->isDOMClass(constraints());
+    if (!canUseTIForSetter) {
+        // If type information is bad, we can still optimize the setter if we
+        // shape guard.
+        obj = addShapeGuardsForGetterSetter(obj, foundProto, lastProperty, receiverShapes,
+                                            isOwnProperty);
+        if (!obj)
+            return false;
+    }
 
     // Emit common setter.
 
@@ -10705,7 +10735,7 @@ IonBuilder::setPropTryCommonSetter(bool *emitted, MDefinition *obj,
     // properties.
 
     // Try emitting dom call.
-    if (!setPropTryCommonDOMSetter(emitted, obj, value, commonSetter, isDOM))
+    if (!setPropTryCommonDOMSetter(emitted, obj, value, commonSetter, objTypes))
         return false;
 
     if (*emitted) {
@@ -10714,7 +10744,7 @@ IonBuilder::setPropTryCommonSetter(bool *emitted, MDefinition *obj,
     }
 
     // Don't call the setter with a primitive value.
-    if (objTypes->getKnownMIRType() != MIRType_Object) {
+    if (obj->type() != MIRType_Object) {
         MGuardObject *guardObj = MGuardObject::New(alloc(), obj);
         current->add(guardObj);
         obj = guardObj;
@@ -10777,14 +10807,13 @@ IonBuilder::setPropTryCommonSetter(bool *emitted, MDefinition *obj,
 bool
 IonBuilder::setPropTryCommonDOMSetter(bool *emitted, MDefinition *obj,
                                       MDefinition *value, JSFunction *setter,
-                                      bool isDOM)
+                                      types::TemporaryTypeSet *objTypes)
 {
     MOZ_ASSERT(*emitted == false);
 
-    if (!isDOM)
+    if (!objTypes || !objTypes->isDOMClass(constraints()))
         return true;
 
-    types::TemporaryTypeSet *objTypes = obj->resultTypeSet();
     if (!testShouldDOMCall(objTypes, setter, JSJitInfo::Setter))
         return true;
 
@@ -11098,17 +11127,9 @@ IonBuilder::setPropTryInlineAccess(bool *emitted, MDefinition *obj,
         return false;
 
     if (sameSlot && unboxedGroups.empty()) {
-        MGuardShapePolymorphic *guard = MGuardShapePolymorphic::New(alloc(), obj);
-        current->add(guard);
-        obj = guard;
-
-        if (failedShapeGuard_)
-            guard->setNotMovable();
-
-        for (size_t i = 0; i < nativeShapes.length(); i++) {
-            if (!guard->addShape(nativeShapes[i]))
-                return false;
-        }
+        obj = addShapeGuardPolymorphic(obj, nativeShapes);
+        if (!obj)
+            return false;
 
         bool needsBarrier = objTypes->propertyNeedsBarrier(constraints(), NameToId(name));
         if (!storeSlot(obj, propShapes[0], value, needsBarrier))
@@ -12019,6 +12040,28 @@ IonBuilder::addShapeGuard(MDefinition *obj, Shape *const shape, BailoutKind bail
     return guard;
 }
 
+MInstruction *
+IonBuilder::addShapeGuardPolymorphic(MDefinition *obj, const BaselineInspector::ShapeVector &shapes)
+{
+    if (shapes.length() == 1)
+        return addShapeGuard(obj, shapes[0], Bailout_ShapeGuard);
+
+    MOZ_ASSERT(shapes.length() > 1);
+
+    MGuardShapePolymorphic *guard = MGuardShapePolymorphic::New(alloc(), obj);
+    current->add(guard);
+
+    if (failedShapeGuard_)
+        guard->setNotMovable();
+
+    for (size_t i = 0, len = shapes.length(); i < len; i++) {
+        if (!guard->addShape(shapes[i]))
+            return nullptr;
+    }
+
+    return guard;
+}
+
 types::TemporaryTypeSet *
 IonBuilder::bytecodeTypes(jsbytecode *pc)
 {

js/src/jit/IonBuilder.h

@@ -394,6 +394,8 @@ class IonBuilder
     MDefinition *addMaybeCopyElementsForWrite(MDefinition *object);
     MInstruction *addBoundsCheck(MDefinition *index, MDefinition *length);
     MInstruction *addShapeGuard(MDefinition *obj, Shape *const shape, BailoutKind bailoutKind);
+    MInstruction *addShapeGuardPolymorphic(MDefinition *obj,
+                                           const BaselineInspector::ShapeVector &shapes);
 
     MDefinition *convertShiftToMaskForStaticTypedArray(MDefinition *id,
                                                        Scalar::Type viewType);
@@ -451,7 +453,7 @@ class IonBuilder
                                 PropertyName *name, MDefinition *value);
     bool setPropTryCommonDOMSetter(bool *emitted, MDefinition *obj,
                                    MDefinition *value, JSFunction *setter,
-                                   bool isDOM);
+                                   types::TemporaryTypeSet *objTypes);
     bool setPropTryDefiniteSlot(bool *emitted, MDefinition *obj,
                                 PropertyName *name, MDefinition *value,
                                 bool barrier, types::TemporaryTypeSet *objTypes);
@@ -873,6 +875,10 @@ class IonBuilder
     bool testShouldDOMCall(types::TypeSet *inTypes,
                            JSFunction *func, JSJitInfo::OpType opType);
 
+    MDefinition *addShapeGuardsForGetterSetter(MDefinition *obj, JSObject *holder, Shape *holderShape,
+                                               const BaselineInspector::ShapeVector &receiverShapes,
+                                               bool isOwnProperty);
+
     bool annotateGetPropertyCache(MDefinition *obj, MGetPropertyCache *getPropCache,
                                   types::TemporaryTypeSet *objTypes,
                                   types::TemporaryTypeSet *pushedTypes);