Bug 1101823 part 1. Refactor testCommonGetterSetter to just return a boolean indicating whether it's OK to proceed with the common getter/setter, and put the shape guards, into outparams. r=efaust
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 26 Nov 2014 17:50:40 -0500
changeset 242118 8665c35187d0053b25ad86bd7b4266d457ea12de
parent 242117 b60b88f91313c26d9d9caece7c58554f5ff4257d
child 242119 6062040e81e6062f2524b9eaea8710be7599ac4c
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1101823
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1101823 part 1. Refactor testCommonGetterSetter to just return a boolean indicating whether it's OK to proceed with the common getter/setter, and put the shape guards, into outparams. r=efaust
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -9075,48 +9075,54 @@ IonBuilder::freezePropertiesForCommonPro
             // with properties unknown to TI.
             if (type->proto() == TaggedProto(foundProto))
                 break;
             type = types::TypeObjectKey::get(type->proto().toObjectOrNull());
         }
     }
 }
 
-inline MDefinition *
+inline bool
 IonBuilder::testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name,
                                    bool isGetter, JSObject *foundProto, Shape *lastProperty,
-                                   Shape *globalShape/* = nullptr*/)
-{
+                                   MDefinition **guard,
+                                   Shape *globalShape/* = nullptr*/,
+                                   MDefinition **globalGuard/* = nullptr */)
+{
+    MOZ_ASSERT_IF(globalShape, globalGuard);
     bool guardGlobal;
 
     // Check if all objects being accessed will lookup the name through foundProto.
     if (!objectsHaveCommonPrototype(types, name, isGetter, foundProto, &guardGlobal) ||
         (guardGlobal && !globalShape))
     {
-        return nullptr;
+        return false;
     }
 
     // We can optimize the getter/setter, so freeze all involved properties to
     // ensure there isn't a lower shadowing getter or setter installed in the
     // future.
     freezePropertiesForCommonPrototype(types, name, foundProto, guardGlobal);
 
     // Add a shape guard on the prototype we found the property on. The rest of
     // the prototype chain is guarded by TI freezes, except when name is a global
     // name. In this case, we also have to guard on the globals shape to be able
-    // to optimize. 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.
+    // to optimize, because the way global property sets are handled means
+    // freezing doesn't work for what we want here. 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.
     if (guardGlobal) {
         JSObject *obj = &script()->global();
         MDefinition *globalObj = constant(ObjectValue(*obj));
-        addShapeGuard(globalObj, globalShape, Bailout_ShapeGuard);
+        *globalGuard = addShapeGuard(globalObj, globalShape, Bailout_ShapeGuard);
     }
 
     MInstruction *wrapper = constant(ObjectValue(*foundProto));
-    return addShapeGuard(wrapper, lastProperty, Bailout_ShapeGuard);
+    *guard = addShapeGuard(wrapper, lastProperty, Bailout_ShapeGuard);
+    return true;
 }
 
 void
 IonBuilder::replaceMaybeFallbackFunctionGetter(MGetPropertyCache *cache)
 {
     // Discard the last prior resume point of the previous MGetPropertyCache.
     WrapMGetPropertyCache rai(maybeFallbackFunctionGetter_);
     maybeFallbackFunctionGetter_ = cache;
@@ -9664,19 +9670,23 @@ IonBuilder::getPropTryCommonGetter(bool 
     Shape *lastProperty = nullptr;
     JSFunction *commonGetter = nullptr;
     Shape *globalShape = nullptr;
     JSObject *foundProto = inspector->commonGetPropFunction(pc, &lastProperty, &commonGetter, &globalShape);
     if (!foundProto)
         return true;
 
     types::TemporaryTypeSet *objTypes = obj->resultTypeSet();
-    MDefinition *guard = testCommonGetterSetter(objTypes, name, /* isGetter = */ true,
-                                                foundProto, lastProperty, globalShape);
-    if (!guard)
+    MDefinition *guard = nullptr;
+    MDefinition *globalGuard = nullptr;
+    bool canUseCommonGetter =
+        testCommonGetterSetter(objTypes, name, /* isGetter = */ true,
+                               foundProto, lastProperty, &guard, globalShape,
+                               &globalGuard);
+    if (!canUseCommonGetter)
         return true;
 
     bool isDOM = objTypes->isDOMClass();
 
     if (isDOM && testShouldDOMCall(objTypes, commonGetter, JSJitInfo::Getter)) {
         const JSJitInfo *jitinfo = commonGetter->jitInfo();
         MInstruction *get;
         if (jitinfo->isAlwaysInSlot) {
@@ -10115,19 +10125,21 @@ IonBuilder::setPropTryCommonSetter(bool 
 
     Shape *lastProperty = nullptr;
     JSFunction *commonSetter = nullptr;
     JSObject *foundProto = inspector->commonSetPropFunction(pc, &lastProperty, &commonSetter);
     if (!foundProto)
         return true;
 
     types::TemporaryTypeSet *objTypes = obj->resultTypeSet();
-    MDefinition *guard = testCommonGetterSetter(objTypes, name, /* isGetter = */ false,
-                                                foundProto, lastProperty);
-    if (!guard)
+    MDefinition *guard = nullptr;
+    bool canUseCommonSetter =
+        testCommonGetterSetter(objTypes, name, /* isGetter = */ false,
+                               foundProto, lastProperty, &guard);
+    if (!canUseCommonSetter)
         return true;
 
     bool isDOM = objTypes->isDOMClass();
 
     // Emit common setter.
 
     // Setters can be called even if the property write needs a type
     // barrier, as calling the setter does not actually write any data
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -836,19 +836,23 @@ class IonBuilder
     MDefinition *patchInlinedReturn(CallInfo &callInfo, MBasicBlock *exit, MBasicBlock *bottom);
     MDefinition *patchInlinedReturns(CallInfo &callInfo, MIRGraphReturns &returns,
                                      MBasicBlock *bottom);
 
     bool objectsHaveCommonPrototype(types::TemporaryTypeSet *types, PropertyName *name,
                                     bool isGetter, JSObject *foundProto, bool *guardGlobal);
     void freezePropertiesForCommonPrototype(types::TemporaryTypeSet *types, PropertyName *name,
                                             JSObject *foundProto, bool allowEmptyTypesForGlobal = false);
-    MDefinition *testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name,
-                                        bool isGetter, JSObject *foundProto, Shape *lastProperty,
-                                        Shape *globalShape = nullptr);
+    /*
+     * Callers must pass a non-null globalGuard if they pass a non-null globalShape.
+     */
+    bool testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name,
+                                bool isGetter, JSObject *foundProto, Shape *lastProperty,
+                                MDefinition **guard, Shape *globalShape = nullptr,
+                                MDefinition **globalGuard = nullptr);
     bool testShouldDOMCall(types::TypeSet *inTypes,
                            JSFunction *func, JSJitInfo::OpType opType);
 
     bool annotateGetPropertyCache(MDefinition *obj, MGetPropertyCache *getPropCache,
                                   types::TemporaryTypeSet *objTypes,
                                   types::TemporaryTypeSet *pushedTypes);
 
     MGetPropertyCache *getInlineableGetPropertyCache(CallInfo &callInfo);