Bug 845458 - IonMonkey: simplify constant double handling on x86 (r=dvander)

js/public/HashTable.h

@@ -527,6 +527,28 @@ template <class T>
 struct DefaultHasher<T *> : PointerHasher<T *, tl::FloorLog2<sizeof(void *)>::result>
 {};
 
+// For doubles, we can xor the two uint32s.
+template <>
+struct DefaultHasher<double>
+{
+    typedef double Lookup;
+    static HashNumber hash(double d) {
+        JS_STATIC_ASSERT(sizeof(HashNumber) == 4);
+        union {
+            struct {
+                uint32_t lo;
+                uint32_t hi;
+            } s;
+            double d;
+        } u;
+        u.d = d;
+        return u.s.lo ^ u.s.hi;
+    }
+    static bool match(double lhs, double rhs) {
+        return lhs == rhs;
+    }
+};
+
 /*****************************************************************************/
 
 // Both HashMap and HashSet are implemented by a single HashTable that is even

js/src/assembler/assembler/X86Assembler.h

@@ -2453,6 +2453,11 @@ public:
         m_formatter.jumpTablePointer(ptr);
     }
 
+    void doubleConstant(double d)
+    {
+        m_formatter.doubleConstant(d);
+    }
+
     // Linking & patching:
     //
     // 'link' and 'patch' methods are for use on unprotected code - such as the code
@@ -3012,6 +3017,17 @@ private:
 #endif
         }
 
+        void doubleConstant(double d)
+        {
+            m_buffer.ensureSpace(sizeof(double));
+            union {
+                uint64_t u64;
+                double d;
+            } u;
+            u.d = d;
+            m_buffer.putInt64Unchecked(u.u64);
+        }
+
         // Administrative methods:
 
         size_t size() const { return m_buffer.size(); }

js/src/ion/CodeGenerator.cpp

@@ -4423,8 +4423,6 @@ CodeGenerator::link()
     if (executionMode == ParallelExecution)
         ionScript->zeroParallelInvalidatedScripts();
 
-    linkAbsoluteLabels();
-
     // The correct state for prebarriers is unknown until the end of compilation,
     // since a GC can occur during code generation. All barriers are emitted
     // off-by-default, and are toggled on here if necessary.

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

@@ -1155,26 +1155,6 @@ CodeGeneratorARM::visitUnbox(LUnbox *unbox)
     return true;
 }
 
-void
-CodeGeneratorARM::linkAbsoluteLabels()
-{
-    // arm doesn't have deferred doubles, so this whole thing should be a NOP (right?)
-    // deferred doubles are an x86 mechanism for loading doubles into registers by storing
-    // them after the function body, then referring to them by their absolute address.
-    // On arm, everything should just go in a pool.
-# if 0
-    JS_NOT_REACHED("Absolute Labels NYI");
-    UnrootedScript script = gen->info().script();
-    IonCode *method = script->ion->method();
-
-    for (size_t i = 0; i < deferredDoubles_.length(); i++) {
-        DeferredDouble *d = deferredDoubles_[i];
-        const Value &v = script->ion->getConstant(d->index());
-        MacroAssembler::Bind(method, d->label(), &v);
-    }
-#endif
-}
-
 bool
 CodeGeneratorARM::visitDouble(LDouble *ins)
 {

js/src/ion/arm/CodeGenerator-arm.h

@@ -117,9 +117,6 @@ class CodeGeneratorARM : public CodeGeneratorShared
     void storeElementTyped(const LAllocation *value, MIRType valueType, MIRType elementType,
                            const Register &elements, const LAllocation *index);
 
-  protected:
-    void linkAbsoluteLabels();
-
   public:
     CodeGeneratorARM(MIRGenerator *gen, LIRGraph *graph);
 

js/src/ion/shared/Assembler-shared.h

@@ -331,6 +331,9 @@ class CodeLabel
   public:
     CodeLabel()
     { }
+    CodeLabel(const AbsoluteLabel &dest)
+       : dest_(dest)
+    { }
     AbsoluteLabel *dest() {
         return &dest_;
     }

js/src/ion/shared/Assembler-x86-shared.h

@@ -28,8 +28,8 @@ class AssemblerX86Shared
         { }
     };
 
-    js::Vector<CodeLabel, 0, SystemAllocPolicy> codeLabels_;
-    js::Vector<RelativePatch, 8, SystemAllocPolicy> jumps_;
+    Vector<CodeLabel, 0, SystemAllocPolicy> codeLabels_;
+    Vector<RelativePatch, 8, SystemAllocPolicy> jumps_;
     CompactBufferWriter jumpRelocations_;
     CompactBufferWriter dataRelocations_;
     CompactBufferWriter preBarriers_;
@@ -207,6 +207,10 @@ class AssemblerX86Shared
         masm.jumpTablePointer(label->prev());
         label->setPrev(masm.size());
     }
+    void writeDoubleConstant(double d, Label *label) {
+        label->bind(masm.size());
+        masm.doubleConstant(d);
+    }
     void movl(const Imm32 &imm32, const Register &dest) {
         masm.movl_i32r(imm32.value, dest.code());
     }

js/src/ion/shared/CodeGenerator-shared.h

@@ -313,9 +313,6 @@ class CodeGeneratorShared : public LInstructionVisitor
     bool addOutOfLineCode(OutOfLineCode *code);
     bool generateOutOfLineCode();
 
-    void linkAbsoluteLabels() {
-    }
-
   private:
     void generateInvalidateEpilogue();
 

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

@@ -88,6 +88,14 @@ CodeGeneratorX86Shared::emitBranch(Assembler::Condition cond, MBasicBlock *mirTr
     }
 }
 
+bool
+CodeGeneratorX86Shared::visitDouble(LDouble *ins)
+{
+    const LDefinition *out = ins->getDef(0);
+    masm.loadConstantDouble(ins->getDouble(), ToFloatRegister(out));
+    return true;
+}
+
 bool
 CodeGeneratorX86Shared::visitTestIAndBranch(LTestIAndBranch *test)
 {

js/src/ion/shared/CodeGenerator-x86-shared.h

@@ -77,6 +77,7 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
 
   public:
     // Instruction visitors.
+    virtual bool visitDouble(LDouble *ins);
     virtual bool visitMinMaxD(LMinMaxD *ins);
     virtual bool visitAbsD(LAbsD *ins);
     virtual bool visitSqrtD(LSqrtD *ins);

js/src/ion/shared/LIR-x86-shared.h

@@ -205,6 +205,23 @@ class LMulI : public LBinaryMath<0, 1>
     }
 };
 
+// Constant double.
+class LDouble : public LInstructionHelper<1, 0, 0>
+{
+    double d_;
+
+  public:
+    LIR_HEADER(Double)
+
+    LDouble(double d)
+      : d_(d)
+    { }
+
+    double getDouble() const {
+        return d_;
+    }
+};
+
 } // namespace ion
 } // namespace js
 

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

@@ -125,3 +125,23 @@ LIRGeneratorX86Shared::lowerUrshD(MUrsh *mir)
     LUrshD *lir = new LUrshD(lhsUse, rhsAlloc, tempCopy(lhs, 0));
     return define(lir, mir);
 }
+
+bool
+LIRGeneratorX86Shared::lowerConstantDouble(double d, MInstruction *mir)
+{
+    return define(new LDouble(d), mir);
+}
+
+bool
+LIRGeneratorX86Shared::visitConstant(MConstant *ins)
+{
+    if (ins->type() == MIRType_Double)
+        return lowerConstantDouble(ins->value().toDouble(), ins);
+
+    // Emit non-double constants at their uses.
+    if (ins->canEmitAtUses())
+        return emitAtUses(ins);
+
+    return LIRGeneratorShared::visitConstant(ins);
+}
+

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

@@ -28,10 +28,12 @@ class LIRGeneratorX86Shared : public LIRGeneratorShared
     bool visitInterruptCheck(MInterruptCheck *ins);
     bool visitGuardShape(MGuardShape *ins);
     bool visitPowHalf(MPowHalf *ins);
+    bool visitConstant(MConstant *ins);
     bool lowerMulI(MMul *mul, MDefinition *lhs, MDefinition *rhs);
     bool lowerDivI(MDiv *div);
     bool lowerModI(MMod *mod);
     bool lowerUrshD(MUrsh *mir);
+    bool lowerConstantDouble(double d, MInstruction *ins);
 };
 
 } // namespace ion

js/src/ion/x64/CodeGenerator-x64.cpp

@@ -41,14 +41,6 @@ CodeGeneratorX64::ToTempValue(LInstruction *ins, size_t pos)
     return ValueOperand(ToRegister(ins->getTemp(pos)));
 }
 
-bool
-CodeGeneratorX64::visitDouble(LDouble *ins)
-{
-    const LDefinition *out = ins->output();
-    masm.loadConstantDouble(ins->getDouble(), ToFloatRegister(out));
-    return true;
-}
-
 FrameSizeClass
 FrameSizeClass::FromDepth(uint32_t frameDepth)
 {

js/src/ion/x64/CodeGenerator-x64.h

@@ -41,7 +41,6 @@ class CodeGeneratorX64 : public CodeGeneratorX86Shared
     bool visitOsrValue(LOsrValue *value);
     bool visitBox(LBox *box);
     bool visitUnbox(LUnbox *unbox);
-    bool visitDouble(LDouble *ins);
     bool visitLoadSlotV(LLoadSlotV *ins);
     bool visitLoadSlotT(LLoadSlotT *load);
     bool visitStoreSlotT(LStoreSlotT *store);

js/src/ion/x64/LIR-x64.h

@@ -66,23 +66,6 @@ class LUnboxDouble : public LUnboxBase {
     { }
 };
 
-// Constant double.
-class LDouble : public LInstructionHelper<1, 0, 0>
-{
-    double d_;
-
-  public:
-    LIR_HEADER(Double)
-
-    LDouble(double d)
-      : d_(d)
-    { }
-
-    double getDouble() const {
-        return d_;
-    }
-};
-
 } // namespace ion
 } // namespace js
 

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

@@ -36,25 +36,6 @@ LIRGeneratorX64::useBoxFixed(LInstruction *lir, size_t n, MDefinition *mir, Regi
     return true;
 }
 
-bool
-LIRGeneratorX64::lowerConstantDouble(double d, MInstruction *mir)
-{
-    return define(new LDouble(d), mir);
-}
-
-bool
-LIRGeneratorX64::visitConstant(MConstant *ins)
-{
-    if (ins->type() == MIRType_Double)
-        return lowerConstantDouble(ins->value().toDouble(), ins);
-
-    // Emit non-double constants at their uses.
-    if (ins->canEmitAtUses())
-        return emitAtUses(ins);
-
-    return LIRGeneratorShared::visitConstant(ins);
-}
-
 bool
 LIRGeneratorX64::visitBox(MBox *box)
 {

js/src/ion/x64/Lowering-x64.h

@@ -37,10 +37,7 @@ class LIRGeneratorX64 : public LIRGeneratorX86Shared
                      MDefinition *rhs);
     bool lowerForFPU(LMathD *ins, MDefinition *mir, MDefinition *lhs, MDefinition *rhs);
 
-    bool lowerConstantDouble(double d, MInstruction *ins);
-
   public:
-    bool visitConstant(MConstant *ins);
     bool visitBox(MBox *box);
     bool visitUnbox(MUnbox *unbox);
     bool visitReturn(MReturn *ret);

js/src/ion/x86/Assembler-x86.h

@@ -235,10 +235,6 @@ class Assembler : public AssemblerX86Shared
 
     static void TraceJumpRelocations(JSTracer *trc, IonCode *code, CompactBufferReader &reader);
 
-    // The buffer is about to be linked, make sure any constant pools or excess
-    // bookkeeping has been flushed to the instruction stream.
-    void finish() { }
-
     // Copy the assembly code to the given buffer, and perform any pending
     // relocations relying on the target address.
     void executableCopy(uint8_t *buffer);
@@ -405,13 +401,6 @@ class Assembler : public AssemblerX86Shared
     void movsd(const double *dp, const FloatRegister &dest) {
         masm.movsd_mr((const void *)dp, dest.code());
     }
-    void movsd(AbsoluteLabel *label, const FloatRegister &dest) {
-        JS_ASSERT(!label->bound());
-        // Thread the patch list through the unpatched address word in the
-        // instruction stream.
-        masm.movsd_mr(reinterpret_cast<void *>(label->prev()), dest.code());
-        label->setPrev(masm.size());
-    }
 };
 
 // Get a register in which we plan to put a quantity that will be used as an

js/src/ion/x86/CodeGenerator-x86.cpp

@@ -139,45 +139,6 @@ CodeGeneratorX86::visitUnbox(LUnbox *unbox)
     return true;
 }
 
-void
-CodeGeneratorX86::linkAbsoluteLabels()
-{
-    ExecutionMode executionMode = gen->info().executionMode();
-    UnrootedScript script = gen->info().script();
-    IonScript *ionScript = GetIonScript(script, executionMode);
-    IonCode *method = ionScript->method();
-
-    for (size_t i = 0; i < deferredDoubles_.length(); i++) {
-        DeferredDouble *d = deferredDoubles_[i];
-        const Value &v = ionScript->getConstant(d->index());
-        MacroAssembler::Bind(method, d->label(), &v);
-    }
-}
-
-bool
-CodeGeneratorX86::visitDouble(LDouble *ins)
-{
-    const LDefinition *out = ins->getDef(0);
-    const LConstantIndex *cindex = ins->getOperand(0)->toConstantIndex();
-    const Value &v = graph.getConstant(cindex->index());
-
-    union DoublePun {
-        uint64_t u;
-        double d;
-    } dpun;
-    dpun.d = v.toDouble();
-
-    if (masm.maybeInlineDouble(dpun.u, ToFloatRegister(out)))
-        return true;
-
-    DeferredDouble *d = new DeferredDouble(cindex->index());
-    if (!deferredDoubles_.append(d))
-        return false;
-
-    masm.movsd(d->label(), ToFloatRegister(out));
-    return true;
-}
-
 bool
 CodeGeneratorX86::visitLoadSlotV(LLoadSlotV *load)
 {

js/src/ion/x86/CodeGenerator-x86.h

@@ -16,26 +16,7 @@ namespace ion {
 
 class CodeGeneratorX86 : public CodeGeneratorX86Shared
 {
-    class DeferredDouble : public TempObject
-    {
-        AbsoluteLabel label_;
-        uint32_t index_;
-
-      public:
-        DeferredDouble(uint32_t index) : index_(index)
-        { }
-
-        AbsoluteLabel *label() {
-            return &label_;
-        }
-        uint32_t index() const {
-            return index_;
-        }
-    };
-
   private:
-    js::Vector<DeferredDouble *, 0, SystemAllocPolicy> deferredDoubles_;
-
     CodeGeneratorX86 *thisFromCtor() {
         return this;
     }
@@ -48,9 +29,6 @@ class CodeGeneratorX86 : public CodeGeneratorX86Shared
     void storeElementTyped(const LAllocation *value, MIRType valueType, MIRType elementType,
                            const Register &elements, const LAllocation *index);
 
-  protected:
-    void linkAbsoluteLabels();
-
   public:
     CodeGeneratorX86(MIRGenerator *gen, LIRGraph *graph);
 
@@ -60,7 +38,6 @@ class CodeGeneratorX86 : public CodeGeneratorX86Shared
     bool visitUnbox(LUnbox *unbox);
     bool visitValue(LValue *value);
     bool visitOsrValue(LOsrValue *value);
-    bool visitDouble(LDouble *ins);
     bool visitLoadSlotV(LLoadSlotV *load);
     bool visitLoadSlotT(LLoadSlotT *load);
     bool visitStoreSlotT(LStoreSlotT *store);

js/src/ion/x86/LIR-x86.h

@@ -70,17 +70,6 @@ class LUnboxDouble : public LInstructionHelper<1, 2, 0>
     }
 };
 
-// Constant double.
-class LDouble : public LInstructionHelper<1, 1, 0>
-{
-  public:
-    LIR_HEADER(Double);
-
-    LDouble(const LConstantIndex &cindex) {
-        setOperand(0, cindex);
-    }
-};
-
 } // namespace ion
 } // namespace js
 

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

@@ -40,35 +40,6 @@ LIRGeneratorX86::useBoxFixed(LInstruction *lir, size_t n, MDefinition *mir, Regi
     return true;
 }
 
-bool
-LIRGeneratorX86::lowerConstantDouble(double d, MInstruction *mir)
-{
-    uint32_t index;
-    if (!lirGraph_.addConstantToPool(DoubleValue(d), &index))
-        return false;
-
-    LDouble *lir = new LDouble(LConstantIndex::FromIndex(index));
-    return define(lir, mir);
-}
-
-bool
-LIRGeneratorX86::visitConstant(MConstant *ins)
-{
-    if (ins->type() == MIRType_Double) {
-        uint32_t index;
-        if (!lirGraph_.addConstantToPool(ins->value(), &index))
-            return false;
-        LDouble *lir = new LDouble(LConstantIndex::FromIndex(index));
-        return define(lir, ins);
-    }
-
-    // Emit non-double constants at their uses.
-    if (ins->canEmitAtUses())
-        return emitAtUses(ins);
-
-    return LIRGeneratorShared::visitConstant(ins);
-}
-
 bool
 LIRGeneratorX86::visitBox(MBox *box)
 {

js/src/ion/x86/Lowering-x86.h

@@ -39,10 +39,7 @@ class LIRGeneratorX86 : public LIRGeneratorX86Shared
     bool lowerForFPU(LInstructionHelper<1, 2, 0> *ins, MDefinition *mir, MDefinition *lhs,
                      MDefinition *rhs);
 
-    bool lowerConstantDouble(double d, MInstruction *ins);
-
   public:
-    bool visitConstant(MConstant *ins);
     bool visitBox(MBox *box);
     bool visitUnbox(MUnbox *unbox);
     bool visitReturn(MReturn *ret);

js/src/ion/x86/MacroAssembler-x86.cpp

@@ -14,6 +14,54 @@
 using namespace js;
 using namespace js::ion;
 
+void
+MacroAssemblerX86::loadConstantDouble(double d, const FloatRegister &dest)
+{
+    union DoublePun {
+        uint64_t u;
+        double d;
+    } dpun;
+    dpun.d = d;
+    if (maybeInlineDouble(dpun.u, dest))
+        return;
+
+    if (!doubleMap_.initialized()) {
+        enoughMemory_ &= doubleMap_.init();
+        if (!enoughMemory_)
+            return;
+    }
+    size_t doubleIndex;
+    DoubleMap::AddPtr p = doubleMap_.lookupForAdd(d);
+    if (p) {
+        doubleIndex = p->value;
+    } else {
+        doubleIndex = doubles_.length();
+        enoughMemory_ &= doubles_.append(Double(d));
+        enoughMemory_ &= doubleMap_.add(p, d, doubleIndex);
+        if (!enoughMemory_)
+            return;
+    }
+    Double &dbl = doubles_[doubleIndex];
+    masm.movsd_mr(reinterpret_cast<void *>(dbl.uses.prev()), dest.code());
+    dbl.uses.setPrev(masm.size());
+}
+
+void
+MacroAssemblerX86::finish()
+{
+    if (doubles_.empty())
+        return;
+
+    masm.align(sizeof(double));
+    for (size_t i = 0; i < doubles_.length(); i++) {
+        CodeLabel cl(doubles_[i].uses);
+        writeDoubleConstant(doubles_[i].value, cl.src());
+        enoughMemory_ &= addCodeLabel(cl);
+        if (!enoughMemory_)
+            return;
+    }
+}
+
 void
 MacroAssemblerX86::setupABICall(uint32_t args)
 {

js/src/ion/x86/MacroAssembler-x86.h

@@ -28,6 +28,16 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared
     bool dynamicAlignment_;
     bool enoughMemory_;
 
+    struct Double {
+        double value;
+        AbsoluteLabel uses;
+        Double(double value) : value(value) {}
+    };
+    Vector<Double, 0, SystemAllocPolicy> doubles_;
+
+    typedef HashMap<double, size_t, DefaultHasher<double>, SystemAllocPolicy> DoubleMap;
+    DoubleMap doubleMap_;
+
   protected:
     MoveResolver moveResolver_;
 
@@ -62,6 +72,10 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared
     {
     }
 
+    // The buffer is about to be linked, make sure any constant pools or excess
+    // bookkeeping has been flushed to the instruction stream.
+    void finish();
+
     bool oom() const {
         return MacroAssemblerX86Shared::oom() || !enoughMemory_;
     }
@@ -639,6 +653,7 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared
         cvtsi2sd(operand.payloadReg(), dest);
     }
 
+    void loadConstantDouble(double d, const FloatRegister &dest);
     void loadStaticDouble(const double *dp, const FloatRegister &dest) {
         movsd(dp, dest);
     }