Bug 1445970 - Fix ARM simulator on MSVC / Windows. r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Thu, 15 Mar 2018 10:58:21 -0400
changeset 462571 7447cde0139912715f547088334e0af5ef2407f6
parent 462570 3f688199eba12db44ee5f2e72905e290e97762c4
child 462572 fa371afe65787ef2d43b3d81c6b393928371e263
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1445970
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1445970 - Fix ARM simulator on MSVC / Windows. r=jandem - Bitfield packing requires same base type. Use uint32_t and add required conversions. - Enums with high-bit set, must be explictly marked unsigned if that is what we expect. - MSVC warns about implicit 32/64 shift confusion, so be explicit. - MSVC doesn't support asm() syntax. - masm.ma_sub requires a ScratchRegisterScope - MSVC sometimes gets confused by DebugOnly<T> - MSVC warns about "-1U", so fix that. MozReview-Commit-ID: BETAqbHKvUl
js/src/jit/arm/Architecture-arm.cpp
js/src/jit/arm/Architecture-arm.h
js/src/jit/arm/Assembler-arm.cpp
js/src/jit/arm/Assembler-arm.h
js/src/jit/arm/CodeGenerator-arm.cpp
js/src/jit/arm/Simulator-arm.cpp
js/src/jit/arm/Trampoline-arm.cpp
--- a/js/src/jit/arm/Architecture-arm.cpp
+++ b/js/src/jit/arm/Architecture-arm.cpp
@@ -6,17 +6,19 @@
 
 #include "jit/arm/Architecture-arm.h"
 
 #if !defined(JS_SIMULATOR_ARM) && !defined(__APPLE__)
 #include <elf.h>
 #endif
 
 #include <fcntl.h>
-#include <unistd.h>
+#ifdef XP_UNIX
+#  include <unistd.h>
+#endif
 
 #include "jit/arm/Assembler-arm.h"
 #include "jit/RegisterSets.h"
 
 #if !defined(__linux__) || defined(ANDROID) || defined(JS_SIMULATOR_ARM)
 // The Android NDK and B2G do not include the hwcap.h kernel header, and it is not
 // defined when building the simulator, so inline the header defines we need.
 # define HWCAP_VFP        (1 << 6)
--- a/js/src/jit/arm/Architecture-arm.h
+++ b/js/src/jit/arm/Architecture-arm.h
@@ -366,51 +366,51 @@ template <typename T>
 class TypedRegisterSet;
 
 class VFPRegister
 {
   public:
     // What type of data is being stored in this register? UInt / Int are
     // specifically for vcvt, where we need to know how the data is supposed to
     // be converted.
-    enum RegType {
+    enum RegType : uint8_t {
         Single = 0x0,
         Double = 0x1,
         UInt   = 0x2,
         Int    = 0x3
     };
 
     typedef FloatRegisters Codes;
     typedef Codes::Code Code;
     typedef Codes::Encoding Encoding;
 
-  protected:
-    RegType kind : 2;
+    // Bitfields below are all uint32_t to make sure MSVC packs them correctly.
   public:
     // ARM doesn't have more than 32 registers of each type, so 5 bits should
     // suffice.
     uint32_t code_ : 5;
   protected:
-    bool _isInvalid : 1;
-    bool _isMissing : 1;
+    uint32_t kind : 2;
+    uint32_t _isInvalid : 1;
+    uint32_t _isMissing : 1;
 
   public:
     constexpr VFPRegister(uint32_t r, RegType k)
-      : kind(k), code_(Code(r)), _isInvalid(false), _isMissing(false)
+      : code_(Code(r)), kind(k), _isInvalid(false), _isMissing(false)
     { }
     constexpr VFPRegister()
-      : kind(Double), code_(Code(0)), _isInvalid(true), _isMissing(false)
+      : code_(Code(0)), kind(Double), _isInvalid(true), _isMissing(false)
     { }
 
     constexpr VFPRegister(RegType k, uint32_t id, bool invalid, bool missing) :
-        kind(k), code_(Code(id)), _isInvalid(invalid), _isMissing(missing) {
+        code_(Code(id)), kind(k), _isInvalid(invalid), _isMissing(missing) {
     }
 
     explicit constexpr VFPRegister(Code id)
-      : kind(Double), code_(id), _isInvalid(false), _isMissing(false)
+      : code_(id), kind(Double), _isInvalid(false), _isMissing(false)
     { }
     bool operator==(const VFPRegister& other) const {
         return kind == other.kind && code_ == other.code_ && isInvalid() == other.isInvalid();
     }
     bool operator!=(const VFPRegister& other) const {
         return !operator==(other);
     }
 
@@ -470,18 +470,18 @@ class VFPRegister
     }
     static VFPRegister FromCode(uint32_t i) {
         uint32_t code = i & 31;
         uint32_t kind = i >> 5;
         return VFPRegister(code, RegType(kind));
     }
     bool volatile_() const {
         if (isDouble())
-            return !!((1 << (code_ >> 1)) & FloatRegisters::VolatileMask);
-        return !!((1 << code_) & FloatRegisters::VolatileMask);
+            return !!((1ULL << (code_ >> 1)) & FloatRegisters::VolatileMask);
+        return !!((1ULL << code_) & FloatRegisters::VolatileMask);
     }
     const char* name() const {
         if (isDouble())
             return FloatRegisters::GetDoubleName(Encoding(code_));
         return FloatRegisters::GetSingleName(Encoding(code_));
     }
     bool aliases(const VFPRegister& other) {
         if (kind == other.kind)
--- a/js/src/jit/arm/Assembler-arm.cpp
+++ b/js/src/jit/arm/Assembler-arm.cpp
@@ -1778,17 +1778,17 @@ class PoolHintData
         PoolDTR    = 1,
         PoolBranch = 2,
         PoolVDTR   = 3
     };
 
   private:
     uint32_t   index_    : 16;
     uint32_t   cond_     : 4;
-    LoadType   loadType_ : 2;
+    uint32_t   loadType_ : 2;
     uint32_t   destReg_  : 5;
     uint32_t   destType_ : 1;
     uint32_t   ONES     : 4;
 
     static const uint32_t ExpectedOnes = 0xfu;
 
   public:
     void init(uint32_t index, Assembler::Condition cond, LoadType lt, Register destReg) {
@@ -1834,17 +1834,17 @@ class PoolHintData
     }
 
     LoadType getLoadType() const {
         // If this *was* a PoolBranch, but the branch has already been bound
         // then this isn't going to look like a real poolhintdata, but we still
         // want to lie about it so everyone knows it *used* to be a branch.
         if (ONES != ExpectedOnes)
             return PoolHintData::PoolBranch;
-        return loadType_;
+        return static_cast<LoadType>(loadType_);
     }
 
     bool isValidPoolHint() const {
         // Most instructions cannot have a condition that is 0xf. Notable
         // exceptions are blx and the entire NEON instruction set. For the
         // purposes of pool loads, and possibly patched branches, the possible
         // instructions are ldr and b, neither of which can have a condition
         // code of 0xf.
@@ -2865,17 +2865,17 @@ Assembler::RetargetFarBranch(Instruction
 
 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.
         uint32_t size : 15;
-        bool isNatural : 1;
+        uint32_t isNatural : 1;
         uint32_t ONES : 16;
 
         Header(int size_, bool isNatural_)
           : size(size_),
             isNatural(isNatural_),
             ONES(0xffff)
         { }
 
--- a/js/src/jit/arm/Assembler-arm.h
+++ b/js/src/jit/arm/Assembler-arm.h
@@ -443,18 +443,18 @@ class O2RegRegShift;
 
 namespace datastore {
 
 class Reg
 {
     // The "second register".
     uint32_t rm_ : 4;
     // Do we get another register for shifting.
-    bool rrs_ : 1;
-    ShiftType type_ : 2;
+    uint32_t rrs_ : 1;
+    uint32_t 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;
 
   public:
     Reg(uint32_t rm, ShiftType type, uint32_t rsr, uint32_t shiftAmount)
       : rm_(rm), rrs_(rsr), type_(type), shiftAmount_(shiftAmount), pad_(0)
@@ -479,17 +479,17 @@ class Imm8mData
 {
     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.
-    bool invalid_ : 1;
+    uint32_t invalid_ : 1;
 
   public:
     // Default constructor makes an invalid immediate.
     Imm8mData()
       : data_(0xff), rot_(0xf), buff_(0), invalid_(true)
     { }
 
     Imm8mData(uint32_t data, uint32_t rot)
@@ -599,17 +599,17 @@ class RIS
 
     uint32_t encode() const {
         return shiftAmount_;
     }
 };
 
 class RRS
 {
-    bool mustZero_ : 1;
+    uint32_t mustZero_ : 1;
     // The register that holds the shift amount.
     uint32_t rs_ : 4;
 
   public:
     explicit RRS(uint32_t rs)
       : rs_(rs)
     {
         MOZ_ASSERT(rs_ == rs);
@@ -627,17 +627,17 @@ class Operand;
 
 class Operand2
 {
     friend class Operand;
     friend class MacroAssemblerARM;
     friend class InstALU;
 
     uint32_t oper_ : 31;
-    bool invalid_ : 1;
+    uint32_t invalid_ : 1;
 
   protected:
     explicit Operand2(datastore::Imm8mData base)
       : oper_(base.invalid() ? -1 : (base.encode() | uint32_t(IsImmOp2))),
         invalid_(base.invalid())
     { }
 
     explicit Operand2(datastore::Reg base)
@@ -950,17 +950,17 @@ class VFPImm
     explicit VFPImm(uint32_t topWordOfDouble);
 
     static const VFPImm One;
 
     uint32_t encode() const {
         return data_;
     }
     bool isValid() const {
-        return data_ != -1U;
+        return data_ != (~0U);
     }
 };
 
 // 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
 {
@@ -1042,79 +1042,86 @@ class Operand
   public:
     enum class Tag : uint8_t {
         OP2,
         MEM,
         FOP
     };
 
   private:
-    Tag tag_ : 8;
+    uint32_t tag_ : 8;
     uint32_t reg_ : 5;
     int32_t offset_;
 
+  protected:
+    Operand(Tag tag, uint32_t regCode, int32_t offset)
+      : tag_(static_cast<uint32_t>(tag)),
+        reg_(regCode),
+        offset_(offset)
+    { }
+
   public:
     explicit Operand(Register reg)
-      : tag_(Tag::OP2), reg_(reg.code())
+      : Operand(Tag::OP2, reg.code(), 0)
     { }
 
     explicit Operand(FloatRegister freg)
-      : tag_(Tag::FOP), reg_(freg.code())
+      : Operand(Tag::FOP, freg.code(), 0)
     { }
 
     explicit Operand(Register base, Imm32 off)
-      : tag_(Tag::MEM), reg_(base.code()), offset_(off.value)
+      : Operand(Tag::MEM, base.code(), off.value)
     { }
 
     explicit Operand(Register base, int32_t off)
-      : tag_(Tag::MEM), reg_(base.code()), offset_(off)
+      : Operand(Tag::MEM, base.code(), off)
     { }
 
     explicit Operand(const Address& addr)
-      : tag_(Tag::MEM), reg_(addr.base.code()), offset_(addr.offset)
+      : Operand(Tag::MEM, addr.base.code(), addr.offset)
     { }
 
   public:
     Tag tag() const {
-        return tag_;
+        return static_cast<Tag>(tag_);
     }
 
     Operand2 toOp2() const {
-        MOZ_ASSERT(tag_ == Tag::OP2);
+        MOZ_ASSERT(tag() == Tag::OP2);
         return O2Reg(Register::FromCode(reg_));
     }
 
     Register toReg() const {
-        MOZ_ASSERT(tag_ == Tag::OP2);
+        MOZ_ASSERT(tag() == Tag::OP2);
         return Register::FromCode(reg_);
     }
 
     Address toAddress() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return Address(Register::FromCode(reg_), offset_);
     }
     int32_t disp() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return offset_;
     }
 
     int32_t base() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return reg_;
     }
     Register baseReg() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return Register::FromCode(reg_);
     }
     DTRAddr toDTRAddr() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return DTRAddr(baseReg(), DtrOffImm(offset_));
     }
     VFPAddr toVFPAddr() const {
-        MOZ_ASSERT(tag_ == Tag::MEM);
+        MOZ_ASSERT(tag() == Tag::MEM);
         return VFPAddr(baseReg(), VFPOffImm(offset_));
     }
 };
 
 inline Imm32
 Imm64::firstHalf() const
 {
     return low();
@@ -1138,17 +1145,17 @@ PatchBackedge(CodeLocationJump& jump_, C
 
 class Assembler;
 typedef js::jit::AssemblerBufferWithConstantPools<1024, 4, Instruction, Assembler> ARMBuffer;
 
 class Assembler : public AssemblerShared
 {
   public:
     // ARM conditional constants:
-    enum ARMCondition {
+    enum ARMCondition : uint32_t {
         EQ = 0x00000000, // Zero
         NE = 0x10000000, // Non-zero
         CS = 0x20000000,
         CC = 0x30000000,
         MI = 0x40000000,
         PL = 0x50000000,
         VS = 0x60000000,
         VC = 0x70000000,
@@ -1156,17 +1163,17 @@ class Assembler : public AssemblerShared
         LS = 0x90000000,
         GE = 0xa0000000,
         LT = 0xb0000000,
         GT = 0xc0000000,
         LE = 0xd0000000,
         AL = 0xe0000000
     };
 
-    enum Condition {
+    enum Condition : uint32_t {
         Equal = EQ,
         NotEqual = NE,
         Above = HI,
         AboveOrEqual = CS,
         Below = CC,
         BelowOrEqual = LS,
         GreaterThan = GT,
         GreaterThanOrEqual = GE,
@@ -1195,17 +1202,17 @@ class Assembler : public AssemblerShared
         VFP_LessThan = CC // MI is valid too.
     };
 
     // Bit set when a DoubleCondition does not map to a single ARM condition.
     // The macro assembler has to special-case these conditions, or else
     // ConditionFromDoubleCondition will complain.
     static const int DoubleConditionBitSpecial = 0x1;
 
-    enum DoubleCondition {
+    enum DoubleCondition : uint32_t {
         // These conditions will only evaluate to true if the comparison is
         // ordered - i.e. neither operand is NaN.
         DoubleOrdered = VFP_NotUnordered,
         DoubleEqual = VFP_Equal,
         DoubleNotEqual = VFP_NotEqualOrUnordered | DoubleConditionBitSpecial,
         DoubleGreaterThan = VFP_GreaterThan,
         DoubleGreaterThanOrEqual = VFP_GreaterThanOrEqual,
         DoubleLessThan = VFP_LessThan,
--- a/js/src/jit/arm/CodeGenerator-arm.cpp
+++ b/js/src/jit/arm/CodeGenerator-arm.cpp
@@ -1903,21 +1903,21 @@ CodeGenerator::visitWasmReinterpret(LWas
     MOZ_ASSERT(gen->compilingWasm());
     MWasmReinterpret* ins = lir->mir();
 
     MIRType to = ins->type();
     DebugOnly<MIRType> from = ins->input()->type();
 
     switch (to) {
       case MIRType::Int32:
-        MOZ_ASSERT(from == MIRType::Float32);
+        MOZ_ASSERT(static_cast<MIRType>(from) == MIRType::Float32);
         masm.ma_vxfer(ToFloatRegister(lir->input()), ToRegister(lir->output()));
         break;
       case MIRType::Float32:
-        MOZ_ASSERT(from == MIRType::Int32);
+        MOZ_ASSERT(static_cast<MIRType>(from) == MIRType::Int32);
         masm.ma_vxfer(ToRegister(lir->input()), ToFloatRegister(lir->output()));
         break;
       case MIRType::Double:
       case MIRType::Int64:
         MOZ_CRASH("not handled by this LIR opcode");
       default:
         MOZ_CRASH("unexpected WasmReinterpret");
     }
--- a/js/src/jit/arm/Simulator-arm.cpp
+++ b/js/src/jit/arm/Simulator-arm.cpp
@@ -867,17 +867,21 @@ ArmDebugger::debug()
 
                     prev = cur;
                     cur += dasm.InstructionDecode(buffer, cur);
                     printf("  0x%08x  %s\n", reinterpret_cast<uint32_t>(prev), buffer.start());
                 }
 #endif
             } else if (strcmp(cmd, "gdb") == 0) {
                 printf("relinquishing control to gdb\n");
+#ifdef _MSC_VER
+                __debugbreak();
+#else
                 asm("int $3");
+#endif
                 printf("regaining control from gdb\n");
             } else if (strcmp(cmd, "break") == 0) {
                 if (argc == 2) {
                     int32_t value;
                     if (getValue(arg1, &value)) {
                         if (!setBreakpoint(reinterpret_cast<SimInstruction*>(value)))
                             printf("setting breakpoint failed\n");
                     } else {
--- a/js/src/jit/arm/Trampoline-arm.cpp
+++ b/js/src/jit/arm/Trampoline-arm.cpp
@@ -258,18 +258,20 @@ JitRuntime::generateEnterJIT(JSContext* 
         masm.mov(sp, framePtr);
 
 #ifdef XP_WIN
         // Can't push large frames blindly on windows. Touch frame memory
         // incrementally.
         masm.ma_lsl(Imm32(3), numStackValues, scratch);
         masm.subPtr(scratch, framePtr);
         {
-            masm.ma_sub(sp, Imm32(WINDOWS_BIG_FRAME_TOUCH_INCREMENT), scratch);
-
+            ScratchRegisterScope asmScratch(masm);
+            masm.ma_sub(sp, Imm32(WINDOWS_BIG_FRAME_TOUCH_INCREMENT), scratch, asmScratch);
+        }
+        {
             Label touchFrameLoop;
             Label touchFrameLoopEnd;
             masm.bind(&touchFrameLoop);
             masm.branchPtr(Assembler::Below, scratch, framePtr, &touchFrameLoopEnd);
             masm.store32(Imm32(0), Address(scratch, 0));
             masm.subPtr(Imm32(WINDOWS_BIG_FRAME_TOUCH_INCREMENT), scratch);
             masm.jump(&touchFrameLoop);
             masm.bind(&touchFrameLoopEnd);