Bug 1230005: Factor out relocation style decision; r=jolesen
authorBenjamin Bouvier <benj@benj.me>
Wed, 23 Dec 2015 23:06:15 +0100
changeset 317588 9c4f5c1196b8b516a772f96463979b5b423eb913
parent 317568 0eed3c26935ae436ddc8ed094e8cff4c5a18b3b4
child 317589 b3ede24e43775e2416c1bb5d4f4cfd17b79cb2f1
child 317591 b9bb3765e712e07dcd35a63f7aec9fd4e61fdd8a
push id8714
push userbenj@benj.me
push dateThu, 24 Dec 2015 10:23:44 +0000
reviewersjolesen
bugs1230005
milestone46.0a1
Bug 1230005: Factor out relocation style decision; r=jolesen
js/src/jit/arm/Assembler-arm.cpp
js/src/jit/arm/Assembler-arm.h
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm/MacroAssembler-arm.h
js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
--- a/js/src/jit/arm/Assembler-arm.cpp
+++ b/js/src/jit/arm/Assembler-arm.cpp
@@ -207,34 +207,34 @@ js::jit::maybeRD(Register r)
     if (r == InvalidReg)
         return 0;
 
     MOZ_ASSERT((r.code() & ~0xf) == 0);
     return r.code() << 12;
 }
 
 Register
-js::jit::toRD(Instruction& i)
+js::jit::toRD(Instruction i)
 {
     return Register::FromCode((i.encode() >> 12) & 0xf);
 }
 Register
-js::jit::toR(Instruction& i)
+js::jit::toR(Instruction i)
 {
     return Register::FromCode(i.encode() & 0xf);
 }
 
 Register
-js::jit::toRM(Instruction& i)
+js::jit::toRM(Instruction i)
 {
     return Register::FromCode((i.encode() >> 8) & 0xf);
 }
 
 Register
-js::jit::toRN(Instruction& i)
+js::jit::toRN(Instruction i)
 {
     return Register::FromCode((i.encode() >> 16) & 0xf);
 }
 
 uint32_t
 js::jit::VD(VFPRegister vr)
 {
     if (vr.isMissing())
@@ -2991,17 +2991,16 @@ void
 Assembler::RetargetFarBranch(Instruction* i, uint8_t** slot, uint8_t* dest, Condition cond)
 {
     int32_t offset = reinterpret_cast<uint8_t*>(slot) - reinterpret_cast<uint8_t*>(i);
     if (!i->is<InstLDR>()) {
         new (i) InstLDR(Offset, pc, DTRAddr(pc, DtrOffImm(offset - 8)), cond);
         AutoFlushICache::flush(uintptr_t(i), 4);
     }
     *slot = dest;
-
 }
 
 struct PoolHeader : Instruction
 {
     struct Header
     {
         // The size should take into account the pool header.
         // The size is in units of Instruction (4 bytes), not byte.
@@ -3087,36 +3086,39 @@ Assembler::PatchWrite_NearCall(CodeLocat
     // Ensure everyone sees the code that was just written into memory.
     AutoFlushICache::flush(uintptr_t(inst), 4);
 }
 
 void
 Assembler::PatchDataWithValueCheck(CodeLocationLabel label, PatchedImmPtr newValue,
                                    PatchedImmPtr expectedValue)
 {
-    Instruction* ptr = (Instruction*) label.raw();
+    Instruction* ptr = reinterpret_cast<Instruction*>(label.raw());
     InstructionIterator iter(ptr);
     Register dest;
     Assembler::RelocStyle rs;
+
     DebugOnly<const uint32_t*> val = GetPtr32Target(&iter, &dest, &rs);
-    MOZ_ASSERT((uint32_t)(const uint32_t*)val == uint32_t(expectedValue.value));
+    MOZ_ASSERT(uint32_t((const uint32_t*)val) == uint32_t(expectedValue.value));
 
     MacroAssembler::ma_mov_patch(Imm32(int32_t(newValue.value)), dest, Always, rs, ptr);
 
     // L_LDR won't cause any instructions to be updated.
     if (rs != L_LDR) {
         AutoFlushICache::flush(uintptr_t(ptr), 4);
         AutoFlushICache::flush(uintptr_t(ptr->next()), 4);
     }
 }
 
 void
 Assembler::PatchDataWithValueCheck(CodeLocationLabel label, ImmPtr newValue, ImmPtr expectedValue)
 {
-    PatchDataWithValueCheck(label, PatchedImmPtr(newValue.value), PatchedImmPtr(expectedValue.value));
+    PatchDataWithValueCheck(label,
+                            PatchedImmPtr(newValue.value),
+                            PatchedImmPtr(expectedValue.value));
 }
 
 // This just stomps over memory with 32 bits of raw data. Its purpose is to
 // overwrite the call of JITed code with 32 bits worth of an offset. This will
 // is only meant to function on code that has been invalidated, so it should be
 // totally safe. Since that instruction will never be executed again, a ICache
 // flush should not be necessary
 void
--- a/js/src/jit/arm/Assembler-arm.h
+++ b/js/src/jit/arm/Assembler-arm.h
@@ -218,20 +218,20 @@ uint32_t RS(Register r);
 uint32_t RD(Register r);
 uint32_t RT(Register r);
 uint32_t RN(Register r);
 
 uint32_t maybeRD(Register r);
 uint32_t maybeRT(Register r);
 uint32_t maybeRN(Register r);
 
-Register toRN (Instruction& i);
-Register toRM (Instruction& i);
-Register toRD (Instruction& i);
-Register toR (Instruction& i);
+Register toRN(Instruction i);
+Register toRM(Instruction i);
+Register toRD(Instruction i);
+Register toR(Instruction i);
 
 class VFPRegister;
 uint32_t VD(VFPRegister vr);
 uint32_t VN(VFPRegister vr);
 uint32_t VM(VFPRegister vr);
 
 // For being passed into the generic vfp instruction generator when there is an
 // instruction that only takes two registers.
@@ -1233,17 +1233,17 @@ class Assembler : public AssemblerShared
     }
 
   protected:
     // Shim around AssemblerBufferWithConstantPools::allocEntry.
     BufferOffset allocEntry(size_t numInst, unsigned numPoolEntries,
                             uint8_t* inst, uint8_t* data, ARMBuffer::PoolEntry* pe = nullptr,
                             bool markAsBranch = false, bool loadToPC = false);
 
-    Instruction* editSrc (BufferOffset bo) {
+    Instruction* editSrc(BufferOffset bo) {
         return m_buffer.getInst(bo);
     }
 
 #ifdef JS_DISASM_ARM
     static void spewInst(Instruction* i);
     void spew(Instruction* i);
     void spewBranch(Instruction* i, Label* target);
     void spewData(BufferOffset addr, size_t numInstr, bool loadToPC);
@@ -1928,21 +1928,25 @@ class Assembler : public AssemblerShared
 class Instruction
 {
     uint32_t data;
 
   protected:
     // This is not for defaulting to always, this is for instructions that
     // cannot be made conditional, and have the usually invalid 4b1111 cond
     // field.
-    Instruction (uint32_t data_, bool fake = false) : data(data_ | 0xf0000000) {
+    explicit Instruction(uint32_t data_, bool fake = false)
+      : data(data_ | 0xf0000000)
+    {
         MOZ_ASSERT(fake || ((data_ & 0xf0000000) == 0));
     }
     // Standard constructor.
-    Instruction (uint32_t data_, Assembler::Condition c) : data(data_ | (uint32_t) c) {
+    Instruction(uint32_t data_, Assembler::Condition c)
+      : data(data_ | (uint32_t) c)
+    {
         MOZ_ASSERT((data_ & 0xf0000000) == 0);
     }
     // You should never create an instruction directly. You should create a more
     // specific instruction which will eventually call one of these constructors
     // for you.
   public:
     uint32_t encode() const {
         return data;
@@ -1950,17 +1954,17 @@ class Instruction
     // Check if this instruction is really a particular case.
     template <class C>
     bool is() const { return C::IsTHIS(*this); }
 
     // Safely get a more specific variant of this pointer.
     template <class C>
     C* as() const { return C::AsTHIS(*this); }
 
-    const Instruction & operator=(const Instruction& src) {
+    const Instruction& operator=(Instruction src) {
         data = src.data;
         return *this;
     }
     // Since almost all instructions have condition codes, the condition code
     // extractor resides in the base class.
     Assembler::Condition extractCond() {
         MOZ_ASSERT(data >> 28 != 0xf, "The instruction does not have condition code");
         return (Assembler::Condition)(data & 0xf0000000);
@@ -2002,17 +2006,17 @@ class InstDTR : public Instruction
 
 };
 JS_STATIC_ASSERT(sizeof(InstDTR) == sizeof(Instruction));
 
 class InstLDR : public InstDTR
 {
   public:
     InstLDR(Index mode, Register rt, DTRAddr addr, Assembler::Condition c)
-        : InstDTR(IsLoad, IsWord, mode, rt, addr, c)
+      : InstDTR(IsLoad, IsWord, mode, rt, addr, c)
     { }
 
     static bool IsTHIS(const Instruction& i);
     static InstLDR* AsTHIS(const Instruction& i);
 
 };
 JS_STATIC_ASSERT(sizeof(InstDTR) == sizeof(InstLDR));
 
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -419,36 +419,31 @@ MacroAssemblerARM::ma_alu(Register src1,
 
 void
 MacroAssemblerARM::ma_nop()
 {
     as_nop();
 }
 
 void
-MacroAssemblerARM::ma_movPatchable(Imm32 imm_, Register dest, Assembler::Condition c,
-                                   RelocStyle rs)
+MacroAssemblerARM::ma_movPatchable(Imm32 imm_, Register dest, Assembler::Condition c)
 {
     int32_t imm = imm_.value;
-    switch(rs) {
-      case L_MOVWT:
+    if (HasMOVWT()) {
         as_movw(dest, Imm16(imm & 0xffff), c);
         as_movt(dest, Imm16(imm >> 16 & 0xffff), c);
-        break;
-      case L_LDR:
+    } else {
         as_Imm32Pool(dest, imm, c);
-        break;
     }
 }
 
 void
-MacroAssemblerARM::ma_movPatchable(ImmPtr imm, Register dest, Assembler::Condition c,
-                                   RelocStyle rs)
-{
-    ma_movPatchable(Imm32(int32_t(imm.value)), dest, c, rs);
+MacroAssemblerARM::ma_movPatchable(ImmPtr imm, Register dest, Assembler::Condition c)
+{
+    ma_movPatchable(Imm32(int32_t(imm.value)), dest, c);
 }
 
 /* static */ void
 MacroAssemblerARM::ma_mov_patch(Imm32 imm_, Register dest, Assembler::Condition c,
                                 RelocStyle rs, Instruction* i)
 {
     MOZ_ASSERT(i);
     int32_t imm = imm_.value;
@@ -498,23 +493,17 @@ MacroAssemblerARM::ma_mov(ImmWord imm, R
 }
 
 void
 MacroAssemblerARM::ma_mov(ImmGCPtr ptr, Register dest)
 {
     // As opposed to x86/x64 version, the data relocation has to be executed
     // before to recover the pointer, and not after.
     writeDataRelocation(ptr);
-    RelocStyle rs;
-    if (HasMOVWT())
-        rs = L_MOVWT;
-    else
-        rs = L_LDR;
-
-    ma_movPatchable(Imm32(uintptr_t(ptr.value)), dest, Always, rs);
+    ma_movPatchable(Imm32(uintptr_t(ptr.value)), dest, Always);
 }
 
 // Shifts (just a move with a shifting op2)
 void
 MacroAssemblerARM::ma_lsl(Imm32 shift, Register src, Register dst)
 {
     as_mov(dst, lsl(src, shift.value));
 }
@@ -1934,24 +1923,18 @@ void
 MacroAssemblerARMCompat::movePtr(ImmPtr imm, Register dest)
 {
     movePtr(ImmWord(uintptr_t(imm.value)), dest);
 }
 
 void
 MacroAssemblerARMCompat::movePtr(wasm::SymbolicAddress imm, Register dest)
 {
-    RelocStyle rs;
-    if (HasMOVWT())
-        rs = L_MOVWT;
-    else
-        rs = L_LDR;
-
     append(AsmJSAbsoluteLink(CodeOffset(currentOffset()), imm));
-    ma_movPatchable(Imm32(-1), dest, Always, rs);
+    ma_movPatchable(Imm32(-1), dest, Always);
 }
 
 void
 MacroAssemblerARMCompat::load8ZeroExtend(const Address& address, Register dest)
 {
     ma_dataTransferN(IsLoad, 8, false, address.base, Imm32(address.offset), dest);
 }
 
@@ -3570,23 +3553,17 @@ MacroAssemblerARMCompat::storeTypeTag(Im
     ma_mov(tag, scratch);
     ma_str(scratch, DTRAddr(base, DtrRegImmShift(index, LSL, shift)));
     ma_sub(base, Imm32(NUNBOX32_TYPE_OFFSET + dest.offset), base);
 }
 
 void
 MacroAssemblerARM::ma_call(ImmPtr dest)
 {
-    RelocStyle rs;
-    if (HasMOVWT())
-        rs = L_MOVWT;
-    else
-        rs = L_LDR;
-
-    ma_movPatchable(dest, CallReg, Always, rs);
+    ma_movPatchable(dest, CallReg, Always);
     as_blx(CallReg);
 }
 
 void
 MacroAssemblerARMCompat::breakpoint()
 {
     as_bkpt();
 }
@@ -3975,17 +3952,17 @@ MacroAssemblerARMCompat::toggledJump(Lab
 }
 
 CodeOffset
 MacroAssemblerARMCompat::toggledCall(JitCode* target, bool enabled)
 {
     BufferOffset bo = nextOffset();
     addPendingJump(bo, ImmPtr(target->raw()), Relocation::JITCODE);
     ScratchRegisterScope scratch(asMasm());
-    ma_movPatchable(ImmPtr(target->raw()), scratch, Always, HasMOVWT() ? L_MOVWT : L_LDR);
+    ma_movPatchable(ImmPtr(target->raw()), scratch, Always);
     if (enabled)
         ma_blx(scratch);
     else
         ma_nop();
     return CodeOffset(bo.getOffset());
 }
 
 void
@@ -4990,24 +4967,18 @@ MacroAssembler::call(wasm::SymbolicAddre
     call(CallReg);
 }
 
 void
 MacroAssembler::call(JitCode* c)
 {
     BufferOffset bo = m_buffer.nextOffset();
     addPendingJump(bo, ImmPtr(c->raw()), Relocation::JITCODE);
-    RelocStyle rs;
-    if (HasMOVWT())
-        rs = L_MOVWT;
-    else
-        rs = L_LDR;
-
     ScratchRegisterScope scratch(*this);
-    ma_movPatchable(ImmPtr(c->raw()), scratch, Always, rs);
+    ma_movPatchable(ImmPtr(c->raw()), scratch, Always);
     callJitNoProfiler(scratch);
 }
 
 CodeOffset
 MacroAssembler::callWithPatch()
 {
     // For now, assume that it'll be nearby.
     as_bl(BOffImm(), Always, /* documentation */ nullptr);
--- a/js/src/jit/arm/MacroAssembler-arm.h
+++ b/js/src/jit/arm/MacroAssembler-arm.h
@@ -110,20 +110,18 @@ class MacroAssemblerARM : public Assembl
     void ma_alu(Register src1, Imm32 imm, Register dest,
                 ALUOp op,
                 SBit s =  LeaveCC, Condition c = Always);
 
     void ma_alu(Register src1, Operand op2, Register dest, ALUOp op,
                 SBit s = LeaveCC, Condition c = Always);
     void ma_nop();
 
-    void ma_movPatchable(Imm32 imm, Register dest, Assembler::Condition c,
-                         RelocStyle rs);
-    void ma_movPatchable(ImmPtr imm, Register dest, Assembler::Condition c,
-                         RelocStyle rs);
+    void ma_movPatchable(Imm32 imm, Register dest, Assembler::Condition c);
+    void ma_movPatchable(ImmPtr imm, Register dest, Assembler::Condition c);
 
     static void ma_mov_patch(Imm32 imm, Register dest, Assembler::Condition c,
                              RelocStyle rs, Instruction* i);
     static void ma_mov_patch(ImmPtr imm, Register dest, Assembler::Condition c,
                              RelocStyle rs, Instruction* i);
 
     // These should likely be wrapped up as a set of macros or something like
     // that. I cannot think of a good reason to explicitly have all of this
@@ -514,24 +512,18 @@ class MacroAssemblerARMCompat : public M
     }
     void mov(Address src, Register dest) {
         MOZ_CRASH("NYI-IC");
     }
 
     void branch(JitCode* c) {
         BufferOffset bo = m_buffer.nextOffset();
         addPendingJump(bo, ImmPtr(c->raw()), Relocation::JITCODE);
-        RelocStyle rs;
-        if (HasMOVWT())
-            rs = L_MOVWT;
-        else
-            rs = L_LDR;
-
         ScratchRegisterScope scratch(asMasm());
-        ma_movPatchable(ImmPtr(c->raw()), scratch, Always, rs);
+        ma_movPatchable(ImmPtr(c->raw()), scratch, Always);
         ma_bx(scratch);
     }
     void branch(const Register reg) {
         ma_bx(reg);
     }
     void nop() {
         ma_nop();
     }
@@ -604,17 +596,17 @@ class MacroAssemblerARMCompat : public M
         ScratchRegisterScope scratch(asMasm());
         CodeOffset label = movWithPatch(imm, scratch);
         ma_push(scratch);
         return label;
     }
 
     CodeOffset movWithPatch(ImmWord imm, Register dest) {
         CodeOffset label = CodeOffset(currentOffset());
-        ma_movPatchable(Imm32(imm.value), dest, Always, HasMOVWT() ? L_MOVWT : L_LDR);
+        ma_movPatchable(Imm32(imm.value), dest, Always);
         return label;
     }
     CodeOffset movWithPatch(ImmPtr imm, Register dest) {
         return movWithPatch(ImmWord(uintptr_t(imm.value)), dest);
     }
 
     void jump(Label* label) {
         as_b(label);
--- a/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
+++ b/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
@@ -827,17 +827,17 @@ struct AssemblerBufferWithConstantPools 
         }
         return this->nextOffset();
     }
 
     BufferOffset allocEntry(size_t numInst, unsigned numPoolEntries,
                             uint8_t* inst, uint8_t* data, PoolEntry* pe = nullptr,
                             bool markAsBranch = false)
     {
-        // The alloction of pool entries is not supported in a no-pool region,
+        // The allocation of pool entries is not supported in a no-pool region,
         // check.
         MOZ_ASSERT_IF(numPoolEntries, !canNotPlacePool_);
 
         if (this->oom() && !this->bail())
             return BufferOffset();
 
         insertNopFill();