From 25201712dbbf98ed29bfcc0864d5d5f21b61680d Mon Sep 17 00:00:00 2001 From: MerryMage Date: Tue, 22 Mar 2016 17:38:22 +0000 Subject: [PATCH] fixup! JitX64/RegAlloc: Rename member functions to (Lock|Bind)ArmFor(Read|ReadWrite|Write). --- .../jit_x64/instructions/data_processing.cpp | 69 +++++----- src/core/arm/jit_x64/reg_alloc.cpp | 43 +++--- src/core/arm/jit_x64/reg_alloc.h | 123 ++++++++++-------- .../arm/jit_x64/fuzz_arm_data_processing.cpp | 4 +- 4 files changed, 121 insertions(+), 118 deletions(-) diff --git a/src/core/arm/jit_x64/instructions/data_processing.cpp b/src/core/arm/jit_x64/instructions/data_processing.cpp index c8916c123..5b88b0af3 100644 --- a/src/core/arm/jit_x64/instructions/data_processing.cpp +++ b/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 body) { if (Rn_index == 15) { - X64Reg Rd = reg_alloc.WriteOnlyLockArm(Rd_index); - code->MOV(32, R(Rd), Imm32(GetReg15Value())); + X64Reg Rd = reg_alloc.BindArmForWrite(Rd_index); + 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); - reg_alloc.MarkDirty(Rd_index); + X64Reg Rd = reg_alloc.BindArmForReadWrite(Rd_index); body(Rd); reg_alloc.UnlockArm(Rd_index); } else { - X64Reg Rd = reg_alloc.WriteOnlyLockArm(Rd_index); - reg_alloc.LockArm(Rn_index); - code->MOV(32, R(Rd), reg_alloc.ArmR(Rn_index)); + X64Reg Rd = reg_alloc.BindArmForWrite(Rd_index); + OpArg Rn = reg_alloc.LockArmForRead(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 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); - code->MOV(32, reg_alloc.ArmR(Rd_index), R(tmp)); + OpArg Rd = reg_alloc.LockArmForReadWrite(Rd_index); + 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) { - reg_alloc.LockAndDirtyArm(Rd_index); - code->MOV(32, reg_alloc.ArmR(Rd_index), Imm32(immediate)); - reg_alloc.UnlockArm(Rd_index); - } else { - code->MOV(32, MJitStateArmPC(), Imm32(immediate)); - } + Gen::OpArg Rd = reg_alloc.LockArmForWrite(Rd_index); + code->MOV(32, Rd, Imm32(immediate)); + reg_alloc.UnlockArm(Rd_index); if (S) { cond_manager.FlagsDirty(); - reg_alloc.LockArm(Rd_index); - code->CMP(32, reg_alloc.ArmR(Rd_index), Imm32(0)); + Gen::OpArg Rd = reg_alloc.LockArmForRead(Rd_index); + 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) { - reg_alloc.LockAndDirtyArm(Rd_index); - code->MOV(32, reg_alloc.ArmR(Rd_index), Imm32(~immediate)); - reg_alloc.UnlockArm(Rd_index); - } else { - code->MOV(32, MJitStateArmPC(), Imm32(~immediate)); - } + Gen::OpArg Rd = reg_alloc.LockArmForWrite(Rd_index); + code->MOV(32, Rd, Imm32(~immediate)); + reg_alloc.UnlockArm(Rd_index); if (S) { cond_manager.FlagsDirty(); - reg_alloc.LockArm(Rd_index); - code->CMP(32, reg_alloc.ArmR(Rd_index), Imm32(0)); + Gen::OpArg Rd = reg_alloc.LockArmForRead(Rd_index); + 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); - code->SUB(32, R(Rd), reg_alloc.ArmR(Rn_index)); + Gen::OpArg Rn = reg_alloc.LockArmForRead(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); - code->SBB(32, R(Rd), reg_alloc.ArmR(Rn_index)); + Gen::OpArg Rn = reg_alloc.LockArmForRead(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); - code->MOV(32, R(Rn), reg_alloc.ArmR(Rn_index)); + Gen::OpArg Rn_real = reg_alloc.LockArmForRead(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)); diff --git a/src/core/arm/jit_x64/reg_alloc.cpp b/src/core/arm/jit_x64/reg_alloc.cpp index 3bc7c779b..556957999 100644 --- a/src/core/arm/jit_x64/reg_alloc.cpp +++ b/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) { - ASSERT(arm_reg >= 0 && arm_reg <= 14); // Not valid for R15 (cannot read from it) +Gen::OpArg RegAlloc::LockArmForWrite(ArmReg arm_reg) { + 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; + return arm_state.location.IsSimpleReg(); } Gen::X64Reg RegAlloc::AllocTemp() { diff --git a/src/core/arm/jit_x64/reg_alloc.h b/src/core/arm/jit_x64/reg_alloc.h index 7687f6e0b..8d5c113a2 100644 --- a/src/core/arm/jit_x64/reg_alloc.h +++ b/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? - * There are two options: + * Where is the current value of this register? There are two cases: * - 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. - * DirtyArmReg (arm_reg is valid): This x64 reg holds the value of an ARM reg. - * It's value has been changed since being loaded from memory. - * This value must be flushed back to memory. - * CleanArmReg (arm_reg is valid): This x64 reg holds the value of an ARM reg. - * It's value has not been changed from when it's been loaded from memory. - * This value may be discarded. - * MemoryMap: This value holds a pointer to the ARM page table (current unimplemented). - * UserManuallyLocked: User has called LockX64 on this register. Must call UnlockX64 to unlock. + * 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 is bound to an ARM reg. + * It is marked as dirty (value has changed). + * This value MUST be flushed back to memory. + * CleanArmReg (arm_reg is valid): This x64 reg is bound to an ARM reg. + * It hasn't been written to (i.e.: value is still the same as the in-memory version). + * This value WILL NOT be flushed back to memory. + * MemoryMap: This value holds a pointer to the ARM page table (currently unimplemented). + * 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. - * ARM reg may either be in a x64 reg or in memory. - * We're going to read from it only. (Use ArmR to do so.) + * 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 read from it only. * Call UnlockArm when done. */ - void LockArm(ArmReg arm_reg); + Gen::OpArg LockArmForRead(ArmReg arm_reg); /** - * 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 read and/or write to it. (Use ArmR to do so.) + * 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 read and/or write to it. * Call UnlockArm when done. */ - void LockAndDirtyArm(ArmReg arm_reg); - /// Gets the current location of this ARM register. (ASSERTS IF NOT LOCKED!) - Gen::OpArg ArmR(ArmReg arm_reg); + Gen::OpArg LockArmForReadWrite(ArmReg arm_reg) { + Gen::OpArg ret = LockArmForRead(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. - * ARM reg is in an x64 reg. - * We're going to read from it only. (Call MarkDirty if you want to write to it.) + * Binds an ARM register to a x64 register. + * We're going to read from it only. * 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. - * ARM reg is in an x64 reg. - * We're going to write to it only. (DO NOT READ, WRITE-ONLY. Also MarkDirty has been called for you.) + * Binds an ARM register to a x64 register. + * We're going to read and/or write to it. * 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. diff --git a/src/tests/core/arm/jit_x64/fuzz_arm_data_processing.cpp b/src/tests/core/arm/jit_x64/fuzz_arm_data_processing.cpp index 498c09747..5fec98107 100644 --- a/src/tests/core/arm/jit_x64/fuzz_arm_data_processing.cpp +++ b/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 {