fixup! JitX64/RegAlloc: Rename member functions to (Lock|Bind)ArmFor(Read|ReadWrite|Write).

src/core/arm/jit_x64/instructions/data_processing.cpp

@@ -12,24 +12,23 @@ using namespace Gen;
 
 void JitX64::CompileDataProcessingHelper(ArmReg Rn_index, ArmReg Rd_index, std::function<void(X64Reg)> body) {
     if (Rn_index == 15) {
-        X64Reg Rd = reg_alloc.WriteOnlyLockArm(Rd_index);
+        X64Reg Rd = reg_alloc.BindArmForWrite(Rd_index);
-        code->MOV(32, R(Rd), Imm32(GetReg15Value()));
 
+        code->MOV(32, R(Rd), Imm32(GetReg15Value()));
         body(Rd);
 
         reg_alloc.UnlockArm(Rd_index);
     } else if (Rn_index == Rd_index) { // Note: Rd_index cannot possibly be 15 in this case.
-        X64Reg Rd = reg_alloc.LoadAndLockArm(Rd_index);
+        X64Reg Rd = reg_alloc.BindArmForReadWrite(Rd_index);
-        reg_alloc.MarkDirty(Rd_index);
 
         body(Rd);
 
         reg_alloc.UnlockArm(Rd_index);
     } else {
-        X64Reg Rd = reg_alloc.WriteOnlyLockArm(Rd_index);
+        X64Reg Rd = reg_alloc.BindArmForWrite(Rd_index);
-        reg_alloc.LockArm(Rn_index);
+        OpArg Rn = reg_alloc.LockArmForRead(Rn_index);
-        code->MOV(32, R(Rd), reg_alloc.ArmR(Rn_index));
 
+        code->MOV(32, R(Rd), Rn);
         body(Rd);
 
         reg_alloc.UnlockArm(Rn_index);
@@ -39,7 +38,7 @@ void JitX64::CompileDataProcessingHelper(ArmReg Rn_index, ArmReg Rd_index, std::
 
 void JitX64::CompileDataProcessingHelper_Reverse(ArmReg Rn_index, ArmReg Rd_index, std::function<void(X64Reg)> body) {
     if (Rd_index != Rn_index) {
-        X64Reg Rd = reg_alloc.WriteOnlyLockArm(Rd_index);
+        X64Reg Rd = reg_alloc.BindArmForWrite(Rd_index);
 
         body(Rd);
 
@@ -51,8 +50,8 @@ void JitX64::CompileDataProcessingHelper_Reverse(ArmReg Rn_index, ArmReg Rd_inde
 
         if (Rd_index != 15) {
             // TODO: Efficiency: Could implement this as a register rebind instead of needing to MOV.
-            reg_alloc.LockAndDirtyArm(Rd_index);
+            OpArg Rd = reg_alloc.LockArmForReadWrite(Rd_index);
-            code->MOV(32, reg_alloc.ArmR(Rd_index), R(tmp));
+            code->MOV(32, Rd, R(tmp));
             reg_alloc.UnlockArm(Rd_index);
         } else {
             code->MOV(32, MJitStateArmPC(), R(tmp));
@@ -190,18 +189,14 @@ void JitX64::MOV_imm(Cond cond, bool S, ArmReg Rd_index, int rotate, ArmImm8 imm
 
     u32 immediate = rotr(imm8, rotate * 2);
 
-    if (Rd_index != 15) {
+    Gen::OpArg Rd = reg_alloc.LockArmForWrite(Rd_index);
-        reg_alloc.LockAndDirtyArm(Rd_index);
+    code->MOV(32, Rd, Imm32(immediate));
-        code->MOV(32, reg_alloc.ArmR(Rd_index), Imm32(immediate));
     reg_alloc.UnlockArm(Rd_index);
-    } else {
-        code->MOV(32, MJitStateArmPC(), Imm32(immediate));
-    }
 
     if (S) {
         cond_manager.FlagsDirty();
-        reg_alloc.LockArm(Rd_index);
+        Gen::OpArg Rd = reg_alloc.LockArmForRead(Rd_index);
-        code->CMP(32, reg_alloc.ArmR(Rd_index), Imm32(0));
+        code->CMP(32, Rd, Imm32(0));
         reg_alloc.UnlockArm(Rd_index);
         UpdateFlagsZN();
         if (rotate != 0) {
@@ -222,18 +217,14 @@ void JitX64::MVN_imm(Cond cond, bool S, ArmReg Rd_index, int rotate, ArmImm8 imm
 
     u32 immediate = rotr(imm8, rotate * 2);
 
-    if (Rd_index != 15) {
+    Gen::OpArg Rd = reg_alloc.LockArmForWrite(Rd_index);
-        reg_alloc.LockAndDirtyArm(Rd_index);
+    code->MOV(32, Rd, Imm32(~immediate));
-        code->MOV(32, reg_alloc.ArmR(Rd_index), Imm32(~immediate));
     reg_alloc.UnlockArm(Rd_index);
-    } else {
-        code->MOV(32, MJitStateArmPC(), Imm32(~immediate));
-    }
 
     if (S) {
         cond_manager.FlagsDirty();
-        reg_alloc.LockArm(Rd_index);
+        Gen::OpArg Rd = reg_alloc.LockArmForRead(Rd_index);
-        code->CMP(32, reg_alloc.ArmR(Rd_index), Imm32(0));
+        code->CMP(32, Rd, Imm32(0));
         reg_alloc.UnlockArm(Rd_index);
         UpdateFlagsZN();
         if (rotate != 0) {
@@ -285,8 +276,8 @@ void JitX64::RSB_imm(Cond cond, bool S, ArmReg Rn_index, ArmReg Rd_index, int ro
         if (Rn_index == 15) {
             code->SUB(32, R(Rd), Imm32(GetReg15Value()));
         } else {
-            reg_alloc.LockArm(Rn_index);
+            Gen::OpArg Rn = reg_alloc.LockArmForRead(Rn_index);
-            code->SUB(32, R(Rd), reg_alloc.ArmR(Rn_index));
+            code->SUB(32, R(Rd), Rn);
             reg_alloc.UnlockArm(Rn_index);
         }
     });
@@ -319,8 +310,8 @@ void JitX64::RSC_imm(Cond cond, bool S, ArmReg Rn_index, ArmReg Rd_index, int ro
         if (Rn_index == 15) {
             code->SBB(32, R(Rd), Imm32(GetReg15Value()));
         } else {
-            reg_alloc.LockArm(Rn_index);
+            Gen::OpArg Rn = reg_alloc.LockArmForRead(Rn_index);
-            code->SBB(32, R(Rd), reg_alloc.ArmR(Rn_index));
+            code->SBB(32, R(Rd), Rn);
             reg_alloc.UnlockArm(Rn_index);
         }
     });
@@ -392,19 +383,19 @@ void JitX64::TEQ_imm(Cond cond, ArmReg Rn_index, int rotate, ArmImm8 imm8) {
 
     u32 immediate = rotr(imm8, rotate * 2);
 
-    X64Reg Rn = reg_alloc.AllocTemp();
+    X64Reg Rn_tmp = reg_alloc.AllocTemp();
 
     if (Rn_index == 15) {
-        code->MOV(32, R(Rn), Imm32(GetReg15Value()));
+        code->MOV(32, R(Rn_tmp), Imm32(GetReg15Value()));
     } else {
-        reg_alloc.LockArm(Rn_index);
+        Gen::OpArg Rn_real = reg_alloc.LockArmForRead(Rn_index);
-        code->MOV(32, R(Rn), reg_alloc.ArmR(Rn_index));
+        code->MOV(32, R(Rn_tmp), Rn_real);
         reg_alloc.UnlockArm(Rn_index);
     }
 
-    code->XOR(32, R(Rn), Imm32(immediate));
+    code->XOR(32, R(Rn_tmp), Imm32(immediate));
 
-    reg_alloc.UnlockTemp(Rn);
+    reg_alloc.UnlockTemp(Rn_tmp);
 
     cond_manager.FlagsDirty();
     UpdateFlagsZN();
@@ -428,7 +419,7 @@ void JitX64::TST_imm(Cond cond, ArmReg Rn_index, int rotate, ArmImm8 imm8) {
         Rn = reg_alloc.AllocTemp();
         code->MOV(32, R(Rn), Imm32(GetReg15Value()));
     } else {
-        Rn = reg_alloc.LoadAndLockArm(Rn_index);
+        Rn = reg_alloc.BindArmForRead(Rn_index);
     }
 
     code->TEST(32, R(Rn), Imm32(immediate));

src/core/arm/jit_x64/reg_alloc.cpp

@@ -130,7 +130,7 @@ void RegAlloc::FlushArm(ArmReg arm_reg) {
     arm_state.location = MJitStateCpuReg(arm_reg);
 }
 
-void RegAlloc::LockArm(ArmReg arm_reg) {
+Gen::OpArg RegAlloc::LockArmForRead(ArmReg arm_reg) {
     ASSERT(arm_reg >= 0 && arm_reg <= 14); // Not valid for R15 (cannot read from it)
 
     ArmState& arm_state = arm_gpr[arm_reg];
@@ -147,17 +147,30 @@ void RegAlloc::LockArm(ArmReg arm_reg) {
 
         x64_state.locked = true;
     }
+
+    return arm_state.location;
 }
 
-void RegAlloc::LockAndDirtyArm(ArmReg arm_reg) {
+Gen::OpArg RegAlloc::LockArmForWrite(ArmReg arm_reg) {
-    ASSERT(arm_reg >= 0 && arm_reg <= 14); // Not valid for R15 (cannot read from it)
+    ASSERT(arm_reg >= 0 && arm_reg <= 15); // Valid for R15 (write-only)
 
     ArmState& arm_state = arm_gpr[arm_reg];
 
-    LockArm(arm_reg);
+    ASSERT(!arm_state.locked);
+    arm_state.locked = true;
+
     if (arm_state.location.IsSimpleReg()) {
-        MarkDirty(arm_reg);
+        Gen::X64Reg x64_reg = arm_state.location.GetSimpleReg();
+        X64State& x64_state = x64_gpr[x64_reg_to_index.at(x64_reg)];
+        ASSERT(!x64_state.locked);
+        ASSERT(x64_state.state == X64State::State::CleanArmReg || x64_state.state == X64State::State::DirtyArmReg);
+        ASSERT(x64_state.arm_reg == arm_reg);
+
+        x64_state.locked = true;
+        x64_state.state = X64State::State::DirtyArmReg;
     }
+
+    return arm_state.location;
 }
 
 Gen::X64Reg RegAlloc::BindArmToX64(ArmReg arm_reg, bool load) {
@@ -191,7 +204,7 @@ Gen::X64Reg RegAlloc::BindArmToX64(ArmReg arm_reg, bool load) {
     return x64_reg;
 }
 
-Gen::X64Reg RegAlloc::LoadAndLockArm(ArmReg arm_reg) {
+Gen::X64Reg RegAlloc::BindArmForRead(ArmReg arm_reg) {
     ASSERT(arm_reg >= 0 && arm_reg <= 14); // Not valid for R15 (cannot read from it)
 
     const Gen::X64Reg x64_reg = BindArmToX64(arm_reg, true);
@@ -199,7 +212,7 @@ Gen::X64Reg RegAlloc::LoadAndLockArm(ArmReg arm_reg) {
     return x64_reg;
 }
 
-Gen::X64Reg RegAlloc::WriteOnlyLockArm(ArmReg arm_reg) {
+Gen::X64Reg RegAlloc::BindArmForWrite(ArmReg arm_reg) {
     ASSERT(arm_reg >= 0 && arm_reg <= 15); // Valid for R15 (we're not reading from it)
 
     const Gen::X64Reg x64_reg = BindArmToX64(arm_reg, false);
@@ -228,16 +241,6 @@ void RegAlloc::UnlockArm(ArmReg arm_reg) {
     }
 }
 
-void RegAlloc::FlushAllArm() {
-    for (ArmReg arm_reg = 0; arm_reg < arm_gpr.size(); arm_reg++) {
-        ArmState& arm_state = arm_gpr[arm_reg];
-        ASSERT(!arm_state.locked);
-        if (arm_state.location.IsSimpleReg()) {
-            FlushArm(arm_reg);
-        }
-    }
-}
-
 void RegAlloc::MarkDirty(ArmReg arm_reg) {
     const ArmState& arm_state = arm_gpr[arm_reg];
 
@@ -277,12 +280,10 @@ Gen::X64Reg RegAlloc::GetX64For(ArmReg arm_reg) {
     return x64_reg;
 }
 
-Gen::OpArg RegAlloc::ArmR(ArmReg arm_reg) {
+bool RegAlloc::IsBoundToX64(ArmReg arm_reg) {
     const ArmState& arm_state = arm_gpr[arm_reg];
 
-    ASSERT(arm_state.locked);
+    return arm_state.location.IsSimpleReg();
-
-    return arm_state.location;
 }
 
 Gen::X64Reg RegAlloc::AllocTemp() {

src/core/arm/jit_x64/reg_alloc.h

@@ -23,8 +23,7 @@ class RegAlloc final {
 private:
     struct ArmState {
         /**
-         * Where is the current value of this register?
+         * Where is the current value of this register? There are two cases:
-         * There are two options:
          * - In an x64 register, in which case location.IsSimpleReg() == true.
          * - In memory in ARMul_State, in which case location == MJitStateCpuReg(arm_reg).
          */
@@ -36,15 +35,15 @@ private:
      * Possible states of X64State:
      *
      * Free (locked must be false):    This x64 reg is free to be allocated for any purpose.
-     * Temp (locked must be true):     This x64 reg is being used as a temporary for a calculation.
+     * Temp (locked must be true):     This x64 reg is being used as a temporary in a calculation.
-     * DirtyArmReg (arm_reg is valid): This x64 reg holds the value of an ARM reg.
+     * DirtyArmReg (arm_reg is valid): This x64 reg is bound to an ARM reg.
-     *                                 It's value has been changed since being loaded from memory.
+     *                                 It is marked as dirty (value has changed).
-     *                                 This value must be flushed back to memory.
+     *                                 This value MUST be flushed back to memory.
-     * CleanArmReg (arm_reg is valid): This x64 reg holds the value of an ARM reg.
+     * CleanArmReg (arm_reg is valid): This x64 reg is bound to an ARM reg.
-     *                                 It's value has not been changed from when it's been loaded from memory.
+     *                                 It hasn't been written to (i.e.: value is still the same as the in-memory version).
-     *                                 This value may be discarded.
+     *                                 This value WILL NOT be flushed back to memory.
-     * MemoryMap:                      This value holds a pointer to the ARM page table (current unimplemented).
+     * MemoryMap:                      This value holds a pointer to the ARM page table (currently unimplemented).
-     * UserManuallyLocked:             User has called LockX64 on this register. Must call UnlockX64 to unlock.
+     * UserManuallyLocked:             User has called LockX64 on this register. User must call UnlockX64 to unlock.
      */
     struct X64State {
         enum class State {
@@ -69,7 +68,7 @@ private:
 public:
     RegAlloc() { Init(nullptr); }
 
-    /// Initialise register allocator (call compiling a basic block as it resets internal state)
+    /// Initialise register allocator (call before compiling a basic block as it resets internal state)
     void Init(Gen::XEmitter* emitter);
 
     // Manually load and unlock x64 registers:
@@ -86,66 +85,56 @@ public:
     // Working with ARM registers:
 
     /**
-     * Locks an ARM register so it doesn't move.
+     * Locks an ARM register so it doesn't move; ARM reg may either be in a x64 reg or in memory.
-     * ARM reg may either be in a x64 reg or in memory.
+     * We're going to read from it only.
-     * We're going to read from it only. (Use ArmR to do so.)
      * Call UnlockArm when done.
      */
-    void LockArm(ArmReg arm_reg);
+    Gen::OpArg LockArmForRead(ArmReg arm_reg);
     /**
-     * Locks an ARM register so it doesn't move.
+     * Locks an ARM register so it doesn't move; ARM reg may either be in a x64 reg or in memory.
-     * ARM reg may either be in a x64 reg or in memory.
+     * We're going to read and/or write to it.
-     * We're going to read and/or write to it. (Use ArmR to do so.)
      * Call UnlockArm when done.
      */
-    void LockAndDirtyArm(ArmReg arm_reg);
+    Gen::OpArg LockArmForReadWrite(ArmReg arm_reg) {
-    /// Gets the current location of this ARM register. (ASSERTS IF NOT LOCKED!)
+        Gen::OpArg ret = LockArmForRead(arm_reg);
-    Gen::OpArg ArmR(ArmReg arm_reg);
+        if (IsBoundToX64(arm_reg)) {
+            MarkDirty(arm_reg);
+        }
+        return ret;
+    }
+    /**
+     * Locks an ARM register so it doesn't move; ARM reg may either be in a x64 reg or in memory.
+     * We're going to write to it only.
+     * Call UnlockArm when done.
+     */
+    Gen::OpArg LockArmForWrite(ArmReg arm_reg);
 
     /**
-     * Allocates a x64 register for an ARM register and ensure it's value is loaded into it.
+     * Binds an ARM register to a x64 register.
-     * ARM reg is in an x64 reg.
+     * We're going to read from it only.
-     * We're going to read from it only. (Call MarkDirty if you want to write to it.)
      * Call UnlockArm when done.
      */
-    Gen::X64Reg LoadAndLockArm(ArmReg arm_reg);
+    Gen::X64Reg BindArmForRead(ArmReg arm_reg);
     /**
-     * Allocates a x64 register for an ARM register and doesn't bother loading it's value to it.
+     * Binds an ARM register to a x64 register.
-     * ARM reg is in an x64 reg.
+     * We're going to read and/or write to it.
-     * We're going to write to it only. (DO NOT READ, WRITE-ONLY. Also MarkDirty has been called for you.)
      * Call UnlockArm when done.
      */
-    Gen::X64Reg WriteOnlyLockArm(ArmReg arm_reg);
+    Gen::X64Reg BindArmForReadWrite(ArmReg arm_reg) {
+        Gen::X64Reg ret = BindArmForRead(arm_reg);
+        MarkDirty(arm_reg);
+        return ret;
+    }
+    /**
+     * Binds an ARM register to a x64 register.
+     * We're going to write to it only.
+     * Call UnlockArm when done.
+     */
+    Gen::X64Reg BindArmForWrite(ArmReg arm_reg);
 
-    /**
-     * Marks an ARM register as dirty.
-     * If you don't mark something as dirty it won't be flushed back to memory.
-     * May only be called while an ARM register is locked.
-     */
-    void MarkDirty(ArmReg arm_reg);
     /// Unlock ARM register.
     void UnlockArm(ArmReg arm_reg);
 
-    /// Ensures that this ARM register is not in an x64 register.
-    void FlushArm(ArmReg arm_reg);
-
-    /// Flush all ARM registers.
-    void FlushAllArm();
-
-    /**
-     * Flush absolutely everything.
-     * You MUST always flush everything:
-     * - just before a branch occurs
-     * - just before calling into the interpreter
-     * - just before calling a host function
-     * - just before returning to the dispatcher
-     * - just before jumping to a new BB
-     */
-    void FlushEverything();
-
-    /// Gets the x64 register which corresponds to that ARM register. (ASSERTS IF NOT IN A x64 REG OR NOT LOCKED!)
-    Gen::X64Reg GetX64For(ArmReg arm_reg);
-
     // Temporaries:
 
     /// Allocates a temporary register
@@ -165,11 +154,33 @@ public:
     /// Returns the register in which the JitState pointer is stored.
     Gen::X64Reg JitStateReg();
 
+    // Flush:
+
+    /**
+     * Flush absolutely everything.
+     * You MUST always flush everything:
+     * - just before a branch occurs
+     * - just before calling into the interpreter
+     * - just before calling a host function
+     * - just before returning to the dispatcher
+     * - just before jumping to a new BB
+     * If unsure, flush. (Only cost is performance.)
+     */
+    void FlushEverything();
+
     // Debug:
 
     void AssertNoLocked();
 
 private:
+    /// INTERNAL: Gets the x64 register this ArmReg is currently bound to.
+    Gen::X64Reg GetX64For(ArmReg arm_reg);
+    /// INTERNAL: Ensures that this ARM register is not in an x64 register.
+    void FlushArm(ArmReg arm_reg);
+    /// INTERNAL: Is this ARM register currently in an x64 register?
+    bool IsBoundToX64(ArmReg arm_reg);
+    /// INTERNAL: Marks register as dirty. Ensures that it is written back to memory if it's in a x64 register.
+    void MarkDirty(ArmReg arm_reg);
     /// INTERNAL: Allocates a register that is free. Flushes registers that are not locked if necessary.
     Gen::X64Reg AllocReg();
     /// INTERNAL: Binds an ARM register to an X64 register. Retrieves binding if already bound.

src/tests/core/arm/jit_x64/fuzz_arm_data_processing.cpp

@@ -91,7 +91,7 @@ void FuzzJit(const int instruction_count, const int run_count, const std::functi
             Memory::Write32(i * 4, inst);
         }
 
-        Memory::Write32(instruction_count * 4, 0b0011001000001111000000000000);
+        Memory::Write32(instruction_count * 4, 0xEAFFFFFE); // b +#0 // busy wait loop
 
         interp.ExecuteInstructions(instruction_count);
         jit.ExecuteInstructions(instruction_count);
@@ -223,7 +223,7 @@ TEST_CASE("Fuzz ARM data processing instructions", "[JitX64]") {
     }
 
     SECTION("long blocks") {
-        FuzzJit(1024, 200, instruction_select_without_R15);
+        FuzzJit(1024, 50, instruction_select_without_R15);
     }
 
     auto instruction_select_only_R15 = [&]() -> u32 {