Bug 881390 - IonMonkey: Hoist floating-point constants out of loops. r=h4writer

js/src/ion/LICM.cpp

@@ -52,6 +52,12 @@ class Loop
     // the loop is first entered and where hoisted instructions will be placed.
     MBasicBlock* preLoop_;
 
+    // This indicates whether the loop contains calls or other things which
+    // clobber most or all floating-point registers. In such loops,
+    // floating-point constants should not be hoisted unless it enables further
+    // hoisting.
+    bool containsPossibleCall_;
+
     bool hoistInstructions(InstructionQueue &toHoist);
 
     // Utility methods for invariance testing and instruction hoisting.
@@ -72,6 +78,8 @@ class Loop
     inline bool isHoistable(const MDefinition *ins) const {
         return ins->isMovable() && !ins->isEffectful() && !ins->neverHoist();
     }
+
+    bool requiresHoistedUse(const MDefinition *ins) const;
 };
 
 } /* namespace anonymous */
@@ -116,7 +124,8 @@ LICM::analyze()
 
 Loop::Loop(MIRGenerator *mir, MBasicBlock *header)
   : mir(mir),
-    header_(header)
+    header_(header),
+    containsPossibleCall_(false)
 {
     preLoop_ = header_->getPredecessor(0);
 }
@@ -174,6 +183,11 @@ Loop::init()
         for (MInstructionIterator i = block->begin(); i != block->end(); i++) {
             MInstruction *ins = *i;
 
+            // Remember whether this loop contains anything which clobbers most
+            // or all floating-point registers. This is just a rough heuristic.
+            if (ins->possiblyCalls())
+                containsPossibleCall_ = true;
+
             if (isHoistable(ins)) {
                 if (!insertInWorklist(ins))
                     return LoopReturn_Error;
@@ -225,6 +239,22 @@ Loop::optimize()
     return true;
 }
 
+bool
+Loop::requiresHoistedUse(const MDefinition *ins) const
+{
+    if (ins->isConstantElements() || ins->isBox())
+        return true;
+
+    // Integer constants can often be folded as immediates and aren't worth
+    // hoisting on their own, in general. Floating-point constants typically
+    // are worth hoisting, unless they'll end up being spilled (eg. due to a
+    // call).
+    if (ins->isConstant() && (ins->type() != MIRType_Double || containsPossibleCall_))
+        return true;
+
+    return false;
+}
+
 bool
 Loop::hoistInstructions(InstructionQueue &toHoist)
 {
@@ -232,10 +262,10 @@ Loop::hoistInstructions(InstructionQueue &toHoist)
     for (int32_t i = toHoist.length() - 1; i >= 0; i--) {
         MInstruction *ins = toHoist[i];
 
-        // Don't hoist MConstantElements, MConstant and MBox
-        // if it doesn't enable us to hoist one of its uses.
-        // We want those instructions as close as possible to their use.
-        if (ins->isConstantElements() || ins->isConstant() || ins->isBox()) {
+        // Don't hoist a cheap constant if it doesn't enable us to hoist one of
+        // its uses. We want those instructions as close as possible to their
+        // use, to facilitate folding and minimize register pressure.
+        if (requiresHoistedUse(ins)) {
             bool loopInvariantUse = false;
             for (MUseDefIterator use(ins); use; use++) {
                 if (use.def()->isLoopInvariant()) {

js/src/ion/MIR.h

@@ -341,6 +341,12 @@ class MDefinition : public MNode
     // For LICM.
     virtual bool neverHoist() const { return false; }
 
+    // Also for LICM. Test whether this definition is likely to be a call, which
+    // would clobber all or many of the floating-point registers, such that
+    // hoisting floating-point constants out of containing loops isn't likely to
+    // be worthwhile.
+    virtual bool possiblyCalls() const { return false; }
+
     void setTrackedPc(jsbytecode *pc) {
         trackedPc_ = pc;
     }
@@ -1113,6 +1119,9 @@ class MThrow
     virtual AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MNewParallelArray : public MNullaryInstruction
@@ -1313,6 +1322,9 @@ class MInitProp
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MInitElem
@@ -1346,6 +1358,9 @@ class MInitElem
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Designates the start of call frame construction.
@@ -1499,6 +1514,10 @@ class MCall
     AliasSet getAliasSet() const {
         return AliasSet::Store(AliasSet::Any);
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // fun.apply(self, arguments)
@@ -1543,6 +1562,9 @@ class MApplyArgs
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MGetDynamicName
@@ -1575,6 +1597,9 @@ class MGetDynamicName
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Bailout if the input string contains 'arguments'
@@ -1605,6 +1630,9 @@ class MFilterArguments
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MCallDirectEval
@@ -1649,6 +1677,10 @@ class MCallDirectEval
         return this;
     }
 
+    bool possiblyCalls() const {
+        return true;
+    }
+
   private:
     jsbytecode *pc_;
 };
@@ -2182,6 +2214,9 @@ class MCreateThisWithProto
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Caller-side allocation of |this| for |new|:
@@ -2214,6 +2249,9 @@ class MCreateThis
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Eager initialization of arguments object.
@@ -2245,6 +2283,9 @@ class MCreateArgumentsObject
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MGetArgumentsObjectArg
@@ -2339,6 +2380,9 @@ class MRunOncePrologue
     static MRunOncePrologue *New() {
         return new MRunOncePrologue();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Given a MIRType_Value A and a MIRType_Object B:
@@ -3120,6 +3164,10 @@ class MAtan2
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Inline implementation of Math.pow().
@@ -3156,6 +3204,9 @@ class MPow
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Inline implementation of Math.pow(x, 0.5), which subtly differs from Math.sqrt(x).
@@ -3203,6 +3254,10 @@ class MRandom : public MNullaryInstruction
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MMathFunction
@@ -3270,6 +3325,10 @@ class MMathFunction
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MAdd : public MBinaryArithInstruction
@@ -3949,7 +4008,9 @@ class MDefVar : public MUnaryInstruction
     MDefinition *scopeChain() const {
         return getOperand(0);
     }
-
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MDefFun : public MUnaryInstruction
@@ -3975,6 +4036,9 @@ class MDefFun : public MUnaryInstruction
     MDefinition *scopeChain() const {
         return getOperand(0);
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MRegExp : public MNullaryInstruction
@@ -4008,6 +4072,9 @@ class MRegExp : public MNullaryInstruction
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MRegExpTest
@@ -4039,6 +4106,10 @@ class MRegExpTest
     TypePolicy *typePolicy() {
         return this;
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MLambda
@@ -4957,6 +5028,9 @@ class MArrayConcat
     AliasSet getAliasSet() const {
         return AliasSet::Store(AliasSet::Element | AliasSet::ObjectFields);
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MLoadTypedArrayElement
@@ -6330,6 +6404,10 @@ class MForkJoinSlice
         // (For all intents and purposes)
         return AliasSet::None();
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Store to vp[slot] (slots that are not inline in an object).
@@ -6454,6 +6532,9 @@ class MCallGetIntrinsicValue : public MNullaryInstruction
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MCallsiteCloneCache
@@ -6598,6 +6679,9 @@ class MCallSetProperty
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MSetPropertyCache
@@ -6691,6 +6775,9 @@ class MCallGetProperty
             return AliasSet::Store(AliasSet::Any);
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Inline call to handle lhs[rhs]. The first input is a Value so that this
@@ -6714,6 +6801,9 @@ class MCallGetElement
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MCallSetElement
@@ -6735,6 +6825,9 @@ class MCallSetElement
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MCallInitElementArray
@@ -6773,6 +6866,9 @@ class MCallInitElementArray
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MSetDOMProperty
@@ -6812,6 +6908,10 @@ class MSetDOMProperty
     TypePolicy *typePolicy() {
         return this;
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MGetDOMProperty
@@ -6897,6 +6997,9 @@ class MGetDOMProperty
         return AliasSet::Store(AliasSet::Any);
     }
 
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MStringLength
@@ -7107,6 +7210,9 @@ class MIn
     TypePolicy *typePolicy() {
         return this;
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 
@@ -7323,6 +7429,9 @@ class MRest
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MRestPar
@@ -7364,6 +7473,9 @@ class MRestPar
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Guard on an object being allocated in the current slice.
@@ -7397,6 +7509,9 @@ class MGuardThreadLocalObject
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // Given a value, guard that the value is in a particular TypeSet, then returns
@@ -7537,6 +7652,9 @@ class MNewSlots : public MNullaryInstruction
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 class MNewDeclEnvObject : public MNullaryInstruction
@@ -7769,6 +7887,10 @@ class MNewDenseArrayPar : public MBinaryInstruction
     JSObject *templateObject() const {
         return templateObject_;
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // A resume point contains the information needed to reconstruct the interpreter
@@ -8281,6 +8403,10 @@ class MAsmJSCall MOZ_FINAL : public MInstruction
     size_t spIncrement() const {
         return spIncrement_;
     }
+
+    bool possiblyCalls() const {
+        return true;
+    }
 };
 
 // The asm.js version doesn't use the bail mechanism: instead it throws and