Merge pull request #1003 from lioncash/armcruft

dyncom: Minor cleanups.

src/core/arm/dyncom/arm_dyncom_dec.cpp

@@ -5,7 +5,7 @@
 #include "core/arm/dyncom/arm_dyncom_dec.h"
 #include "core/arm/skyeye_common/armsupp.h"
 
-const ISEITEM arm_instruction[] = {
+const InstructionSetEncodingItem arm_instruction[] = {
     { "vmla", 4, ARMVFP2,      { 23, 27, 0x1C, 20, 21, 0x0, 9, 11, 0x5, 4, 4, 0 }},
     { "vmls", 7, ARMVFP2,      { 28, 31, 0xF, 25, 27, 0x1, 23, 23, 1, 11, 11, 0, 8, 9, 0x2, 6, 6, 1, 4, 4, 0 }},
     { "vnmla", 4, ARMVFP2,     { 23, 27, 0x1C, 20, 21, 0x1, 9, 11, 0x5, 4, 4, 0 }},
@@ -207,7 +207,7 @@ const ISEITEM arm_instruction[] = {
     { "bbl", 1, 0,         { 25, 27, 0x00000005 }},
 };
 
-const ISEITEM arm_exclusion_code[] = {
+const InstructionSetEncodingItem arm_exclusion_code[] = {
     { "vmla", 0, ARMVFP2,      { 0 }},
     { "vmls", 0, ARMVFP2,      { 0 }},
     { "vnmla", 0, ARMVFP2,     { 0 }},
@@ -414,14 +414,13 @@ const ISEITEM arm_exclusion_code[] = {
     { "invalid", 0, INVALID,     { 0 }}
 };
 
-int decode_arm_instr(u32 instr, s32* idx) {
+ARMDecodeStatus DecodeARMInstruction(u32 instr, s32* idx) {
     int n = 0;
     int base = 0;
-    int ret = DECODE_FAILURE;
+    int instr_slots = sizeof(arm_instruction) / sizeof(InstructionSetEncodingItem);
-    int i = 0;
+    ARMDecodeStatus ret = ARMDecodeStatus::FAILURE;
-    int instr_slots = sizeof(arm_instruction) / sizeof(ISEITEM);
 
-    for (i = 0; i < instr_slots; i++) {
+    for (int i = 0; i < instr_slots; i++) {
         n = arm_instruction[i].attribute_value;
         base = 0;
 
@@ -438,11 +437,11 @@ int decode_arm_instr(u32 instr, s32* idx) {
             n--;
         }
 
-        // All conditions is satisfied.
+        // All conditions are satisfied.
         if (n == 0)
-            ret = DECODE_SUCCESS;
+            ret = ARMDecodeStatus::SUCCESS;
 
-        if (ret == DECODE_SUCCESS) {
+        if (ret == ARMDecodeStatus::SUCCESS) {
             n = arm_exclusion_code[i].attribute_value;
             if (n != 0) {
                 base = 0;
@@ -454,13 +453,13 @@ int decode_arm_instr(u32 instr, s32* idx) {
                     n--;
                 }
 
-                // All conditions is satisfied.
+                // All conditions are satisfied.
                 if (n == 0)
-                    ret = DECODE_FAILURE;
+                    ret = ARMDecodeStatus::FAILURE;
             }
         }
 
-        if (ret == DECODE_SUCCESS) {
+        if (ret == ARMDecodeStatus::SUCCESS) {
             *idx = i;
             return ret;
         }

src/core/arm/dyncom/arm_dyncom_dec.h

@@ -6,22 +6,20 @@
 
 #include "common/common_types.h"
 
-int decode_arm_instr(u32 instr, s32* idx);
+enum class ARMDecodeStatus {
-
+    SUCCESS,
-enum DECODE_STATUS {
+    FAILURE
-    DECODE_SUCCESS,
-    DECODE_FAILURE
 };
 
-struct instruction_set_encoding_item {
+ARMDecodeStatus DecodeARMInstruction(u32 instr, s32* idx);
+
+struct InstructionSetEncodingItem {
     const char *name;
     int attribute_value;
     int version;
     u32 content[21];
 };
 
-typedef struct instruction_set_encoding_item ISEITEM;
-
 // ARM versions
 enum {
     INVALID = 0,
@@ -38,4 +36,4 @@ enum {
     ARMV6K,
 };
 
-extern const ISEITEM arm_instruction[];
+extern const InstructionSetEncodingItem arm_instruction[];

src/core/arm/dyncom/arm_dyncom_interpreter.cpp

@@ -47,27 +47,6 @@ enum {
 
 typedef unsigned int (*shtop_fp_t)(ARMul_State* cpu, unsigned int sht_oper);
 
-// Defines a reservation granule of 2 words, which protects the first 2 words starting at the tag.
-// This is the smallest granule allowed by the v7 spec, and is coincidentally just large enough to
-// support LDR/STREXD.
-static const u32 RESERVATION_GRANULE_MASK = 0xFFFFFFF8;
-
-// Exclusive memory access
-static int exclusive_detect(ARMul_State* state, u32 addr) {
-    if(state->exclusive_tag == (addr & RESERVATION_GRANULE_MASK))
-        return 0;
-    else
-        return -1;
-}
-
-static void add_exclusive_addr(ARMul_State* state, u32 addr){
-    state->exclusive_tag = addr & RESERVATION_GRANULE_MASK;
-}
-
-static void remove_exclusive(ARMul_State* state, u32 addr){
-    state->exclusive_tag = 0xFFFFFFFF;
-}
-
 static int CondPassed(ARMul_State* cpu, unsigned int cond) {
     const u32 NFLAG = cpu->NFlag;
     const u32 ZFLAG = cpu->ZFlag;
@@ -3489,21 +3468,15 @@ enum {
     FETCH_FAILURE
 };
 
-static tdstate decode_thumb_instr(u32 inst, u32 addr, u32* arm_inst, u32* inst_size, ARM_INST_PTR* ptr_inst_base) {
+static ThumbDecodeStatus DecodeThumbInstruction(u32 inst, u32 addr, u32* arm_inst, u32* inst_size, ARM_INST_PTR* ptr_inst_base) {
     // Check if in Thumb mode
-    tdstate ret = thumb_translate (addr, inst, arm_inst, inst_size);
+    ThumbDecodeStatus ret = TranslateThumbInstruction (addr, inst, arm_inst, inst_size);
-    if(ret == t_branch){
+    if (ret == ThumbDecodeStatus::BRANCH) {
-        // TODO: FIXME, endian should be judged
-        u32 tinstr;
-        if((addr & 0x3) != 0)
-            tinstr = inst >> 16;
-        else
-            tinstr = inst & 0xFFFF;
-
         int inst_index;
         int table_length = sizeof(arm_instruction_trans) / sizeof(transop_fp_t);
+        u32 tinstr = GetThumbInstruction(inst, addr);
 
-        switch((tinstr & 0xF800) >> 11){
+        switch ((tinstr & 0xF800) >> 11) {
         case 26:
         case 27:
             if (((tinstr & 0x0F00) != 0x0E00) && ((tinstr & 0x0F00) != 0x0F00)){
@@ -3536,7 +3509,7 @@ static tdstate decode_thumb_instr(u32 inst, u32 addr, u32* arm_inst, u32* inst_s
             *ptr_inst_base = arm_instruction_trans[inst_index](tinstr, inst_index);
             break;
         default:
-            ret = t_undefined;
+            ret = ThumbDecodeStatus::UNDEFINED;
             break;
         }
     }
@@ -3548,10 +3521,6 @@ enum {
     FETCH_EXCEPTION
 };
 
-typedef struct instruction_set_encoding_item ISEITEM;
-
-extern const ISEITEM arm_instruction[];
-
 static int InterpreterTranslate(ARMul_State* cpu, int& bb_start, u32 addr) {
     Common::Profiling::ScopeTimer timer_decode(profile_decode);
 
@@ -3573,20 +3542,19 @@ static int InterpreterTranslate(ARMul_State* cpu, int& bb_start, u32 addr) {
         inst = Memory::Read32(phys_addr & 0xFFFFFFFC);
 
         size++;
-        // If we are in thumb instruction, we will translate one thumb to one corresponding arm instruction
+        // If we are in Thumb mode, we'll translate one Thumb instruction to the corresponding ARM instruction
         if (cpu->TFlag) {
             uint32_t arm_inst;
-            tdstate state = decode_thumb_instr(inst, phys_addr, &arm_inst, &inst_size, &inst_base);
+            ThumbDecodeStatus state = DecodeThumbInstruction(inst, phys_addr, &arm_inst, &inst_size, &inst_base);
 
-            // We have translated the branch instruction of thumb in thumb decoder
+            // We have translated the Thumb branch instruction in the Thumb decoder
-            if(state == t_branch){
+            if (state == ThumbDecodeStatus::BRANCH) {
                 goto translated;
             }
             inst = arm_inst;
         }
 
-        ret = decode_arm_instr(inst, &idx);
+        if (DecodeARMInstruction(inst, &idx) == ARMDecodeStatus::FAILURE) {
-        if (ret == DECODE_FAILURE) {
             std::string disasm = ARM_Disasm::Disassemble(phys_addr, inst);
             LOG_ERROR(Core_ARM11, "Decode failure.\tPC : [0x%x]\tInstruction : %s [%x]", phys_addr, disasm.c_str(), inst);
             LOG_ERROR(Core_ARM11, "cpsr=0x%x, cpu->TFlag=%d, r15=0x%x", cpu->Cpsr, cpu->TFlag, cpu->Reg[15]);
@@ -4174,9 +4142,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
 
     CLREX_INST:
     {
-        remove_exclusive(cpu, 0);
+        cpu->UnsetExclusiveMemoryAddress();
-        cpu->exclusive_state = 0;
-
         cpu->Reg[15] += cpu->GetInstructionSize();
         INC_PC(sizeof(clrex_inst));
         FETCH_INST;
@@ -4543,8 +4509,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int read_addr = RN;
 
-            add_exclusive_addr(cpu, read_addr);
+            cpu->SetExclusiveMemoryAddress(read_addr);
-            cpu->exclusive_state = 1;
 
             RD = cpu->ReadMemory32(read_addr);
             if (inst_cream->Rd == 15) {
@@ -4563,8 +4528,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int read_addr = RN;
 
-            add_exclusive_addr(cpu, read_addr);
+            cpu->SetExclusiveMemoryAddress(read_addr);
-            cpu->exclusive_state = 1;
 
             RD = Memory::Read8(read_addr);
             if (inst_cream->Rd == 15) {
@@ -4583,8 +4547,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int read_addr = RN;
 
-            add_exclusive_addr(cpu, read_addr);
+            cpu->SetExclusiveMemoryAddress(read_addr);
-            cpu->exclusive_state = 1;
 
             RD = cpu->ReadMemory16(read_addr);
             if (inst_cream->Rd == 15) {
@@ -4603,8 +4566,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int read_addr = RN;
 
-            add_exclusive_addr(cpu, read_addr);
+            cpu->SetExclusiveMemoryAddress(read_addr);
-            cpu->exclusive_state = 1;
 
             RD  = cpu->ReadMemory32(read_addr);
             RD2 = cpu->ReadMemory32(read_addr + 4);
@@ -6089,10 +6051,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int write_addr = cpu->Reg[inst_cream->Rn];
 
-            if ((exclusive_detect(cpu, write_addr) == 0) && (cpu->exclusive_state == 1)) {
+            if (cpu->IsExclusiveMemoryAccess(write_addr)) {
-                remove_exclusive(cpu, write_addr);
+                cpu->UnsetExclusiveMemoryAddress();
-                cpu->exclusive_state = 0;
-
                 cpu->WriteMemory32(write_addr, RM);
                 RD = 0;
             } else {
@@ -6111,10 +6071,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int write_addr = cpu->Reg[inst_cream->Rn];
 
-            if ((exclusive_detect(cpu, write_addr) == 0) && (cpu->exclusive_state == 1)) {
+            if (cpu->IsExclusiveMemoryAccess(write_addr)) {
-                remove_exclusive(cpu, write_addr);
+                cpu->UnsetExclusiveMemoryAddress();
-                cpu->exclusive_state = 0;
-
                 Memory::Write8(write_addr, cpu->Reg[inst_cream->Rm]);
                 RD = 0;
             } else {
@@ -6133,9 +6091,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int write_addr = cpu->Reg[inst_cream->Rn];
 
-            if ((exclusive_detect(cpu, write_addr) == 0) && (cpu->exclusive_state == 1)) {
+            if (cpu->IsExclusiveMemoryAccess(write_addr)) {
-                remove_exclusive(cpu, write_addr);
+                cpu->UnsetExclusiveMemoryAddress();
-                cpu->exclusive_state = 0;
 
                 const u32 rt  = cpu->Reg[inst_cream->Rm + 0];
                 const u32 rt2 = cpu->Reg[inst_cream->Rm + 1];
@@ -6165,10 +6122,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) {
             generic_arm_inst* inst_cream = (generic_arm_inst*)inst_base->component;
             unsigned int write_addr = cpu->Reg[inst_cream->Rn];
 
-            if ((exclusive_detect(cpu, write_addr) == 0) && (cpu->exclusive_state == 1)) {
+            if (cpu->IsExclusiveMemoryAccess(write_addr)) {
-                remove_exclusive(cpu, write_addr);
+                cpu->UnsetExclusiveMemoryAddress();
-                cpu->exclusive_state = 0;
-
                 cpu->WriteMemory16(write_addr, RM);
                 RD = 0;
             } else {

src/core/arm/dyncom/arm_dyncom_thumb.cpp

@@ -12,15 +12,9 @@
 // with the following Thumb instruction held in the high 16-bits.  Passing in two Thumb instructions
 // allows easier simulation of the special dual BL instruction.
 
-tdstate thumb_translate(u32 addr, u32 instr, u32* ainstr, u32* inst_size) {
+ThumbDecodeStatus TranslateThumbInstruction(u32 addr, u32 instr, u32* ainstr, u32* inst_size) {
-    tdstate valid = t_uninitialized;
+    ThumbDecodeStatus valid = ThumbDecodeStatus::UNINITIALIZED;
-    u32 tinstr = instr;
+    u32 tinstr = GetThumbInstruction(instr, addr);
-
-    // The endian should be judge here
-    if((addr & 0x3) != 0)
-        tinstr = instr >> 16;
-    else
-        tinstr &= 0xFFFF;
 
     *ainstr = 0xDEADC0DE; // Debugging to catch non updates
 
@@ -357,21 +351,21 @@ tdstate thumb_translate(u32 addr, u32 instr, u32* ainstr, u32* inst_size) {
             else
                 *ainstr |= (tinstr & 0x00FF);
         } else if ((tinstr & 0x0F00) != 0x0E00)
-            valid = t_branch;
+            valid = ThumbDecodeStatus::BRANCH;
         else //  UNDEFINED : cc=1110(AL) uses different format
-            valid = t_undefined;
+            valid = ThumbDecodeStatus::UNDEFINED;
 
         break;
 
     case 28: // B
-        valid = t_branch;
+        valid = ThumbDecodeStatus::BRANCH;
         break;
 
     case 29:
-        if(tinstr & 0x1)
+        if (tinstr & 0x1)
-            valid = t_undefined;
+            valid = ThumbDecodeStatus::UNDEFINED;
         else
-            valid = t_branch;
+            valid = ThumbDecodeStatus::BRANCH;
         break;
 
     case 30: // BL instruction 1
@@ -380,7 +374,7 @@ tdstate thumb_translate(u32 addr, u32 instr, u32* ainstr, u32* inst_size) {
         // simulation simple (from the user perspective) we check if the following instruction is
         // the second half of this BL, and if it is we simulate it immediately
 
-        valid = t_branch;
+        valid = ThumbDecodeStatus::BRANCH;
         break;
 
     case 31: // BL instruction 2
@@ -389,7 +383,7 @@ tdstate thumb_translate(u32 addr, u32 instr, u32* ainstr, u32* inst_size) {
         // ever be matched with the fmt19 "BL instruction 1" instruction. However, we do allow the
         // simulation of it on its own, with undefined results if r14 is not suitably initialised.
 
-        valid = t_branch;
+        valid = ThumbDecodeStatus::BRANCH;
         break;
     }
 

src/core/arm/dyncom/arm_dyncom_thumb.h

@@ -28,20 +28,22 @@
 
 #include "common/common_types.h"
 
-enum tdstate {
+enum class ThumbDecodeStatus {
-    t_undefined,    // Undefined Thumb instruction
+    UNDEFINED,    // Undefined Thumb instruction
-    t_decoded,      // Instruction decoded to ARM equivalent
+    DECODED,      // Instruction decoded to ARM equivalent
-    t_branch,       // Thumb branch (already processed)
+    BRANCH,       // Thumb branch (already processed)
-    t_uninitialized,
+    UNINITIALIZED,
 };
 
-tdstate thumb_translate(u32 addr, u32 instr, u32* ainstr, u32* inst_size);
+// Translates a Thumb mode instruction into its ARM equivalent.
+ThumbDecodeStatus TranslateThumbInstruction(u32 addr, u32 instr, u32* ainstr, u32* inst_size);
 
-static inline u32 get_thumb_instr(u32 instr, u32 pc) {
+static inline u32 GetThumbInstruction(u32 instr, u32 address) {
-    u32 tinstr;
+    // Normally you would need to handle instruction endianness,
-    if ((pc & 0x3) != 0)
+    // however, it is fixed to little-endian on the MPCore, so
-        tinstr = instr >> 16;
+    // there's no need to check for this beforehand.
-    else
+    if ((address & 0x3) != 0)
-        tinstr = instr & 0xFFFF;
+        return instr >> 16;
-    return tinstr;
+
+    return instr & 0xFFFF;
 }

src/core/arm/skyeye_common/armstate.h

@@ -163,6 +163,19 @@ public:
     u32 ReadCP15Register(u32 crn, u32 opcode_1, u32 crm, u32 opcode_2) const;
     void WriteCP15Register(u32 value, u32 crn, u32 opcode_1, u32 crm, u32 opcode_2);
 
+    // Exclusive memory access functions
+    bool IsExclusiveMemoryAccess(u32 address) const {
+        return exclusive_state && exclusive_tag == (address & RESERVATION_GRANULE_MASK);
+    }
+    void SetExclusiveMemoryAddress(u32 address) {
+        exclusive_tag = address & RESERVATION_GRANULE_MASK;
+        exclusive_state = true;
+    }
+    void UnsetExclusiveMemoryAddress() {
+        exclusive_tag = 0xFFFFFFFF;
+        exclusive_state = false;
+    }
+
     // Whether or not the given CPU is in big endian mode (E bit is set)
     bool InBigEndianMode() const {
         return (Cpsr & (1 << 9)) != 0;
@@ -203,9 +216,6 @@ public:
 
     u32 Mode;          // The current mode
     u32 Bank;          // The current register bank
-    u32 exclusive_tag; // The address for which the local monitor is in exclusive access mode
-    u32 exclusive_state;
-    u32 exclusive_result;
 
     u32 NFlag, ZFlag, CFlag, VFlag, IFFlags; // Dummy flags for speed
     unsigned int shifter_carry_out;
@@ -230,4 +240,13 @@ public:
 
 private:
     void ResetMPCoreCP15Registers();
+
+    // Defines a reservation granule of 2 words, which protects the first 2 words starting at the tag.
+    // This is the smallest granule allowed by the v7 spec, and is coincidentally just large enough to
+    // support LDR/STREXD.
+    static const u32 RESERVATION_GRANULE_MASK = 0xFFFFFFF8;
+
+    u32 exclusive_tag; // The address for which the local monitor is in exclusive access mode
+    u32 exclusive_result;
+    bool exclusive_state;
 };