Bug 939581 part 3. Mark DOM calls as movable as needed and allow them to be CSE'd. r=efaust

dom/bindings/Codegen.py

@@ -6391,8 +6391,16 @@ class CGMemberJITInfo(CGThing):
                 # one of them must take arguments.
                 methodInfal = False
                 args = None
+                movable = False
             else:
                 sig = sigs[0]
+                # For pure methods, it's OK to set movable to our notion of
+                # infallible on the C++ side, without considering argument
+                # conversions, since argument conversions that can reliably
+                # throw would be effectful anyway and the jit doesn't move
+                # effectful things.
+                hasInfallibleImpl = "infallible" in self.descriptor.getExtendedAttributes(self.member)
+                movable = methodPure and hasInfallibleImpl
                 # XXXbz can we move the smarts about fallibility due to arg
                 # conversions into the JIT, using our new args stuff?
                 if (len(sig[1]) != 0 or
@@ -6400,19 +6408,19 @@ class CGMemberJITInfo(CGThing):
                     # We have arguments or our return-value boxing can fail
                     methodInfal = False
                 else:
-                    methodInfal = "infallible" in self.descriptor.getExtendedAttributes(self.member)
+                    methodInfal = hasInfallibleImpl
                 # For now, only bother to output args if we're pure
                 if methodPure:
                     args = sig[1]
                 else:
                     args = None
 
-            if args:
+            if args is not None:
                 aliasSet = "AliasDOMSets"
             else:
                 aliasSet = "AliasEverything"
             result = self.defineJitInfo(methodinfo, method, "Method",
-                                        methodInfal, False, aliasSet, False, "0",
+                                        methodInfal, movable, aliasSet, False, "0",
                                         [s[0] for s in sigs], args)
             return result
         raise TypeError("Illegal member type to CGPropertyJITInfo")

js/src/jit/IonBuilder.cpp

@@ -5202,6 +5202,9 @@ IonBuilder::makeCallHelper(JSFunction *target, CallInfo &callInfo, bool cloneAtC
     for (int32_t i = callInfo.argc() - 1; i >= 0; i--)
         call->addArg(i + 1, callInfo.getArg(i));
 
+    // Now that we've told it about all the args, compute whether it's movable
+    call->computeMovable();
+
     // Inline the constructor on the caller-side.
     if (callInfo.constructing()) {
         MDefinition *create = createThis(target, callInfo.fun());

js/src/jit/MIR.cpp

@@ -690,12 +690,15 @@ MCallDOMNative::getAliasSet() const
         // getArg(0) is "this", so skip it
         MDefinition *arg = getArg(argIndex+1);
         MIRType actualType = arg->type();
-        // The only way to get side-effects is if we're passing in
+        // The only way to reliably avoid side-effects given the informtion we
-        // something that might be an object to an argument that
+        // have here is if we're passing in a known primitive value to an
-        // expects a numeric, string, or boolean value.
+        // argument that expects a primitive value.  XXXbz maybe we need to
-        if ((actualType == MIRType_Value || actualType == MIRType_Object) &&
+        // communicate better information.  For example, a sequence argument
-            (*argType &
+        // will sort of unavoidably have side effects, while a typed array
-             (JSJitInfo::Boolean | JSJitInfo::String | JSJitInfo::Numeric)))
+        // argument won't have any, but both are claimed to be
+        // JSJitInfo::Object.
+        if ((actualType == MIRType_Value || actualType == MIRType_Object) ||
+            (*argType & JSJitInfo::Object))
          {
              return AliasSet::Store(AliasSet::Any);
          }
@@ -706,6 +709,59 @@ MCallDOMNative::getAliasSet() const
     return AliasSet::Load(AliasSet::DOMProperty);
 }
 
+void
+MCallDOMNative::computeMovable()
+{
+    // We are movable if the jitinfo says we can be and if we're also not
+    // effectful.  The jitinfo can't check for the latter, since it depends on
+    // the types of our arguments.
+    JS_ASSERT(getSingleTarget() && getSingleTarget()->isNative());
+
+    const JSJitInfo *jitInfo = getSingleTarget()->jitInfo();
+    JS_ASSERT(jitInfo);
+
+    JS_ASSERT_IF(jitInfo->isMovable,
+                 jitInfo->aliasSet != JSJitInfo::AliasEverything);
+
+    if (jitInfo->isMovable && !isEffectful())
+        setMovable();
+}
+
+bool
+MCallDOMNative::congruentTo(MDefinition *ins) const
+{
+    if (!isMovable())
+        return false;
+
+    if (!ins->isCall())
+        return false;
+
+    MCall *call = ins->toCall();
+
+    if (!call->isCallDOMNative())
+        return false;
+
+    if (getSingleTarget() != call->getSingleTarget())
+        return false;
+
+    if (isConstructing() != call->isConstructing())
+        return false;
+
+    if (numActualArgs() != call->numActualArgs())
+        return false;
+
+    if (needsArgCheck() != call->needsArgCheck())
+        return false;
+
+    if (!congruentIfOperandsEqual(call))
+        return false;
+
+    // The other call had better be movable at this point!
+    JS_ASSERT(call->isMovable());
+
+    return true;
+}
+
 MApplyArgs *
 MApplyArgs::New(TempAllocator &alloc, JSFunction *target, MDefinition *fun, MDefinition *argc,
                 MDefinition *self)

js/src/jit/MIR.h

@@ -1830,10 +1830,6 @@ class MCall
         return getOperand(NumNonArgumentOperands + index);
     }
 
-    void replaceArg(uint32_t index, MDefinition *def) {
-        replaceOperand(NumNonArgumentOperands + index, def);
-    }
-
     static size_t IndexOfThis() {
         return NumNonArgumentOperands;
     }
@@ -1877,6 +1873,13 @@ class MCall
     virtual bool isCallDOMNative() const {
         return false;
     }
+
+    // A method that can be called to tell the MCall to figure out whether it's
+    // movable or not.  This can't be done in the constructor, because it
+    // depends on the arguments to the call, and those aren't passed to the
+    // constructor but are set up later via addArg.
+    virtual void computeMovable() {
+    }
 };
 
 class MCallDOMNative : public MCall
@@ -1897,9 +1900,13 @@ class MCallDOMNative : public MCall
   public:
     virtual AliasSet getAliasSet() const MOZ_OVERRIDE;
 
+    virtual bool congruentTo(MDefinition *ins) const MOZ_OVERRIDE;
+
     virtual bool isCallDOMNative() const MOZ_OVERRIDE {
         return true;
     }
+
+    virtual void computeMovable() MOZ_OVERRIDE;
 };
 
 // fun.apply(self, arguments)