Bug 835003 - simplify generation of MTableSwitch jump tables (r=h4writer)

--HG--
extra : rebase_source : dc652a92a43e9af389f925539cfeebee70b313ea

js/src/assembler/assembler/X86Assembler.h

@@ -2442,6 +2442,11 @@ public:
         return label();
     }
 
+    void jumpTablePointer(uintptr_t ptr)
+    {
+        m_formatter.jumpTablePointer(ptr);
+    }
+
     // Linking & patching:
     //
     // 'link' and 'patch' methods are for use on unprotected code - such as the code
@@ -2449,7 +2454,7 @@ public:
     // code has been finalized it is (platform support permitting) within a non-
     // writable region of memory; to modify the code in an execute-only execuable
     // pool the 'repatch' and 'relink' methods should be used.
-    
+
     // Like Lua's emitter, we thread jump lists through the unpatched target
     // field, which will get fixed up when the label (which has a pointer to
     // the head of the jump list) is bound.
@@ -2989,6 +2994,18 @@ private:
             return JmpSrc(m_buffer.size());
         }
 
+        // Data:
+
+        void jumpTablePointer(uintptr_t ptr)
+        {
+            m_buffer.ensureSpace(sizeof(uintptr_t));
+#if WTF_CPU_X86_64
+            m_buffer.putInt64Unchecked(ptr);
+#else
+            m_buffer.putIntUnchecked(ptr);
+#endif
+        }
+
         // Administrative methods:
 
         size_t size() const { return m_buffer.size(); }

js/src/ion/Ion.cpp

@@ -350,9 +350,6 @@ IonCode::copyFrom(MacroAssembler &masm)
     insnSize_ = masm.instructionsSize();
     masm.executableCopy(code_);
 
-    dataSize_ = masm.dataSize();
-    masm.processDeferredData(this, code_ + dataOffset());
-
     jumpRelocTableBytes_ = masm.jumpRelocationTableBytes();
     masm.copyJumpRelocationTable(code_ + jumpRelocTableOffset());
 

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

@@ -744,36 +744,38 @@ Assembler::trace(JSTracer *trc)
         ::TraceDataRelocations(trc, &m_buffer, &tmpDataRelocations_);
 }
 
-void
-Assembler::processDeferredData(IonCode *code, uint8_t *data)
-{
-    // Deferred Data is something like Pools for X86.
-    // Since ARM has competent pools, this isn't actually used.
-    // Except of course, for SwitchTables.  Those are really shoehorned
-    // in and don't take up any space in the instruction stream, so dataSize()
-    // is still 0.
-    // NOTE: this means arm will in fact break if the for loop is removed.
-    JS_ASSERT(dataSize() == 0);
-
-    for (size_t i = 0; i < data_.length(); i++) {
-        DeferredData *deferred = data_[i];
-        //Bind(code, deferred->label(), data + deferred->offset());
-        deferred->copy(code, data + deferred->offset());
-    }
-
-}
-
-// As far as I can tell, CodeLabels were supposed to be used in switch tables
-// and they aren't used there, nor anywhere else.
 void
 Assembler::processCodeLabels(IonCode *code)
 {
     for (size_t i = 0; i < codeLabels_.length(); i++) {
-        //Bind(code, label->dest(), code->raw() + label->src()->offset());
-        JS_NOT_REACHED("dead code?");
+        CodeLabel *label = codeLabels_[i];
+        Bind(code, label->dest(), code->raw() + actualOffset(label->src()->offset()));
     }
 }
 
+void
+Assembler::writeCodePointer(AbsoluteLabel *absoluteLabel) {
+    JS_ASSERT(!absoluteLabel->bound());
+    BufferOffset off = writeInst(-1);
+
+    // x86/x64 makes general use of AbsoluteLabel and weaves a linked list of
+    // uses of an AbsoluteLabel through the assembly. ARM only uses labels
+    // for the case statements of switch jump tables. Thus, for simplicity, we
+    // simply treat the AbsoluteLabel as a label and bind it to the offset of
+    // the jump table entry that needs to be patched.
+    LabelBase *label = absoluteLabel;
+    label->bind(off.getOffset());
+}
+
+void
+Assembler::Bind(IonCode *code, AbsoluteLabel *label, const void *address)
+{
+    // See writeCodePointer comment.
+    uint8_t *raw = code->raw();
+    uint32_t off = actualOffset(label->offset());
+    *reinterpret_cast<const void **>(raw + off) = address;
+}
+
 Assembler::Condition
 Assembler::InvertCondition(Condition cond)
 {
@@ -1141,16 +1143,6 @@ Assembler::oom() const
         preBarriers_.oom();
 }
 
-bool
-Assembler::addDeferredData(DeferredData *data, size_t bytes)
-{
-    data->setOffset(dataBytesNeeded_);
-    dataBytesNeeded_ += bytes;
-    if (dataBytesNeeded_ >= MAX_BUFFER_SIZE)
-        return false;
-    return data_.append(data);
-}
-
 bool
 Assembler::addCodeLabel(CodeLabel *label)
 {
@@ -1185,15 +1177,9 @@ Assembler::preBarrierTableBytes() const
 
 // Size of the data table, in bytes.
 size_t
-Assembler::dataSize() const
-{
-    return dataBytesNeeded_;
-}
-size_t
 Assembler::bytesNeeded() const
 {
     return size() +
-        dataSize() +
         jumpRelocationTableBytes() +
         dataRelocationTableBytes() +
         preBarrierTableBytes();
@@ -2167,19 +2153,6 @@ Assembler::leaveNoPool()
     m_buffer.leaveNoPool();
 }
 
-BufferOffset
-Assembler::as_jumpPool(uint32_t numCases)
-{
-    if (numCases == 0)
-        return BufferOffset();
-
-    BufferOffset ret = writeInst(-1);
-    for (uint32_t i = 1; i < numCases; i++)
-        writeInst(-1);
-
-    return ret;
-}
-
 ptrdiff_t
 Assembler::getBranchOffset(const Instruction *i_)
 {

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

@@ -1172,7 +1172,6 @@ class Assembler
     // TODO: this should actually be a pool-like object
     //       It is currently a big hack, and probably shouldn't exist
     class JumpPool;
-    js::Vector<DeferredData *, 0, SystemAllocPolicy> data_;
     js::Vector<CodeLabel *, 0, SystemAllocPolicy> codeLabels_;
     js::Vector<RelativePatch, 8, SystemAllocPolicy> jumps_;
     js::Vector<JumpPool *, 0, SystemAllocPolicy> jumpPools_;
@@ -1190,7 +1189,6 @@ class Assembler
     CompactBufferWriter dataRelocations_;
     CompactBufferWriter relocations_;
     CompactBufferWriter preBarriers_;
-    size_t dataBytesNeeded_;
 
     bool enoughMemory_;
 
@@ -1213,8 +1211,7 @@ class Assembler
 
   public:
     Assembler()
-      : dataBytesNeeded_(0),
-        enoughMemory_(true),
+      : enoughMemory_(true),
         m_buffer(4, 4, 0, &pools_[0], 8),
         int32Pool(m_buffer.getPool(1)),
         doublePool(m_buffer.getPool(0)),
@@ -1288,14 +1285,11 @@ class Assembler
   public:
     void finish();
     void executableCopy(void *buffer);
-    void processDeferredData(IonCode *code, uint8_t *data);
     void processCodeLabels(IonCode *code);
     void copyJumpRelocationTable(uint8_t *dest);
     void copyDataRelocationTable(uint8_t *dest);
     void copyPreBarrierTable(uint8_t *dest);
 
-    bool addDeferredData(DeferredData *data, size_t bytes);
-
     bool addCodeLabel(CodeLabel *label);
 
     // Size of the instruction stream, in bytes.
@@ -1306,7 +1300,6 @@ class Assembler
     size_t preBarrierTableBytes() const;
 
     // Size of the data table, in bytes.
-    size_t dataSize() const;
     size_t bytesNeeded() const;
 
     // Write a blob of binary into the instruction stream *OR*
@@ -1320,9 +1313,7 @@ class Assembler
     static void writeInstStatic(uint32_t x, uint32_t *dest);
 
   public:
-    // resreve enough space in the instruction stream for a jumpPool.
-    // return the reserved space.
-    BufferOffset as_jumpPool(uint32_t size);
+    void writeCodePointer(AbsoluteLabel *label);
 
     BufferOffset align(int alignment);
     BufferOffset as_nop();
@@ -1540,6 +1531,7 @@ class Assembler
     void retarget(Label *label, Label *target);
     // I'm going to pretend this doesn't exist for now.
     void retarget(Label *label, void *target, Relocation::Kind reloc);
+    void Bind(IonCode *code, AbsoluteLabel *label, const void *address);
 
     void call(Label *label);
     void call(void *target);

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

@@ -26,32 +26,6 @@
 using namespace js;
 using namespace js::ion;
 
-class DeferredJumpTable : public DeferredData
-{
-    MTableSwitch *mswitch;
-    BufferOffset off;
-    MacroAssembler *masm;
-  public:
-    DeferredJumpTable(MTableSwitch *mswitch, BufferOffset off_, MacroAssembler *masm_)
-      : mswitch(mswitch), off(off_), masm(masm_)
-    { }
-
-    void copy(IonCode *code, uint8_t *ignore__) const {
-        void **jumpData = (void **)(((char*)code->raw()) + masm->actualOffset(off).getOffset());
-        int numCases =  mswitch->numCases();
-        // For every case write the pointer to the start in the table
-        for (int j = 0; j < numCases; j++) {
-            LBlock *caseblock = mswitch->getCase(numCases - 1 - j)->lir();
-            Label *caseheader = caseblock->label();
-
-            uint32_t offset = caseheader->offset();
-            *jumpData = (void *)(code->raw() + masm->actualOffset(offset));
-            jumpData++;
-        }
-    }
-};
-
-
 // shared
 CodeGeneratorARM::CodeGeneratorARM(MIRGenerator *gen, LIRGraph *graph)
   : CodeGeneratorShared(gen, graph),
@@ -898,6 +872,54 @@ CodeGeneratorARM::visitMoveGroup(LMoveGroup *group)
     return true;
 }
 
+class js::ion::OutOfLineTableSwitch : public OutOfLineCodeBase<CodeGeneratorARM>
+{
+    MTableSwitch *mir_;
+    Vector<CodeLabel*, 8, IonAllocPolicy> codeLabels_;
+
+    bool accept(CodeGeneratorARM *codegen) {
+        return codegen->visitOutOfLineTableSwitch(this);
+    }
+
+  public:
+    OutOfLineTableSwitch(MTableSwitch *mir)
+      : mir_(mir)
+    {}
+
+    MTableSwitch *mir() const {
+        return mir_;
+    }
+
+    bool addCodeLabel(CodeLabel *label) {
+        return codeLabels_.append(label);
+    }
+    CodeLabel *codeLabel(unsigned i) {
+        return codeLabels_[i];
+    }
+};
+
+bool
+CodeGeneratorARM::visitOutOfLineTableSwitch(OutOfLineTableSwitch *ool)
+{
+    MTableSwitch *mir = ool->mir();
+
+    int numCases = mir->numCases();
+    for (size_t i = 0; i < numCases; i++) {
+        LBlock *caseblock = mir->getCase(numCases - 1 - i)->lir();
+        Label *caseheader = caseblock->label();
+        uint32_t caseoffset = caseheader->offset();
+
+        // The entries of the jump table need to be absolute addresses and thus
+        // must be patched after codegen is finished.
+        CodeLabel *cl = ool->codeLabel(i);
+        cl->src()->bind(caseoffset);
+        if (!masm.addCodeLabel(cl))
+            return false;
+    }
+
+    return true;
+}
+
 bool
 CodeGeneratorARM::emitTableSwitchDispatch(MTableSwitch *mir, const Register &index,
                                           const Register &base)
@@ -937,11 +959,20 @@ CodeGeneratorARM::emitTableSwitchDispatch(MTableSwitch *mir, const Register &ind
     AutoForbidPools afp(&masm);
     masm.ma_ldr(DTRAddr(pc, DtrRegImmShift(index, LSL, 2)), pc, Offset, Assembler::Unsigned);
     masm.ma_b(defaultcase);
-    DeferredJumpTable *d = new DeferredJumpTable(mir, masm.nextOffset(), &masm);
-    masm.as_jumpPool(cases);
 
-    if (!masm.addDeferredData(d, 0))
+    // To fill in the CodeLabels for the case entries, we need to first
+    // generate the case entries (we don't yet know their offsets in the
+    // instruction stream).
+    OutOfLineTableSwitch *ool = new OutOfLineTableSwitch(mir);
+    for (int32_t i = 0; i < cases; i++) {
+        CodeLabel *cl = new CodeLabel();
+        masm.writeCodePointer(cl->dest());
+        if (!ool->addCodeLabel(cl))
+            return false;
+    }
+    if (!addOutOfLineCode(ool))
         return false;
+
     return true;
 }
 

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

@@ -15,6 +15,7 @@ namespace js {
 namespace ion {
 
 class OutOfLineBailout;
+class OutOfLineTableSwitch;
 
 class CodeGeneratorARM : public CodeGeneratorShared
 {
@@ -107,6 +108,7 @@ class CodeGeneratorARM : public CodeGeneratorShared
 
     // Out of line visitors.
     bool visitOutOfLineBailout(OutOfLineBailout *ool);
+    bool visitOutOfLineTableSwitch(OutOfLineTableSwitch *ool);
 
   protected:
     ValueOperand ToValue(LInstruction *ins, size_t pos);

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

@@ -339,36 +339,6 @@ class CodeLabel : public TempObject
     }
 };
 
-// Deferred data is a chunk of data that cannot be computed until an assembly
-// buffer has been fully allocated, but should be attached to the final code
-// stream. At the time deferred data is emitted, the code buffer has been
-// completely allocated.
-class DeferredData : public TempObject
-{
-    // Label, which before linking is unbound.
-    AbsoluteLabel label_;
-
-    // Offset from the start of the data section.
-    int32_t offset_;
-
-  public:
-    DeferredData() : offset_(-1)
-    { }
-    int32_t offset() const {
-        JS_ASSERT(offset_ > -1);
-        return offset_;
-    }
-    void setOffset(int32_t offset) {
-        offset_ = offset;
-    }
-    AbsoluteLabel *label() {
-        return &label_;
-    }
-
-    // Must copy pending data into the buffer.
-    virtual void copy(IonCode *code, uint8_t *buffer) const = 0;
-};
-
 // Location of a jump or label in a generated IonCode block, relative to the
 // start of the block.
 

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

@@ -90,16 +90,6 @@ AssemblerX86Shared::executableCopy(void *buffer)
     masm.executableCopy(buffer);
 }
 
-void
-AssemblerX86Shared::processDeferredData(IonCode *code, uint8_t *data)
-{
-    for (size_t i = 0; i < data_.length(); i++) {
-        DeferredData *deferred = data_[i];
-        Bind(code, deferred->label(), data + deferred->offset());
-        deferred->copy(code, data + deferred->offset());
-    }
-}
-
 void
 AssemblerX86Shared::processCodeLabels(IonCode *code)
 {

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

@@ -28,13 +28,11 @@ class AssemblerX86Shared
         { }
     };
 
-    js::Vector<DeferredData *, 0, SystemAllocPolicy> data_;
     js::Vector<CodeLabel *, 0, SystemAllocPolicy> codeLabels_;
     js::Vector<RelativePatch, 8, SystemAllocPolicy> jumps_;
     CompactBufferWriter jumpRelocations_;
     CompactBufferWriter dataRelocations_;
     CompactBufferWriter preBarriers_;
-    size_t dataBytesNeeded_;
     bool enoughMemory_;
 
     void writeDataRelocation(const Value &val) {
@@ -139,8 +137,7 @@ class AssemblerX86Shared
     }
 
     AssemblerX86Shared()
-      : dataBytesNeeded_(0),
-        enoughMemory_(true)
+      : enoughMemory_(true)
     {
     }
 
@@ -168,20 +165,11 @@ class AssemblerX86Shared
     }
 
     void executableCopy(void *buffer);
-    void processDeferredData(IonCode *code, uint8_t *data);
     void processCodeLabels(IonCode *code);
     void copyJumpRelocationTable(uint8_t *dest);
     void copyDataRelocationTable(uint8_t *dest);
     void copyPreBarrierTable(uint8_t *dest);
 
-    bool addDeferredData(DeferredData *data, size_t bytes) {
-        data->setOffset(dataBytesNeeded_);
-        dataBytesNeeded_ += bytes;
-        if (dataBytesNeeded_ >= MAX_BUFFER_SIZE)
-            return false;
-        return data_.append(data);
-    }
-    
     bool addCodeLabel(CodeLabel *label) {
         return codeLabels_.append(label);
     }
@@ -201,12 +189,8 @@ class AssemblerX86Shared
         return preBarriers_.length();
     }
     // Size of the data table, in bytes.
-    size_t dataSize() const {
-        return dataBytesNeeded_;
-    }
     size_t bytesNeeded() const {
         return size() +
-               dataSize() +
                jumpRelocationTableBytes() +
                dataRelocationTableBytes() +
                preBarrierTableBytes();
@@ -216,6 +200,13 @@ class AssemblerX86Shared
     void align(int alignment) {
         masm.align(alignment);
     }
+    void writeCodePointer(AbsoluteLabel *label) {
+        JS_ASSERT(!label->bound());
+        // Thread the patch list through the unpatched address word in the
+        // instruction stream.
+        masm.jumpTablePointer(label->prev());
+        label->setPrev(masm.size());
+    }
     void movl(const Imm32 &imm32, const Register &dest) {
         masm.movl_i32r(imm32.value, dest.code());
     }

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

@@ -22,30 +22,6 @@ using namespace js::ion;
 namespace js {
 namespace ion {
 
-class DeferredJumpTable : public DeferredData
-{
-    MTableSwitch *mswitch;
-
-  public:
-    DeferredJumpTable(MTableSwitch *mswitch)
-      : mswitch(mswitch)
-    { }
-
-    void copy(IonCode *code, uint8_t *buffer) const {
-        void **jumpData = (void **)buffer;
-
-        // For every case write the pointer to the start in the table
-        for (size_t j = 0; j < mswitch->numCases(); j++) {
-            LBlock *caseblock = mswitch->getCase(j)->lir();
-            Label *caseheader = caseblock->label();
-
-            uint32_t offset = caseheader->offset();
-            *jumpData = (void *)(code->raw() + offset);
-            jumpData++;
-        }
-    }
-};
-
 CodeGeneratorX86Shared::CodeGeneratorX86Shared(MIRGenerator *gen, LIRGraph *graph)
   : CodeGeneratorShared(gen, graph),
     deoptLabel_(NULL)
@@ -1026,6 +1002,56 @@ CodeGeneratorX86Shared::visitMoveGroup(LMoveGroup *group)
     return true;
 }
 
+class OutOfLineTableSwitch : public OutOfLineCodeBase<CodeGeneratorX86Shared>
+{
+    MTableSwitch *mir_;
+    CodeLabel *jumpLabel_;
+
+    bool accept(CodeGeneratorX86Shared *codegen) {
+        return codegen->visitOutOfLineTableSwitch(this);
+    }
+
+  public:
+    OutOfLineTableSwitch(MTableSwitch *mir)
+      : mir_(mir), jumpLabel_(new CodeLabel)
+    {}
+
+    MTableSwitch *mir() const {
+        return mir_;
+    }
+
+    CodeLabel *jumpLabel() const {
+        return jumpLabel_;
+    }
+};
+
+bool
+CodeGeneratorX86Shared::visitOutOfLineTableSwitch(OutOfLineTableSwitch *ool)
+{
+    MTableSwitch *mir = ool->mir();
+
+    masm.align(sizeof(void*));
+    masm.bind(ool->jumpLabel()->src());
+    if (!masm.addCodeLabel(ool->jumpLabel()))
+        return false;
+
+    for (size_t i = 0; i < mir->numCases(); i++) {
+        LBlock *caseblock = mir->getCase(i)->lir();
+        Label *caseheader = caseblock->label();
+        uint32_t caseoffset = caseheader->offset();
+
+        // The entries of the jump table need to be absolute addresses and thus
+        // must be patched after codegen is finished.
+        CodeLabel *cl = new CodeLabel();
+        masm.writeCodePointer(cl->dest());
+        cl->src()->bind(caseoffset);
+        if (!masm.addCodeLabel(cl))
+            return false;
+    }
+
+    return true;
+}
+
 bool
 CodeGeneratorX86Shared::emitTableSwitchDispatch(MTableSwitch *mir, const Register &index,
                                                 const Register &base)
@@ -1041,13 +1067,15 @@ CodeGeneratorX86Shared::emitTableSwitchDispatch(MTableSwitch *mir, const Registe
     masm.cmpl(index, Imm32(cases));
     masm.j(AssemblerX86Shared::AboveOrEqual, defaultcase);
 
-    // Create a JumpTable that during linking will get written.
-    DeferredJumpTable *d = new DeferredJumpTable(mir);
-    if (!masm.addDeferredData(d, (1 << ScalePointer) * cases))
+    // To fill in the CodeLabels for the case entries, we need to first
+    // generate the case entries (we don't yet know their offsets in the
+    // instruction stream).
+    OutOfLineTableSwitch *ool = new OutOfLineTableSwitch(mir);
+    if (!addOutOfLineCode(ool))
         return false;
 
     // Compute the position where a pointer to the right case stands.
-    masm.mov(d->label(), base);
+    masm.mov(ool->jumpLabel()->dest(), base);
     Operand pointer = Operand(base, index, ScalePointer);
 
     // Jump to the right case

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

@@ -17,6 +17,7 @@ class OutOfLineBailout;
 class OutOfLineUndoALUOperation;
 class MulNegativeZeroCheck;
 class OutOfLineTruncate;
+class OutOfLineTableSwitch;
 
 class CodeGeneratorX86Shared : public CodeGeneratorShared
 {
@@ -118,6 +119,7 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
     bool visitOutOfLineUndoALUOperation(OutOfLineUndoALUOperation *ool);
     bool visitMulNegativeZeroCheck(MulNegativeZeroCheck *ool);
     bool visitOutOfLineTruncate(OutOfLineTruncate *ool);
+    bool visitOutOfLineTableSwitch(OutOfLineTableSwitch *ool);
     bool generateInvalidateEpilogue();
 };