Bug 1127269 - Clean up some code in the ARM backend. r=sstangl
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 10 Nov 2016 10:14:17 +0100
changeset 348658 10205a882c3c11cb74065e86d6727a0920276b35
parent 348657 fe91fde5ba7a7ad83b307b5f5a247d861aa76947
child 348659 9afc1d0229c76f3876972638bbe398a91385e241
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstangl
bugs1127269
milestone52.0a1
Bug 1127269 - Clean up some code in the ARM backend. r=sstangl
js/src/jit/arm/Assembler-arm.cpp
js/src/jit/arm/Assembler-arm.h
js/src/jit/arm/MacroAssembler-arm-inl.h
js/src/jit/arm/MacroAssembler-arm.cpp
--- a/js/src/jit/arm/Assembler-arm.cpp
+++ b/js/src/jit/arm/Assembler-arm.cpp
@@ -585,42 +585,32 @@ InstMOV::IsTHIS(const Instruction& i)
     return InstALU::IsTHIS(i) && InstALU::AsTHIS(i)->checkOp1(r0) && InstALU::AsTHIS(i)->checkOp(OpMov);
 }
 
 Op2Reg
 Operand2::toOp2Reg() const {
     return *(Op2Reg*)this;
 }
 
-O2RegImmShift
-Op2Reg::toO2RegImmShift() const {
-    return *(O2RegImmShift*)this;
-}
-
-O2RegRegShift
-Op2Reg::toO2RegRegShift() const {
-    return *(O2RegRegShift*)this;
-}
-
 Imm16::Imm16(Instruction& inst)
-  : lower(inst.encode() & 0xfff),
-    upper(inst.encode() >> 16),
-    invalid(0xfff)
+  : lower_(inst.encode() & 0xfff),
+    upper_(inst.encode() >> 16),
+    invalid_(0xfff)
 { }
 
 Imm16::Imm16(uint32_t imm)
-  : lower(imm & 0xfff), pad(0),
-    upper((imm >> 12) & 0xf),
-    invalid(0)
+  : lower_(imm & 0xfff), pad_(0),
+    upper_((imm >> 12) & 0xf),
+    invalid_(0)
 {
     MOZ_ASSERT(decode() == imm);
 }
 
 Imm16::Imm16()
-  : invalid(0xfff)
+  : invalid_(0xfff)
 { }
 
 void
 jit::PatchJump(CodeLocationJump& jump_, CodeLocationLabel label, ReprotectCode reprotect)
 {
     // We need to determine if this jump can fit into the standard 24+2 bit
     // address or if we need a larger branch (or just need to use our pool
     // entry).
@@ -930,20 +920,20 @@ Assembler::copyPreBarrierTable(uint8_t* 
         memcpy(dest, preBarriers_.buffer(), preBarriers_.length());
 }
 
 void
 Assembler::trace(JSTracer* trc)
 {
     for (size_t i = 0; i < jumps_.length(); i++) {
         RelativePatch& rp = jumps_[i];
-        if (rp.kind == Relocation::JITCODE) {
-            JitCode* code = JitCode::FromExecutable((uint8_t*)rp.target);
+        if (rp.kind() == Relocation::JITCODE) {
+            JitCode* code = JitCode::FromExecutable((uint8_t*)rp.target());
             TraceManuallyBarrieredEdge(trc, &code, "masmrel32");
-            MOZ_ASSERT(code == JitCode::FromExecutable((uint8_t*)rp.target));
+            MOZ_ASSERT(code == JitCode::FromExecutable((uint8_t*)rp.target()));
         }
     }
 
     if (dataRelocations_.length()) {
         CompactBufferReader reader(dataRelocations_);
         ::TraceDataRelocations(trc, &m_buffer, reader);
     }
 }
@@ -1288,34 +1278,34 @@ jit::asr(Register r, Register amt)
 }
 
 static js::jit::DoubleEncoder doubleEncoder;
 
 /* static */ const js::jit::VFPImm js::jit::VFPImm::One(0x3FF00000);
 
 js::jit::VFPImm::VFPImm(uint32_t top)
 {
-    data = -1;
+    data_ = -1;
     datastore::Imm8VFPImmData tmp;
     if (doubleEncoder.lookup(top, &tmp))
-        data = tmp.encode();
+        data_ = tmp.encode();
 }
 
-BOffImm::BOffImm(Instruction& inst)
-  : data(inst.encode() & 0x00ffffff)
+BOffImm::BOffImm(const Instruction& inst)
+  : data_(inst.encode() & 0x00ffffff)
 {
 }
 
 Instruction*
 BOffImm::getDest(Instruction* src) const
 {
     // TODO: It is probably worthwhile to verify that src is actually a branch.
     // NOTE: This does not explicitly shift the offset of the destination left by 2,
     // since it is indexing into an array of instruction sized objects.
-    return &src[(((int32_t)data << 8) >> 8) + 2];
+    return &src[((int32_t(data_) << 8) >> 8) + 2];
 }
 
 const js::jit::DoubleEncoder::DoubleEntry js::jit::DoubleEncoder::table[256] = {
 #include "jit/arm/DoubleEntryTable.tbl"
 };
 
 // VFPRegister implementation
 VFPRegister
--- a/js/src/jit/arm/Assembler-arm.h
+++ b/js/src/jit/arm/Assembler-arm.h
@@ -267,24 +267,24 @@ 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.
 static constexpr VFPRegister NoVFPRegister(VFPRegister::Double, 0, false, true);
 
 struct ImmTag : public Imm32
 {
-    ImmTag(JSValueTag mask)
+    explicit ImmTag(JSValueTag mask)
       : Imm32(int32_t(mask))
     { }
 };
 
 struct ImmType : public ImmTag
 {
-    ImmType(JSValueType type)
+    explicit ImmType(JSValueType type)
       : ImmTag(JSVAL_TYPE_TO_TAG(type))
     { }
 };
 
 enum Index {
     Offset = 0 << 21 | 1<<24,
     PreIndex = 1 << 21 | 1 << 24,
     PostIndex = 0 << 21 | 0 << 24
@@ -309,26 +309,16 @@ enum IsImmEDTR_ {
 enum ShiftType {
     LSL = 0, // << 5
     LSR = 1, // << 5
     ASR = 2, // << 5
     ROR = 3, // << 5
     RRX = ROR // RRX is encoded as ROR with a 0 offset.
 };
 
-// The actual codes that get set by instructions and the codes that are checked
-// by the conditions below.
-struct ConditionCodes
-{
-    bool Zero : 1;
-    bool Overflow : 1;
-    bool Carry : 1;
-    bool Minus : 1;
-};
-
 // Modes for STM/LDM. Names are the suffixes applied to the instruction.
 enum DTMMode {
     A = 0 << 24, // empty / after
     B = 1 << 24, // full / before
     D = 0 << 23, // decrement
     I = 1 << 23, // increment
     DA = D | A,
     DB = D | B,
@@ -444,112 +434,111 @@ static const ValueOperand softfpReturnOp
 // storage. We also wanted to avoid passing around raw integers at all since
 // they are error prone.
 class Op2Reg;
 class O2RegImmShift;
 class O2RegRegShift;
 
 namespace datastore {
 
-struct Reg
+class Reg
 {
     // The "second register".
-    uint32_t RM : 4;
+    uint32_t rm_ : 4;
     // Do we get another register for shifting.
-    uint32_t RRS : 1;
-    ShiftType Type : 2;
+    bool rrs_ : 1;
+    ShiftType type_ : 2;
     // We'd like this to be a more sensible encoding, but that would need to be
     // a struct and that would not pack :(
-    uint32_t ShiftAmount : 5;
-    uint32_t pad : 20;
+    uint32_t shiftAmount_ : 5;
+    uint32_t pad_ : 20;
 
-    Reg(uint32_t rm, ShiftType type, uint32_t rsr, uint32_t shiftamount)
-      : RM(rm), RRS(rsr), Type(type), ShiftAmount(shiftamount), pad(0)
+  public:
+    Reg(uint32_t rm, ShiftType type, uint32_t rsr, uint32_t shiftAmount)
+      : rm_(rm), rrs_(rsr), type_(type), shiftAmount_(shiftAmount), pad_(0)
     { }
-
     explicit Reg(const Op2Reg& op) {
         memcpy(this, &op, sizeof(*this));
     }
 
+    uint32_t shiftAmount() const {
+        return shiftAmount_;
+    }
+
     uint32_t encode() const {
-        return RM | RRS << 4 | Type << 5 | ShiftAmount << 7;
+        return rm_ | (rrs_ << 4) | (type_ << 5) | (shiftAmount_ << 7);
     }
 };
 
 // Op2 has a mode labelled "<imm8m>", which is arm's magical immediate encoding.
 // Some instructions actually get 8 bits of data, which is called Imm8Data
 // below. These should have edit distance > 1, but this is how it is for now.
-struct Imm8mData
+class Imm8mData
 {
-  private:
-    uint32_t data : 8;
-    uint32_t rot : 4;
+    uint32_t data_ : 8;
+    uint32_t rot_ : 4;
+    uint32_t buff_ : 19;
+
     // Throw in an extra bit that will be 1 if we can't encode this properly.
     // if we can encode it properly, a simple "|" will still suffice to meld it
     // into the instruction.
-    uint32_t buff : 19;
-
-  public:
-    uint32_t invalid : 1;
+    bool invalid_ : 1;
 
   public:
-    uint32_t encode() const {
-        MOZ_ASSERT(!invalid);
-        return data | rot << 8;
-    };
-
     // Default constructor makes an invalid immediate.
     Imm8mData()
-      : data(0xff), rot(0xf), buff(0), invalid(1)
+      : data_(0xff), rot_(0xf), buff_(0), invalid_(true)
     { }
 
-    Imm8mData(uint32_t data_, uint32_t rot_)
-      : data(data_), rot(rot_), buff(0), invalid(0)
+    Imm8mData(uint32_t data, uint32_t rot)
+      : data_(data), rot_(rot), buff_(0), invalid_(false)
     {
         MOZ_ASSERT(data == data_);
         MOZ_ASSERT(rot == rot_);
     }
+
+    bool invalid() const { return invalid_; }
+
+    uint32_t encode() const {
+        MOZ_ASSERT(!invalid_);
+        return data_ | (rot_ << 8);
+    };
 };
 
-struct Imm8Data
+class Imm8Data
 {
-  private:
-    uint32_t imm4L : 4;
-    uint32_t pad : 4;
-    uint32_t imm4H : 4;
+    uint32_t imm4L_ : 4;
+    uint32_t pad_ : 4;
+    uint32_t imm4H_ : 4;
 
   public:
-    Imm8Data(uint32_t imm)
-      : imm4L(imm & 0xf), imm4H(imm >> 4)
+    explicit Imm8Data(uint32_t imm)
+      : imm4L_(imm & 0xf), imm4H_(imm >> 4)
     {
         MOZ_ASSERT(imm <= 0xff);
     }
 
-  public:
     uint32_t encode() const {
-        return imm4L | (imm4H << 8);
+        return imm4L_ | (imm4H_ << 8);
     };
 };
 
 // VLDR/VSTR take an 8 bit offset, which is implicitly left shifted by 2.
-struct Imm8VFPOffData
+class Imm8VFPOffData
 {
-  private:
-    uint32_t data;
+    uint32_t data_;
 
   public:
-    Imm8VFPOffData(uint32_t imm)
-      : data (imm)
+    explicit Imm8VFPOffData(uint32_t imm)
+      : data_(imm)
     {
         MOZ_ASSERT((imm & ~(0xff)) == 0);
     }
-
-  public:
     uint32_t encode() const {
-        return data;
+        return data_;
     };
 };
 
 // ARM can magically encode 256 very special immediates to be moved into a
 // register.
 struct Imm8VFPImmData
 {
     // This structure's members are public and it has no constructor to
@@ -568,204 +557,182 @@ struct Imm8VFPImmData
     uint32_t encode() const {
         // This assert is an attempting at ensuring that we don't create random
         // instances of this structure and then asking to encode() it.
         MOZ_ASSERT(isInvalid == 0);
         return imm4L | (imm4H << 16);
     };
 };
 
-struct Imm12Data
+class Imm12Data
 {
-    uint32_t data : 12;
+    uint32_t data_ : 12;
 
-    Imm12Data(uint32_t imm)
-      : data(imm)
+  public:
+    explicit Imm12Data(uint32_t imm)
+      : data_(imm)
     {
-        MOZ_ASSERT(data == imm);
+        MOZ_ASSERT(data_ == imm);
     }
 
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 };
 
-struct RIS
+class RIS
 {
-    uint32_t ShiftAmount : 5;
+    uint32_t shiftAmount_ : 5;
 
-    RIS(uint32_t imm)
-      : ShiftAmount(imm)
+  public:
+    explicit RIS(uint32_t imm)
+      : shiftAmount_(imm)
     {
-        MOZ_ASSERT(ShiftAmount == imm);
+        MOZ_ASSERT(shiftAmount_ == imm);
     }
 
     explicit RIS(Reg r)
-      : ShiftAmount(r.ShiftAmount)
+      : shiftAmount_(r.shiftAmount())
     { }
 
     uint32_t encode() const {
-        return ShiftAmount;
+        return shiftAmount_;
     }
 };
 
-struct RRS
+class RRS
 {
-    uint32_t MustZero : 1;
+    bool mustZero_ : 1;
     // The register that holds the shift amount.
-    uint32_t RS : 4;
+    uint32_t rs_ : 4;
 
-    RRS(uint32_t rs)
-      : RS(rs)
+  public:
+    explicit RRS(uint32_t rs)
+      : rs_(rs)
     {
-        MOZ_ASSERT(rs == RS);
+        MOZ_ASSERT(rs_ == rs);
     }
 
     uint32_t encode() const {
-        return RS << 1;
+        return rs_ << 1;
     }
 };
 
 } // namespace datastore
 
 class MacroAssemblerARM;
 class Operand;
+
 class Operand2
 {
     friend class Operand;
     friend class MacroAssemblerARM;
     friend class InstALU;
 
-  public:
-    uint32_t oper : 31;
-    uint32_t invalid : 1;
+    uint32_t oper_ : 31;
+    bool invalid_ : 1;
 
   protected:
     explicit Operand2(datastore::Imm8mData base)
-      : oper(base.invalid ? -1 : (base.encode() | (uint32_t)IsImmOp2)),
-        invalid(base.invalid)
+      : oper_(base.invalid() ? -1 : (base.encode() | uint32_t(IsImmOp2))),
+        invalid_(base.invalid())
     { }
 
     explicit Operand2(datastore::Reg base)
-      : oper(base.encode() | (uint32_t)IsNotImmOp2)
+      : oper_(base.encode() | uint32_t(IsNotImmOp2)),
+        invalid_(false)
     { }
 
   private:
-    explicit Operand2(int blob)
-      : oper(blob)
+    explicit Operand2(uint32_t blob)
+      : oper_(blob),
+        invalid_(false)
     { }
 
   public:
     bool isO2Reg() const {
-        return !(oper & IsImmOp2);
+        return !(oper_ & IsImmOp2);
     }
 
     Op2Reg toOp2Reg() const;
 
     bool isImm8() const {
-        return oper & IsImmOp2;
+        return oper_ & IsImmOp2;
+    }
+
+    bool invalid() const {
+        return invalid_;
     }
 
     uint32_t encode() const {
-        return oper;
+        return oper_;
     }
 };
 
 class Imm8 : public Operand2
 {
   public:
     explicit Imm8(uint32_t imm)
       : Operand2(EncodeImm(imm))
     { }
 
-  public:
     static datastore::Imm8mData EncodeImm(uint32_t imm) {
         // RotateLeft below may not be called with a shift of zero.
         if (imm <= 0xFF)
             return datastore::Imm8mData(imm, 0);
 
         // An encodable integer has a maximum of 8 contiguous set bits,
         // with an optional wrapped left rotation to even bit positions.
         for (int rot = 1; rot < 16; rot++) {
-            uint32_t rotimm = mozilla::RotateLeft(imm, rot*2);
+            uint32_t rotimm = mozilla::RotateLeft(imm, rot * 2);
             if (rotimm <= 0xFF)
                 return datastore::Imm8mData(rotimm, rot);
         }
         return datastore::Imm8mData();
     }
 
     // Pair template?
     struct TwoImm8mData
     {
-        datastore::Imm8mData fst, snd;
+        datastore::Imm8mData fst_, snd_;
 
-        TwoImm8mData()
-          : fst(), snd()
+        TwoImm8mData() = default;
+
+        TwoImm8mData(datastore::Imm8mData fst, datastore::Imm8mData snd)
+          : fst_(fst), snd_(snd)
         { }
 
-        TwoImm8mData(datastore::Imm8mData _fst, datastore::Imm8mData _snd)
-          : fst(_fst), snd(_snd)
-        { }
+        datastore::Imm8mData fst() const { return fst_; }
+        datastore::Imm8mData snd() const { return snd_; }
     };
 
     static TwoImm8mData EncodeTwoImms(uint32_t);
 };
 
 class Op2Reg : public Operand2
 {
   public:
     explicit Op2Reg(Register rm, ShiftType type, datastore::RIS shiftImm)
       : Operand2(datastore::Reg(rm.code(), type, 0, shiftImm.encode()))
     { }
 
     explicit Op2Reg(Register rm, ShiftType type, datastore::RRS shiftReg)
       : Operand2(datastore::Reg(rm.code(), type, 1, shiftReg.encode()))
     { }
-
-  public:
-    bool isO2RegImmShift() const {
-        datastore::Reg r(*this);
-        return !r.RRS;
-    }
-    O2RegImmShift toO2RegImmShift() const;
-
-    bool isO2RegRegShift() const {
-        datastore::Reg r(*this);
-        return r.RRS;
-    }
-    O2RegRegShift toO2RegRegShift() const;
+};
 
-    bool checkType(ShiftType type) const {
-        datastore::Reg r(*this);
-        return r.Type == type;
-    }
-    bool checkRM(Register rm) const {
-        datastore::Reg r(*this);
-        return r.RM == rm.code();
-    }
-    bool getRM(Register* rm) const {
-        datastore::Reg r(*this);
-        *rm = Register::FromCode(r.RM);
-        return true;
-    }
-};
+static_assert(sizeof(Op2Reg) == sizeof(datastore::Reg),
+              "datastore::Reg(const Op2Reg&) constructor relies on Reg/Op2Reg having same size");
 
 class O2RegImmShift : public Op2Reg
 {
   public:
     explicit O2RegImmShift(Register rn, ShiftType type, uint32_t shift)
       : Op2Reg(rn, type, datastore::RIS(shift))
     { }
-
-  public:
-    int getShift() const {
-        datastore::Reg r(*this);
-        datastore::RIS ris(r);
-        return ris.ShiftAmount;
-    }
 };
 
 class O2RegRegShift : public Op2Reg
 {
   public:
     explicit O2RegRegShift(Register rn, ShiftType type, Register rs)
       : Op2Reg(rn, type, datastore::RRS(rs.code()))
     { }
@@ -785,45 +752,46 @@ O2RegRegShift ror(Register r, Register a
 
 // An offset from a register to be used for ldr/str. This should include the
 // sign bit, since ARM has "signed-magnitude" offsets. That is it encodes an
 // unsigned offset, then the instruction specifies if the offset is positive or
 // negative. The +/- bit is necessary if the instruction set wants to be able to
 // have a negative register offset e.g. ldr pc, [r1,-r2];
 class DtrOff
 {
-    uint32_t data;
+    uint32_t data_;
 
   protected:
     explicit DtrOff(datastore::Imm12Data immdata, IsUp_ iu)
-      : data(immdata.encode() | (uint32_t)IsImmDTR | ((uint32_t)iu))
+      : data_(immdata.encode() | uint32_t(IsImmDTR) | uint32_t(iu))
     { }
 
     explicit DtrOff(datastore::Reg reg, IsUp_ iu = IsUp)
-      : data(reg.encode() | (uint32_t) IsNotImmDTR | iu)
+      : data_(reg.encode() | uint32_t(IsNotImmDTR) | iu)
     { }
 
   public:
-    uint32_t encode() const { return data; }
+    uint32_t encode() const { return data_; }
 };
 
 class DtrOffImm : public DtrOff
 {
   public:
     explicit DtrOffImm(int32_t imm)
       : DtrOff(datastore::Imm12Data(mozilla::Abs(imm)), imm >= 0 ? IsUp : IsDown)
     {
         MOZ_ASSERT(mozilla::Abs(imm) < 4096);
     }
 };
 
 class DtrOffReg : public DtrOff
 {
     // These are designed to be called by a constructor of a subclass.
-    // Constructing the necessary RIS/RRS structures are annoying.
+    // Constructing the necessary RIS/RRS structures is annoying.
+
   protected:
     explicit DtrOffReg(Register rn, ShiftType type, datastore::RIS shiftImm, IsUp_ iu = IsUp)
       : DtrOff(datastore::Reg(rn.code(), type, 0, shiftImm.encode()), iu)
     { }
 
     explicit DtrOffReg(Register rn, ShiftType type, datastore::RRS shiftReg, IsUp_ iu = IsUp)
       : DtrOff(datastore::Reg(rn.code(), type, 1, shiftReg.encode()), iu)
     { }
@@ -846,56 +814,50 @@ class DtrRegRegShift : public DtrOffReg
 };
 
 // We will frequently want to bundle a register with its offset so that we have
 // an "operand" to a load instruction.
 class DTRAddr
 {
     friend class Operand;
 
-    uint32_t data;
+    uint32_t data_;
 
   public:
     explicit DTRAddr(Register reg, DtrOff dtr)
-      : data(dtr.encode() | (reg.code() << 16))
+      : data_(dtr.encode() | (reg.code() << 16))
     { }
 
-  private:
-    explicit DTRAddr(uint32_t blob)
-      : data(blob)
-    { }
-
-  public:
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 
     Register getBase() const {
-        return Register::FromCode((data >> 16) &0xf);
+        return Register::FromCode((data_ >> 16) & 0xf);
     }
 };
 
 // Offsets for the extended data transfer instructions:
 // ldrsh, ldrd, ldrsb, etc.
 class EDtrOff
 {
-    uint32_t data;
+    uint32_t data_;
 
   protected:
     explicit EDtrOff(datastore::Imm8Data imm8, IsUp_ iu = IsUp)
-      : data(imm8.encode() | IsImmEDTR | (uint32_t)iu)
+      : data_(imm8.encode() | IsImmEDTR | uint32_t(iu))
     { }
 
     explicit EDtrOff(Register rm, IsUp_ iu = IsUp)
-      : data(rm.code() | IsNotImmEDTR | iu)
+      : data_(rm.code() | IsNotImmEDTR | iu)
     { }
 
   public:
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 };
 
 class EDtrOffImm : public EDtrOff
 {
   public:
     explicit EDtrOffImm(int32_t imm)
       : EDtrOff(datastore::Imm8Data(mozilla::Abs(imm)), (imm >= 0) ? IsUp : IsDown)
@@ -911,47 +873,47 @@ class EDtrOffReg : public EDtrOff
   public:
     explicit EDtrOffReg(Register rm)
       : EDtrOff(rm)
     { }
 };
 
 class EDtrAddr
 {
-    uint32_t data;
+    uint32_t data_;
 
   public:
     explicit EDtrAddr(Register r, EDtrOff off)
-      : data(RN(r) | off.encode())
+      : data_(RN(r) | off.encode())
     { }
 
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 #ifdef DEBUG
     Register maybeOffsetRegister() const {
-        if (data & IsImmEDTR)
+        if (data_ & IsImmEDTR)
             return InvalidReg;
-        return Register::FromCode(data & 0xf);
+        return Register::FromCode(data_ & 0xf);
     }
 #endif
 };
 
 class VFPOff
 {
-    uint32_t data;
+    uint32_t data_;
 
   protected:
     explicit VFPOff(datastore::Imm8VFPOffData imm, IsUp_ isup)
-      : data(imm.encode() | (uint32_t)isup)
+      : data_(imm.encode() | uint32_t(isup))
     { }
 
   public:
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 };
 
 class VFPOffImm : public VFPOff
 {
   public:
     explicit VFPOffImm(int32_t imm)
       : VFPOff(datastore::Imm8VFPOffData(mozilla::Abs(imm) / 4), imm < 0 ? IsDown : IsUp)
@@ -959,206 +921,197 @@ class VFPOffImm : public VFPOff
         MOZ_ASSERT(mozilla::Abs(imm) <= 255 * 4);
     }
 };
 
 class VFPAddr
 {
     friend class Operand;
 
-    uint32_t data;
+    uint32_t data_;
 
   public:
     explicit VFPAddr(Register base, VFPOff off)
-      : data(RN(base) | off.encode())
+      : data_(RN(base) | off.encode())
     { }
 
-  protected:
-    VFPAddr(uint32_t blob)
-      : data(blob)
-    { }
-
-  public:
     uint32_t encode() const {
-        return data;
+        return data_;
     }
 };
 
 class VFPImm
 {
-    uint32_t data;
+    uint32_t data_;
 
   public:
     explicit VFPImm(uint32_t topWordOfDouble);
 
-  public:
     static const VFPImm One;
 
     uint32_t encode() const {
-        return data;
+        return data_;
     }
     bool isValid() const {
-        return data != -1U;
+        return data_ != -1U;
     }
 };
 
 // A BOffImm is an immediate that is used for branches. Namely, it is the offset
 // that will be encoded in the branch instruction. This is the only sane way of
 // constructing a branch.
 class BOffImm
 {
     friend class InstBranchImm;
 
-    uint32_t data;
+    uint32_t data_;
 
   public:
     explicit BOffImm(int offset)
-      : data ((offset - 8) >> 2 & 0x00ffffff)
+      : data_((offset - 8) >> 2 & 0x00ffffff)
     {
         MOZ_ASSERT((offset & 0x3) == 0);
         if (!IsInRange(offset))
             MOZ_CRASH("BOffImm offset out of range");
     }
 
     explicit BOffImm()
-      : data(INVALID)
+      : data_(INVALID)
     { }
 
   private:
-    BOffImm(Instruction& inst);
+    explicit BOffImm(const Instruction& inst);
 
   public:
-    static const int INVALID = 0x00800000;
+    static const uint32_t INVALID = 0x00800000;
 
     uint32_t encode() const {
-        return data;
+        return data_;
     }
     int32_t decode() const {
-        return ((((int32_t)data) << 8) >> 6) + 8;
+        return ((int32_t(data_) << 8) >> 6) + 8;
     }
 
     static bool IsInRange(int offset) {
         if ((offset - 8) < -33554432)
             return false;
         if ((offset - 8) > 33554428)
             return false;
         return true;
     }
 
     bool isInvalid() const {
-        return data == uint32_t(INVALID);
+        return data_ == INVALID;
     }
     Instruction* getDest(Instruction* src) const;
 };
 
 class Imm16
 {
-    uint32_t lower : 12;
-    uint32_t pad : 4;
-    uint32_t upper : 4;
-    uint32_t invalid : 12;
+    uint32_t lower_ : 12;
+    uint32_t pad_ : 4;
+    uint32_t upper_ : 4;
+    uint32_t invalid_ : 12;
 
   public:
     explicit Imm16();
     explicit Imm16(uint32_t imm);
     explicit Imm16(Instruction& inst);
 
     uint32_t encode() const {
-        return lower | upper << 16;
+        return lower_ | (upper_ << 16);
     }
     uint32_t decode() const {
-        return lower | upper << 12;
+        return lower_ | (upper_ << 12);
     }
 
-    bool isInvalid () const {
-        return invalid;
+    bool isInvalid() const {
+        return invalid_;
     }
 };
 
 // I would preffer that these do not exist, since there are essentially no
 // instructions that would ever take more than one of these, however, the MIR
 // wants to only have one type of arguments to functions, so bugger.
 class Operand
 {
     // The encoding of registers is the same for OP2, DTR and EDTR yet the type
     // system doesn't let us express this, so choices must be made.
   public:
-    enum Tag_ {
+    enum class Tag : uint8_t {
         OP2,
         MEM,
         FOP
     };
 
   private:
-    Tag_ Tag : 3;
-    uint32_t reg : 5;
-    int32_t offset;
+    Tag tag_ : 8;
+    uint32_t reg_ : 5;
+    int32_t offset_;
 
   public:
-    explicit Operand(Register reg_)
-      : Tag(OP2), reg(reg_.code())
+    explicit Operand(Register reg)
+      : tag_(Tag::OP2), reg_(reg.code())
     { }
 
     explicit Operand(FloatRegister freg)
-      : Tag(FOP), reg(freg.code())
+      : tag_(Tag::FOP), reg_(freg.code())
     { }
 
     explicit Operand(Register base, Imm32 off)
-      : Tag(MEM), reg(base.code()), offset(off.value)
+      : tag_(Tag::MEM), reg_(base.code()), offset_(off.value)
     { }
 
     explicit Operand(Register base, int32_t off)
-      : Tag(MEM), reg(base.code()), offset(off)
+      : tag_(Tag::MEM), reg_(base.code()), offset_(off)
     { }
 
     explicit Operand(const Address& addr)
-      : Tag(MEM), reg(addr.base.code()), offset(addr.offset)
+      : tag_(Tag::MEM), reg_(addr.base.code()), offset_(addr.offset)
     { }
 
   public:
-    Tag_ getTag() const {
-        return Tag;
+    Tag tag() const {
+        return tag_;
     }
 
     Operand2 toOp2() const {
-        MOZ_ASSERT(Tag == OP2);
-        return O2Reg(Register::FromCode(reg));
+        MOZ_ASSERT(tag_ == Tag::OP2);
+        return O2Reg(Register::FromCode(reg_));
     }
 
     Register toReg() const {
-        MOZ_ASSERT(Tag == OP2);
-        return Register::FromCode(reg);
+        MOZ_ASSERT(tag_ == Tag::OP2);
+        return Register::FromCode(reg_);
     }
 
-    void toAddr(Register* r, Imm32* dest) const {
-        MOZ_ASSERT(Tag == MEM);
-        *r = Register::FromCode(reg);
-        *dest = Imm32(offset);
-    }
     Address toAddress() const {
-        MOZ_ASSERT(Tag == MEM);
-        return Address(Register::FromCode(reg), offset);
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return Address(Register::FromCode(reg_), offset_);
     }
     int32_t disp() const {
-        MOZ_ASSERT(Tag == MEM);
-        return offset;
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return offset_;
     }
 
     int32_t base() const {
-        MOZ_ASSERT(Tag == MEM);
-        return reg;
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return reg_;
     }
     Register baseReg() const {
-        return Register::FromCode(reg);
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return Register::FromCode(reg_);
     }
     DTRAddr toDTRAddr() const {
-        return DTRAddr(baseReg(), DtrOffImm(offset));
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return DTRAddr(baseReg(), DtrOffImm(offset_));
     }
     VFPAddr toVFPAddr() const {
-        return VFPAddr(baseReg(), VFPOffImm(offset));
+        MOZ_ASSERT(tag_ == Tag::MEM);
+        return VFPAddr(baseReg(), VFPOffImm(offset_));
     }
 };
 
 inline Imm32
 Imm64::firstHalf() const
 {
     return low();
 }
@@ -1313,21 +1266,25 @@ class Assembler : public AssemblerShared
     static uint32_t AsmPoolMaxOffset;
     static uint32_t GetPoolMaxOffset();
 
   protected:
     // Structure for fixing up pc-relative loads/jumps when a the machine code
     // gets moved (executable copy, gc, etc.).
     struct RelativePatch
     {
-        void* target;
-        Relocation::Kind kind;
+        void* target_;
+        Relocation::Kind kind_;
+
+      public:
         RelativePatch(void* target, Relocation::Kind kind)
-            : target(target), kind(kind)
+          : target_(target), kind_(kind)
         { }
+        void* target() const { return target_; }
+        Relocation::Kind kind() const { return kind_; }
     };
 
     // TODO: this should actually be a pool-like object. It is currently a big
     // hack, and probably shouldn't exist.
     js::Vector<RelativePatch, 8, SystemAllocPolicy> jumps_;
 
     CompactBufferWriter jumpRelocations_;
     CompactBufferWriter dataRelocations_;
@@ -2253,17 +2210,17 @@ class InstMovT : public InstMovWT
     static InstMovT* AsTHIS (const Instruction& i);
 };
 
 class InstALU : public Instruction
 {
     static const int32_t ALUMask = 0xc << 24;
 
   public:
-    InstALU (Register rd, Register rn, Operand2 op2, ALUOp op, SBit s, Assembler::Condition c)
+    InstALU(Register rd, Register rn, Operand2 op2, ALUOp op, SBit s, Assembler::Condition c)
       : Instruction(maybeRD(rd) | maybeRN(rn) | op2.encode() | op | s, c)
     { }
 
     static bool IsTHIS (const Instruction& i);
     static InstALU* AsTHIS (const Instruction& i);
 
     void extractOp(ALUOp* ret);
     bool checkOp(ALUOp op);
@@ -2416,18 +2373,16 @@ GetDoubleArgStackDisp(uint32_t usedIntAr
     }
     uint32_t doubleSlots = usedFloatArgs - NumFloatArgRegs;
     doubleSlots *= 2;
     return (intSlots + doubleSlots + *padding) * sizeof(intptr_t);
 }
 
 #endif
 
-
-
 class DoubleEncoder
 {
     struct DoubleEntry
     {
         uint32_t dblTop;
         datastore::Imm8VFPImmData data;
     };
 
--- a/js/src/jit/arm/MacroAssembler-arm-inl.h
+++ b/js/src/jit/arm/MacroAssembler-arm-inl.h
@@ -2116,17 +2116,17 @@ MacroAssembler::wasmPatchBoundsCheck(uin
     InstCMP* cmp = inst->as<InstCMP>();
 
     Register index;
     cmp->extractOp1(&index);
 
     MOZ_ASSERT(cmp->extractOp2().isImm8());
 
     Imm8 imm8 = Imm8(limit);
-    MOZ_RELEASE_ASSERT(!imm8.invalid);
+    MOZ_RELEASE_ASSERT(!imm8.invalid());
 
     *inst = InstALU(InvalidReg, index, imm8, OpCmp, SetCC, Always);
     // Don't call Auto Flush Cache; the wasm caller has done this for us.
 }
 
 //}}} check_macroassembler_style
 // ===============================================================
 
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -237,61 +237,60 @@ bool
 MacroAssemblerARM::alu_dbl(Register src1, Imm32 imm, Register dest, ALUOp op,
                            SBit s, Condition c)
 {
     if ((s == SetCC && ! condsAreSafe(op)) || !can_dbl(op))
         return false;
 
     ALUOp interop = getDestVariant(op);
     Imm8::TwoImm8mData both = Imm8::EncodeTwoImms(imm.value);
-    if (both.fst.invalid)
+    if (both.fst().invalid())
         return false;
 
     // For the most part, there is no good reason to set the condition codes for
     // the first instruction. We can do better things if the second instruction
     // doesn't have a dest, such as check for overflow by doing first operation
     // don't do second operation if first operation overflowed. This preserves
     // the overflow condition code. Unfortunately, it is horribly brittle.
-    as_alu(dest, src1, Operand2(both.fst), interop, LeaveCC, c);
-    as_alu(dest, dest, Operand2(both.snd), op, s, c);
+    as_alu(dest, src1, Operand2(both.fst()), interop, LeaveCC, c);
+    as_alu(dest, dest, Operand2(both.snd()), op, s, c);
     return true;
 }
 
 void
 MacroAssemblerARM::ma_alu(Register src1, Imm32 imm, Register dest, AutoRegisterScope& scratch,
                           ALUOp op, SBit s, Condition c)
 {
     // ma_mov should be used for moves.
     MOZ_ASSERT(op != OpMov);
     MOZ_ASSERT(op != OpMvn);
     MOZ_ASSERT(src1 != scratch);
 
     // As it turns out, if you ask for a compare-like instruction you *probably*
     // want it to set condition codes.
-    if (dest == InvalidReg)
-        MOZ_ASSERT(s == SetCC);
+    MOZ_ASSERT_IF(dest == InvalidReg, s == SetCC);
 
     // The operator gives us the ability to determine how this can be used.
     Imm8 imm8 = Imm8(imm.value);
     // One instruction: If we can encode it using an imm8m, then do so.
-    if (!imm8.invalid) {
+    if (!imm8.invalid()) {
         as_alu(dest, src1, imm8, op, s, c);
         return;
     }
 
     // One instruction, negated:
     Imm32 negImm = imm;
     Register negDest;
     ALUOp negOp = ALUNeg(op, dest, scratch, &negImm, &negDest);
     Imm8 negImm8 = Imm8(negImm.value);
     // 'add r1, r2, -15' can be replaced with 'sub r1, r2, 15'.
     // The dest can be replaced (InvalidReg => scratch).
     // This is useful if we wish to negate tst. tst has an invalid (aka not
     // used) dest, but its negation bic requires a dest.
-    if (negOp != OpInvalid && !negImm8.invalid) {
+    if (negOp != OpInvalid && !negImm8.invalid()) {
         as_alu(negDest, src1, negImm8, negOp, s, c);
         return;
     }
 
     // Start by attempting to generate a two instruction form. Some things
     // cannot be made into two-inst forms correctly. Namely, adds dest, src,
     // 0xffff. Since we want the condition codes (and don't know which ones
     // will be checked), we need to assume that the overflow flag will be
@@ -306,19 +305,19 @@ MacroAssemblerARM::ma_alu(Register src1,
         return;
 
     ma_mov(imm, scratch, c);
     as_alu(dest, src1, O2Reg(scratch), op, s, c);
 }
 
 void
 MacroAssemblerARM::ma_alu(Register src1, Operand op2, Register dest, ALUOp op,
-            SBit s, Assembler::Condition c)
-{
-    MOZ_ASSERT(op2.getTag() == Operand::OP2);
+                          SBit s, Assembler::Condition c)
+{
+    MOZ_ASSERT(op2.tag() == Operand::Tag::OP2);
     as_alu(dest, src1, op2.toOp2(), op, s, c);
 }
 
 void
 MacroAssemblerARM::ma_alu(Register src1, Operand2 op2, Register dest, ALUOp op, SBit s, Condition c)
 {
     as_alu(dest, src1, op2, op, s, c);
 }
@@ -384,24 +383,24 @@ MacroAssemblerARM::ma_mov(Register src, 
         as_mov(dest, O2Reg(src), s, c);
 }
 
 void
 MacroAssemblerARM::ma_mov(Imm32 imm, Register dest, Assembler::Condition c)
 {
     // Try mov with Imm8 operand.
     Imm8 imm8 = Imm8(imm.value);
-    if (!imm8.invalid) {
+    if (!imm8.invalid()) {
         as_alu(dest, InvalidReg, imm8, OpMov, LeaveCC, c);
         return;
     }
 
     // Try mvn with Imm8 operand.
     Imm8 negImm8 = Imm8(~imm.value);
-    if (!negImm8.invalid) {
+    if (!negImm8.invalid()) {
         as_alu(dest, InvalidReg, negImm8, OpMvn, LeaveCC, c);
         return;
     }
 
     // Try movw/movt.
     if (HasMOVWT()) {
         // ARMv7 supports movw/movt. movw zero-extends its 16 bit argument,
         // so we can set the register this way. movt leaves the bottom 16
@@ -769,17 +768,17 @@ MacroAssemblerARM::ma_cmp(Register src1,
     ma_alu(src1, imm, InvalidReg, scratch, OpCmp, SetCC, c);
 }
 
 void
 MacroAssemblerARM::ma_cmp(Register src1, ImmTag tag, Condition c)
 {
     // ImmTag comparisons can always be done without use of a scratch register.
     Imm8 negtag = Imm8(-tag.value);
-    MOZ_ASSERT(!negtag.invalid);
+    MOZ_ASSERT(!negtag.invalid());
     as_cmn(src1, negtag, c);
 }
 
 void
 MacroAssemblerARM::ma_cmp(Register src1, ImmWord ptr, AutoRegisterScope& scratch, Condition c)
 {
     ma_cmp(src1, Imm32(ptr.value), scratch, c);
 }
@@ -790,21 +789,21 @@ MacroAssemblerARM::ma_cmp(Register src1,
     ma_mov(ptr, scratch);
     ma_cmp(src1, scratch, c);
 }
 
 void
 MacroAssemblerARM::ma_cmp(Register src1, Operand op, AutoRegisterScope& scratch,
                           AutoRegisterScope& scratch2, Condition c)
 {
-    switch (op.getTag()) {
-      case Operand::OP2:
+    switch (op.tag()) {
+      case Operand::Tag::OP2:
         as_cmp(src1, op.toOp2(), c);
         break;
-      case Operand::MEM:
+      case Operand::Tag::MEM:
         ma_ldr(op.toAddress(), scratch, scratch2);
         as_cmp(src1, O2Reg(scratch), c);
         break;
       default:
         MOZ_CRASH("trying to compare FP and integer registers");
     }
 }
 
@@ -1204,43 +1203,43 @@ MacroAssemblerARM::ma_dataTransferN(Load
         // At this point, both off - bottom and off + neg_bottom will be
         // reasonable-ish quantities.
         //
         // Note a neg_bottom of 0x1000 can not be encoded as an immediate
         // negative offset in the instruction and this occurs when bottom is
         // zero, so this case is guarded against below.
         if (off < 0) {
             Operand2 sub_off = Imm8(-(off - bottom)); // sub_off = bottom - off
-            if (!sub_off.invalid) {
+            if (!sub_off.invalid()) {
                 // - sub_off = off - bottom
                 as_sub(scratch, rn, sub_off, LeaveCC, cc);
                 return as_dtr(ls, size, Offset, rt, DTRAddr(scratch, DtrOffImm(bottom)), cc);
             }
 
             // sub_off = -neg_bottom - off
             sub_off = Imm8(-(off + neg_bottom));
-            if (!sub_off.invalid && bottom != 0) {
+            if (!sub_off.invalid() && bottom != 0) {
                 // Guarded against by: bottom != 0
                 MOZ_ASSERT(neg_bottom < 0x1000);
                 // - sub_off = neg_bottom + off
                 as_sub(scratch, rn, sub_off, LeaveCC, cc);
                 return as_dtr(ls, size, Offset, rt, DTRAddr(scratch, DtrOffImm(-neg_bottom)), cc);
             }
         } else {
             // sub_off = off - bottom
             Operand2 sub_off = Imm8(off - bottom);
-            if (!sub_off.invalid) {
+            if (!sub_off.invalid()) {
                 //  sub_off = off - bottom
                 as_add(scratch, rn, sub_off, LeaveCC, cc);
                 return as_dtr(ls, size, Offset, rt, DTRAddr(scratch, DtrOffImm(bottom)), cc);
             }
 
             // sub_off = neg_bottom + off
             sub_off = Imm8(off + neg_bottom);
-            if (!sub_off.invalid && bottom != 0) {
+            if (!sub_off.invalid() && bottom != 0) {
                 // Guarded against by: bottom != 0
                 MOZ_ASSERT(neg_bottom < 0x1000);
                 // sub_off = neg_bottom + off
                 as_add(scratch, rn, sub_off, LeaveCC,  cc);
                 return as_dtr(ls, size, Offset, rt, DTRAddr(scratch, DtrOffImm(-neg_bottom)), cc);
             }
         }
 
@@ -1259,47 +1258,47 @@ MacroAssemblerARM::ma_dataTransferN(Load
         // reasonable-ish quantities.
         //
         // Note a neg_bottom of 0x100 can not be encoded as an immediate
         // negative offset in the instruction and this occurs when bottom is
         // zero, so this case is guarded against below.
         if (off < 0) {
             // sub_off = bottom - off
             Operand2 sub_off = Imm8(-(off - bottom));
-            if (!sub_off.invalid) {
+            if (!sub_off.invalid()) {
                 // - sub_off = off - bottom
                 as_sub(scratch, rn, sub_off, LeaveCC, cc);
                 return as_extdtr(ls, size, IsSigned, Offset, rt,
                                  EDtrAddr(scratch, EDtrOffImm(bottom)),
                                  cc);
             }
             // sub_off = -neg_bottom - off
             sub_off = Imm8(-(off + neg_bottom));
-            if (!sub_off.invalid && bottom != 0) {
+            if (!sub_off.invalid() && bottom != 0) {
                 // Guarded against by: bottom != 0
                 MOZ_ASSERT(neg_bottom < 0x100);
                 // - sub_off = neg_bottom + off
                 as_sub(scratch, rn, sub_off, LeaveCC, cc);
                 return as_extdtr(ls, size, IsSigned, Offset, rt,
                                  EDtrAddr(scratch, EDtrOffImm(-neg_bottom)),
                                  cc);
             }
         } else {
             // sub_off = off - bottom
             Operand2 sub_off = Imm8(off - bottom);
-            if (!sub_off.invalid) {
+            if (!sub_off.invalid()) {
                 // sub_off = off - bottom
                 as_add(scratch, rn, sub_off, LeaveCC, cc);
                 return as_extdtr(ls, size, IsSigned, Offset, rt,
                                  EDtrAddr(scratch, EDtrOffImm(bottom)),
                                  cc);
             }
             // sub_off = neg_bottom + off
             sub_off = Imm8(off + neg_bottom);
-            if (!sub_off.invalid && bottom != 0) {
+            if (!sub_off.invalid() && bottom != 0) {
                 // Guarded against by: bottom != 0
                 MOZ_ASSERT(neg_bottom < 0x100);
                 // sub_off = neg_bottom + off
                 as_add(scratch, rn, sub_off, LeaveCC,  cc);
                 return as_extdtr(ls, size, IsSigned, Offset, rt,
                                  EDtrAddr(scratch, EDtrOffImm(-neg_bottom)),
                                  cc);
             }
@@ -1720,41 +1719,41 @@ MacroAssemblerARM::ma_vdtr(LoadStore ls,
     // reasonable-ish quantities.
     //
     // Note a neg_bottom of 0x400 can not be encoded as an immediate negative
     // offset in the instruction and this occurs when bottom is zero, so this
     // case is guarded against below.
     if (off < 0) {
         // sub_off = bottom - off
         Operand2 sub_off = Imm8(-(off - bottom));
-        if (!sub_off.invalid) {
+        if (!sub_off.invalid()) {
             // - sub_off = off - bottom
             as_sub(scratch, base, sub_off, LeaveCC, cc);
             return as_vdtr(ls, rt, VFPAddr(scratch, VFPOffImm(bottom)), cc);
         }
         // sub_off = -neg_bottom - off
         sub_off = Imm8(-(off + neg_bottom));
-        if (!sub_off.invalid && bottom != 0) {
+        if (!sub_off.invalid() && bottom != 0) {
             // Guarded against by: bottom != 0
             MOZ_ASSERT(neg_bottom < 0x400);
             // - sub_off = neg_bottom + off
             as_sub(scratch, base, sub_off, LeaveCC, cc);
             return as_vdtr(ls, rt, VFPAddr(scratch, VFPOffImm(-neg_bottom)), cc);
         }
     } else {
         // sub_off = off - bottom
         Operand2 sub_off = Imm8(off - bottom);
-        if (!sub_off.invalid) {
+        if (!sub_off.invalid()) {
             // sub_off = off - bottom
             as_add(scratch, base, sub_off, LeaveCC, cc);
             return as_vdtr(ls, rt, VFPAddr(scratch, VFPOffImm(bottom)), cc);
         }
         // sub_off = neg_bottom + off
         sub_off = Imm8(off + neg_bottom);
-        if (!sub_off.invalid && bottom != 0) {
+        if (!sub_off.invalid() && bottom != 0) {
             // Guarded against by: bottom != 0
             MOZ_ASSERT(neg_bottom < 0x400);
             // sub_off = neg_bottom + off
             as_add(scratch, base, sub_off, LeaveCC, cc);
             return as_vdtr(ls, rt, VFPAddr(scratch, VFPOffImm(-neg_bottom)), cc);
         }
     }