From c34a738b4c7d12d8d72a343ee3c1c46073dd7e38 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 11:33:59 -0700 Subject: [PATCH 1/6] DSPSpy: Add missing jumps to end_of_test Without this, execution continues beyond the end of the function, into the great unknown (probably eventually falling into either code left from a previous test, or the start of the DSP ROM). end_of_test is just an infinite loop to stop executing until the DSP is reset. --- Source/DSPSpy/tests/arith_test.ds | 3 +++ Source/DSPSpy/tests/ld_test.ds | 4 ++-- Source/DSPSpy/tests/neg_test.ds | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Source/DSPSpy/tests/arith_test.ds b/Source/DSPSpy/tests/arith_test.ds index 96884c6bbc..f93f428b06 100644 --- a/Source/DSPSpy/tests/arith_test.ds +++ b/Source/DSPSpy/tests/arith_test.ds @@ -138,3 +138,6 @@ sub $acc1, $acc0 set16 call send_back ; 20 + +; We're done, DO NOT DELETE THIS LINE +jmp end_of_test diff --git a/Source/DSPSpy/tests/ld_test.ds b/Source/DSPSpy/tests/ld_test.ds index 045680c877..4857b61d3e 100644 --- a/Source/DSPSpy/tests/ld_test.ds +++ b/Source/DSPSpy/tests/ld_test.ds @@ -244,5 +244,5 @@ lri $AX1.L, #0x13 nx'ldnm : $AX0.L, $AX1.L, @$AR0 call send_back ; 20 - - +; We're done, DO NOT DELETE THIS LINE +jmp end_of_test diff --git a/Source/DSPSpy/tests/neg_test.ds b/Source/DSPSpy/tests/neg_test.ds index 57c1610511..63f535bb6a 100644 --- a/Source/DSPSpy/tests/neg_test.ds +++ b/Source/DSPSpy/tests/neg_test.ds @@ -114,4 +114,5 @@ neg $ACC0 set40 call send_back ; 18 - +; We're done, DO NOT DELETE THIS LINE +jmp end_of_test From e7f6e19c615146ef44f1598c5b8f071b7a23786e Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 13:49:31 -0700 Subject: [PATCH 2/6] DSPAssembler: Slightly improve readability of AssemblePass This should result in no behavior differences. --- Source/Core/Core/DSP/DSPAssembler.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAssembler.cpp b/Source/Core/Core/DSP/DSPAssembler.cpp index 2e6f29939e..48165eb83f 100644 --- a/Source/Core/Core/DSP/DSPAssembler.cpp +++ b/Source/Core/Core/DSP/DSPAssembler.cpp @@ -751,7 +751,7 @@ void DSPAssembler::InitPass(int pass) bool DSPAssembler::AssemblePass(const std::string& text, int pass) { - int disable_text = 0; // modified by Hermes + bool disable_text = false; // modified by Hermes std::istringstream fsrc(text); @@ -781,10 +781,10 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) // modified by Hermes : added // and /* */ for long commentaries if (c == '/') { - if (i < 1023) + if (i + 1 < LINEBUF_SIZE) { if (line[i + 1] == '/') - c = 0x00; + c = '\0'; else if (line[i + 1] == '*') { // toggle comment mode. @@ -794,28 +794,28 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) } else if (c == '*') { - if (i < 1023 && line[i + 1] == '/' && disable_text) + if (i + 1 < LINEBUF_SIZE && line[i + 1] == '/' && disable_text) { - disable_text = 0; - c = 32; - line[i + 1] = 32; + disable_text = false; + c = ' '; + line[i + 1] = ' '; } } // turn text into spaces if disable_text is on (in a comment). - if (disable_text && ((unsigned char)c) > 32) - c = 32; + if (disable_text && ((unsigned char)c) > ' ') + c = ' '; - if (c == 0x0a || c == 0x0d || c == ';') - c = 0x00; - if (c == 0x09) // tabs to spaces + if (c == '\r' || c == '\n' || c == ';') + c = '\0'; + if (c == '\t') // tabs to spaces c = ' '; if (c == '"') upper = !upper; if (upper && c >= 'a' && c <= 'z') // convert to uppercase c = c - 'a' + 'A'; line[i] = c; - if (c == 0) + if (c == '\0') break; // modified by Hermes } char* ptr = line; From 8b52c7315bb75c7268dfa1d23ab50266cb2cd673 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 16:04:57 -0700 Subject: [PATCH 3/6] DSP: Fix assembling x8/x9/xA/xB conditions The assembler upper-cases the mnemonic internally, so it never would match the lower-case x. --- Source/Core/Core/DSP/DSPTables.cpp | 56 +++++++++++++++--------------- Source/DSPSpy/tests/cond_test.ds | 8 ++--- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Source/Core/Core/DSP/DSPTables.cpp b/Source/Core/Core/DSP/DSPTables.cpp index 7f1ab00a6b..9ec8aba5bc 100644 --- a/Source/Core/Core/DSP/DSPTables.cpp +++ b/Source/Core/Core/DSP/DSPTables.cpp @@ -39,10 +39,10 @@ const std::array s_opcodes = {"RETZ", 0x02d5, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if zero {"RETNC", 0x02d6, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if not carry {"RETC", 0x02d7, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if carry - {"RETx8", 0x02d8, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO - {"RETx9", 0x02d9, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO - {"RETxA", 0x02da, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO - {"RETxB", 0x02db, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO + {"RETX8", 0x02d8, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO + {"RETX9", 0x02d9, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO + {"RETXA", 0x02da, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO + {"RETXB", 0x02db, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if TODO {"RETLNZ", 0x02dc, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if logic not zero {"RETLZ", 0x02dd, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if logic zero {"RETO", 0x02de, 0xffff, 1, 0, {}, false, true, false, true, false}, // return if overflow @@ -56,10 +56,10 @@ const std::array s_opcodes = {"RTIZ", 0x02f5, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if zero {"RTINC", 0x02f6, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if not carry {"RTIC", 0x02f7, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if carry - {"RTIx8", 0x02f8, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO - {"RTIx9", 0x02f9, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO - {"RTIxA", 0x02fa, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO - {"RTIxB", 0x02fb, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO + {"RTIX8", 0x02f8, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO + {"RTIX9", 0x02f9, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO + {"RTIXA", 0x02fa, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO + {"RTIXB", 0x02fb, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if TODO {"RTILNZ", 0x02fc, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if logic not zero {"RTILZ", 0x02fd, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if logic zero {"RTIO", 0x02fe, 0xffff, 1, 0, {}, false, true, false, true, false}, // return from interrupt if overflow @@ -73,10 +73,10 @@ const std::array s_opcodes = {"CALLZ", 0x02b5, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if zero {"CALLNC", 0x02b6, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if not carry {"CALLC", 0x02b7, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if carry - {"CALLx8", 0x02b8, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO - {"CALLx9", 0x02b9, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO - {"CALLxA", 0x02ba, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO - {"CALLxB", 0x02bb, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO + {"CALLX8", 0x02b8, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO + {"CALLX9", 0x02b9, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO + {"CALLXA", 0x02ba, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO + {"CALLXB", 0x02bb, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if TODO {"CALLLNZ", 0x02bc, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if logic not zero {"CALLLZ", 0x02bd, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if logic zero {"CALLO", 0x02be, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // call if overflow @@ -90,10 +90,10 @@ const std::array s_opcodes = {"IFZ", 0x0275, 0xffff, 1, 0, {}, false, true, false, true, false}, // if zero {"IFNC", 0x0276, 0xffff, 1, 0, {}, false, true, false, true, false}, // if not carry {"IFC", 0x0277, 0xffff, 1, 0, {}, false, true, false, true, false}, // if carry - {"IFx8", 0x0278, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO - {"IFx9", 0x0279, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO - {"IFxA", 0x027a, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO - {"IFxB", 0x027b, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO + {"IFX8", 0x0278, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO + {"IFX9", 0x0279, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO + {"IFXA", 0x027a, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO + {"IFXB", 0x027b, 0xffff, 1, 0, {}, false, true, false, true, false}, // if TODO {"IFLNZ", 0x027c, 0xffff, 1, 0, {}, false, true, false, true, false}, // if logic not zero {"IFLZ", 0x027d, 0xffff, 1, 0, {}, false, true, false, true, false}, // if logic zero {"IFO", 0x027e, 0xffff, 1, 0, {}, false, true, false, true, false}, // if overflow @@ -107,10 +107,10 @@ const std::array s_opcodes = {"JZ", 0x0295, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if zero {"JNC", 0x0296, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if not carry {"JC", 0x0297, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if carry - {"JMPx8", 0x0298, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO - {"JMPx9", 0x0299, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO - {"JMPxA", 0x029a, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO - {"JMPxB", 0x029b, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO + {"JMPX8", 0x0298, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO + {"JMPX9", 0x0299, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO + {"JMPXA", 0x029a, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO + {"JMPXB", 0x029b, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if TODO {"JLNZ", 0x029c, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if logic not zero {"JLZ", 0x029d, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if logic zero {"JO", 0x029e, 0xffff, 2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}}, false, true, false, true, false}, // jump if overflow @@ -124,10 +124,10 @@ const std::array s_opcodes = {"JRZ", 0x1705, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if zero {"JRNC", 0x1706, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if not carry {"JRC", 0x1707, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if carry - {"JMPRx8", 0x1708, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO - {"JMPRx9", 0x1709, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO - {"JMPRxA", 0x170a, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO - {"JMPRxB", 0x170b, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO + {"JMPRX8", 0x1708, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO + {"JMPRX9", 0x1709, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO + {"JMPRXA", 0x170a, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO + {"JMPRXB", 0x170b, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if TODO {"JRLNZ", 0x170c, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if logic not zero {"JRLZ", 0x170d, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if logic zero {"JRO", 0x170e, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, false, false}, // jump to $R if overflow @@ -141,10 +141,10 @@ const std::array s_opcodes = {"CALLRZ", 0x1715, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if zero {"CALLRNC", 0x1716, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if not carry {"CALLRC", 0x1717, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if carry - {"CALLRx8", 0x1718, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO - {"CALLRx9", 0x1719, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO - {"CALLRxA", 0x171a, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO - {"CALLRxB", 0x171b, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO + {"CALLRX8", 0x1718, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO + {"CALLRX9", 0x1719, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO + {"CALLRXA", 0x171a, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO + {"CALLRXB", 0x171b, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if TODO {"CALLRLNZ", 0x171c, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if logic not zero {"CALLRLZ", 0x171d, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if logic zero {"CALLRO", 0x171e, 0xff1f, 1, 1, {{P_REG, 1, 0, 5, 0x00e0}}, false, true, false, true, false}, // call $R if overflow diff --git a/Source/DSPSpy/tests/cond_test.ds b/Source/DSPSpy/tests/cond_test.ds index 151f3e3a9a..411fc73c76 100644 --- a/Source/DSPSpy/tests/cond_test.ds +++ b/Source/DSPSpy/tests/cond_test.ds @@ -213,19 +213,19 @@ test_cond: ADDARN $AR0, $IX0 LRI $IX0, #0x0100 - CW 0x0278 ; IFx8 + IFx8 ADDARN $AR0, $IX0 LRI $IX0, #0x0200 - CW 0x0279 ; IFx9 + IFx9 ADDARN $AR0, $IX0 LRI $IX0, #0x0400 - CW 0x027A ; IFxA + IFxA ADDARN $AR0, $IX0 LRI $IX0, #0x0800 - CW 0x027B ; IFxB + IFxB ADDARN $AR0, $IX0 LRI $IX0, #0x1000 From b06d38389bddf9fc5962aecbe1b31292a71b80a5 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 16:51:46 -0700 Subject: [PATCH 4/6] DSP: Remove some magic numbers for register IDs --- Source/Core/Core/DSP/DSPAssembler.cpp | 30 +++++------ Source/Core/Core/DSP/DSPCore.h | 16 ++++-- Source/Core/Core/DSP/DSPTables.cpp | 74 +++++++++++++-------------- Source/Core/Core/DSP/DSPTables.h | 31 +++++------ 4 files changed, 80 insertions(+), 71 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAssembler.cpp b/Source/Core/Core/DSP/DSPAssembler.cpp index 48165eb83f..67077f8675 100644 --- a/Source/Core/Core/DSP/DSPAssembler.cpp +++ b/Source/Core/Core/DSP/DSPAssembler.cpp @@ -22,6 +22,7 @@ #include "Common/FileUtil.h" #include "Common/StringUtil.h" +#include "Core/DSP/DSPCore.h" #include "Core/DSP/DSPDisassembler.h" #include "Core/DSP/DSPTables.h" @@ -483,17 +484,15 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t if ((opc->params[i].type & P_REG) && (par[i].type & P_REG)) { - // Just a temp. Should be replaced with more purposeful vars. - int value; - // modified by Hermes: test the register range - switch ((unsigned)opc->params[i].type) + switch (opc->params[i].type) { case P_REG18: case P_REG19: case P_REG1A: case P_REG1C: - value = (opc->params[i].type >> 8) & 31; + { + int value = (opc->params[i].type >> 8) & 0x1f; if ((int)par[i].val < value || (int)par[i].val > value + get_mask_shifted_down(opc->params[i].mask)) { @@ -504,8 +503,9 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t ShowError(AssemblerError::InvalidRegister); } break; + } case P_PRG: - if ((int)par[i].val < 0 || (int)par[i].val > 0x3) + if ((int)par[i].val < DSP_REG_AR0 || (int)par[i].val > DSP_REG_AR3) { if (type == OpcodeType::Extension) fprintf(stderr, "(ext) "); @@ -515,12 +515,12 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t } break; case P_ACC: - if ((int)par[i].val < 0x20 || (int)par[i].val > 0x21) + if ((int)par[i].val < DSP_REG_ACC0_FULL || (int)par[i].val > DSP_REG_ACC1_FULL) { if (type == OpcodeType::Extension) fprintf(stderr, "(ext) "); - if (par[i].val >= 0x1e && par[i].val <= 0x1f) + if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { fprintf(stderr, "%i : %s ", m_code_line, m_cur_line.c_str()); fprintf(stderr, @@ -529,7 +529,7 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t (par[i].val & 1), (par[i].val & 1), m_code_line, current_param, static_cast(type)); } - else if (par[i].val >= 0x1c && par[i].val <= 0x1d) + else if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { fprintf( stderr, @@ -543,19 +543,19 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t } break; case P_ACCM: - if ((int)par[i].val < 0x1e || (int)par[i].val > 0x1f) + if ((int)par[i].val < DSP_REG_ACM0 || (int)par[i].val > DSP_REG_ACM1) { if (type == OpcodeType::Extension) fprintf(stderr, "(ext) "); - if (par[i].val >= 0x1c && par[i].val <= 0x1d) + if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { fprintf( stderr, "WARNING: $ACL%d register used instead of $ACM%d register Line: %d Param: %zu\n", (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); } - else if (par[i].val >= 0x20 && par[i].val <= 0x21) + else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { fprintf( stderr, @@ -570,12 +570,12 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t break; case P_ACCL: - if ((int)par[i].val < 0x1c || (int)par[i].val > 0x1d) + if ((int)par[i].val < DSP_REG_ACL0 || (int)par[i].val > DSP_REG_ACL1) { if (type == OpcodeType::Extension) fprintf(stderr, "(ext) "); - if (par[i].val >= 0x1e && par[i].val <= 0x1f) + if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { fprintf(stderr, "%s ", m_cur_line.c_str()); fprintf( @@ -583,7 +583,7 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t "WARNING: $ACM%d register used instead of $ACL%d register Line: %d Param: %zu\n", (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); } - else if (par[i].val >= 0x20 && par[i].val <= 0x21) + else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { fprintf(stderr, "%s ", m_cur_line.c_str()); fprintf( diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index c61ac3940e..4c293fecf7 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -122,15 +122,23 @@ enum : int DSP_REG_AXH1 = 0x1b, // Accumulator (global) - DSP_REG_ACC0 = 0x1c, - DSP_REG_ACC1 = 0x1d, - DSP_REG_ACL0 = 0x1c, // Low accumulator DSP_REG_ACL1 = 0x1d, DSP_REG_ACM0 = 0x1e, // Mid accumulator DSP_REG_ACM1 = 0x1f, DSP_REG_ACH0 = 0x10, // Sign extended 8 bit register 0 - DSP_REG_ACH1 = 0x11 // Sign extended 8 bit register 1 + DSP_REG_ACH1 = 0x11, // Sign extended 8 bit register 1 +}; + +enum : int +{ + // Magic values used by DSPTables.h + // These do not correspond to real registers like above, but instead combined versions of the + // registers. + DSP_REG_ACC0_FULL = 0x20, + DSP_REG_ACC1_FULL = 0x21, + DSP_REG_AX0_FULL = 0x22, + DSP_REG_AX1_FULL = 0x23, }; // Hardware registers address diff --git a/Source/Core/Core/DSP/DSPTables.cpp b/Source/Core/Core/DSP/DSPTables.cpp index 9ec8aba5bc..5b93beea8e 100644 --- a/Source/Core/Core/DSP/DSPTables.cpp +++ b/Source/Core/Core/DSP/DSPTables.cpp @@ -13,7 +13,7 @@ #include "Common/CommonTypes.h" #include "Common/Logging/Log.h" -#include "Core/DSP/Interpreter/DSPIntTables.h" +#include "Core/DSP/DSPCore.h" namespace DSP { @@ -451,44 +451,44 @@ const std::array pdlabels = const std::array regnames = {{ - {0x00, "AR0", "Addr Reg 00",}, - {0x01, "AR1", "Addr Reg 01",}, - {0x02, "AR2", "Addr Reg 02",}, - {0x03, "AR3", "Addr Reg 03",}, - {0x04, "IX0", "Index Reg 0",}, - {0x05, "IX1", "Index Reg 1",}, - {0x06, "IX2", "Index Reg 2",}, - {0x07, "IX3", "Index Reg 3",}, - {0x08, "WR0", "Wrapping Register 0",}, - {0x09, "WR1", "Wrapping Register 1",}, - {0x0a, "WR2", "Wrapping Register 2",}, - {0x0b, "WR3", "Wrapping Register 3",}, - {0x0c, "ST0", "Call stack",}, - {0x0d, "ST1", "Data stack",}, - {0x0e, "ST2", "Loop addr stack",}, - {0x0f, "ST3", "Loop counter stack",}, - {0x10, "AC0.H", "Accu High 0",}, - {0x11, "AC1.H", "Accu High 1",}, - {0x12, "CR", "Config Register",}, - {0x13, "SR", "Special Register",}, - {0x14, "PROD.L", "Prod L",}, - {0x15, "PROD.M1", "Prod M1",}, - {0x16, "PROD.H", "Prod H",}, - {0x17, "PROD.M2", "Prod M2",}, - {0x18, "AX0.L", "Extra Accu L 0",}, - {0x19, "AX1.L", "Extra Accu L 1",}, - {0x1a, "AX0.H", "Extra Accu H 0",}, - {0x1b, "AX1.H", "Extra Accu H 1",}, - {0x1c, "AC0.L", "Accu Low 0",}, - {0x1d, "AC1.L", "Accu Low 1",}, - {0x1e, "AC0.M", "Accu Mid 0",}, - {0x1f, "AC1.M", "Accu Mid 1",}, + {DSP_REG_AR0, "AR0", "Addr Reg 00",}, + {DSP_REG_AR1, "AR1", "Addr Reg 01",}, + {DSP_REG_AR2, "AR2", "Addr Reg 02",}, + {DSP_REG_AR3, "AR3", "Addr Reg 03",}, + {DSP_REG_IX0, "IX0", "Index Reg 0",}, + {DSP_REG_IX1, "IX1", "Index Reg 1",}, + {DSP_REG_IX2, "IX2", "Index Reg 2",}, + {DSP_REG_IX3, "IX3", "Index Reg 3",}, + {DSP_REG_WR0, "WR0", "Wrapping Register 0",}, + {DSP_REG_WR1, "WR1", "Wrapping Register 1",}, + {DSP_REG_WR2, "WR2", "Wrapping Register 2",}, + {DSP_REG_WR3, "WR3", "Wrapping Register 3",}, + {DSP_REG_ST0, "ST0", "Call stack",}, + {DSP_REG_ST1, "ST1", "Data stack",}, + {DSP_REG_ST2, "ST2", "Loop addr stack",}, + {DSP_REG_ST3, "ST3", "Loop counter stack",}, + {DSP_REG_ACH0, "AC0.H", "Accu High 0",}, + {DSP_REG_ACH1, "AC1.H", "Accu High 1",}, + {DSP_REG_CR, "CR", "Config Register",}, + {DSP_REG_SR, "SR", "Special Register",}, + {DSP_REG_PRODL, "PROD.L", "Prod L",}, + {DSP_REG_PRODM, "PROD.M1", "Prod M1",}, + {DSP_REG_PRODH, "PROD.H", "Prod H",}, + {DSP_REG_PRODM2, "PROD.M2", "Prod M2",}, + {DSP_REG_AXL0, "AX0.L", "Extra Accu L 0",}, + {DSP_REG_AXL1, "AX1.L", "Extra Accu L 1",}, + {DSP_REG_AXH0, "AX0.H", "Extra Accu H 0",}, + {DSP_REG_AXH1, "AX1.H", "Extra Accu H 1",}, + {DSP_REG_ACL0, "AC0.L", "Accu Low 0",}, + {DSP_REG_ACL1, "AC1.L", "Accu Low 1",}, + {DSP_REG_ACM0, "AC0.M", "Accu Mid 0",}, + {DSP_REG_ACM1, "AC1.M", "Accu Mid 1",}, // To resolve combined register names. - {0x20, "ACC0", "Accu Full 0",}, - {0x21, "ACC1", "Accu Full 1",}, - {0x22, "AX0", "Extra Accu 0",}, - {0x23, "AX1", "Extra Accu 1",}, + {DSP_REG_ACC0_FULL, "ACC0", "Accu Full 0",}, + {DSP_REG_ACC1_FULL, "ACC1", "Accu Full 1",}, + {DSP_REG_AX0_FULL, "AX0", "Extra Accu 0",}, + {DSP_REG_AX1_FULL, "AX1", "Extra Accu 1",}, }}; // clang-format on diff --git a/Source/Core/Core/DSP/DSPTables.h b/Source/Core/Core/DSP/DSPTables.h index 2dead9094e..e2a90d8e7f 100644 --- a/Source/Core/Core/DSP/DSPTables.h +++ b/Source/Core/Core/DSP/DSPTables.h @@ -11,6 +11,7 @@ #include #include "Core/DSP/DSPCommon.h" +#include "Core/DSP/DSPCore.h" namespace DSP { @@ -31,23 +32,23 @@ enum partype_t P_ADDR_I = 0x0005, P_ADDR_D = 0x0006, P_REG = 0x8000, - P_REG04 = P_REG | 0x0400, // IX - P_REG08 = P_REG | 0x0800, - P_REG18 = P_REG | 0x1800, - P_REGM18 = P_REG | 0x1810, // used in multiply instructions - P_REG19 = P_REG | 0x1900, - P_REGM19 = P_REG | 0x1910, // used in multiply instructions - P_REG1A = P_REG | 0x1a80, + P_REG04 = P_REG | DSP_REG_IX0 << 8, + P_REG08 = P_REG | DSP_REG_WR0 << 8, + P_REG18 = P_REG | DSP_REG_AXL0 << 8, + P_REGM18 = P_REG | DSP_REG_AXL0 << 8 | 0x10, // used in multiply instructions + P_REG19 = P_REG | DSP_REG_AXL1 << 8, + P_REGM19 = P_REG | DSP_REG_AXL1 << 8 | 0x10, // used in multiply instructions + P_REG1A = P_REG | DSP_REG_AXH0 << 8 | 0x80, // P_ACC = P_REG | 0x1c10, // used for global accum (gcdsptool's value) - P_ACCL = P_REG | 0x1c00, // used for low part of accum - P_REG1C = P_REG | 0x1c10, // gcdsptool calls this P_ACCLM - P_ACCM = P_REG | 0x1e00, // used for mid part of accum + P_ACCL = P_REG | DSP_REG_ACL0 << 8, // used for low part of accum + P_REG1C = P_REG | DSP_REG_ACL0 << 8 | 0x10, // gcdsptool calls this P_ACCLM + P_ACCM = P_REG | DSP_REG_ACM0 << 8, // used for mid part of accum // The following are not in gcdsptool - P_ACCM_D = P_REG | 0x1e80, - P_ACC = P_REG | 0x2000, // used for full accum. - P_ACCH = P_REG | 0x1000, // used for high part of accum - P_ACC_D = P_REG | 0x2080, - P_AX = P_REG | 0x2200, + P_ACCM_D = P_REG | DSP_REG_ACM0 << 8 | 0x80, + P_ACC = P_REG | DSP_REG_ACC0_FULL << 8, // used for full accum. + P_ACCH = P_REG | DSP_REG_ACH0 << 8, // used for high part of accum + P_ACC_D = P_REG | DSP_REG_ACC0_FULL << 8 | 0x80, + P_AX = P_REG | DSP_REG_AX0_FULL << 8, P_REGS_MASK = 0x03f80, // gcdsptool's value = 0x01f80 P_REF = P_REG | 0x4000, P_PRG = P_REF | P_REG, From db3d457e5f4b37b9271407db920362e5d37d9a94 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 17:04:27 -0700 Subject: [PATCH 5/6] DSPDisassembler: Remove redundant definition of CW CW is used as a fallback to write a full instruction as hex, but we already declare it in DSPTables.h for the assembler. --- Source/Core/Core/DSP/DSPDisassembler.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index ea78d237a6..01817c0780 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -152,10 +152,8 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, u16* pc, std::string& // Find main opcode const DSPOPCTemplate* opc = FindOpInfoByOpcode(op1); - const DSPOPCTemplate fake_op = {"CW", 0x0000, 0x0000, 1, 1, {{P_VAL, 2, 0, 0, 0xffff}}, - false, false, false, false, false}; if (!opc) - opc = &fake_op; + opc = &cw; bool is_extended = false; bool is_only_7_bit_ext = false; From bd3173e344912d0b2031c46407e2f18cf6a059fd Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 23 May 2022 14:10:49 -0700 Subject: [PATCH 6/6] DSPAssembler: Rework errors and warnings Among other things, this trims trailing newline characters. Before (on windows) the \r would corrupt the output and make them very hard to understand (as the error message would be drawn over the code line, but part of the code line would peek out from behind it). --- Source/Core/Core/DSP/DSPAssembler.cpp | 248 ++++++++++++++------------ Source/Core/Core/DSP/DSPAssembler.h | 26 ++- Source/Core/Core/DSP/DSPCodeUtil.cpp | 8 +- 3 files changed, 156 insertions(+), 126 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAssembler.cpp b/Source/Core/Core/DSP/DSPAssembler.cpp index 67077f8675..ed181b7a3c 100644 --- a/Source/Core/Core/DSP/DSPAssembler.cpp +++ b/Source/Core/Core/DSP/DSPAssembler.cpp @@ -26,6 +26,42 @@ #include "Core/DSP/DSPDisassembler.h" #include "Core/DSP/DSPTables.h" +template <> +struct fmt::formatter +{ + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + template + auto format(const DSP::DSPAssembler::LocationContext& loc, FormatContext& ctx) const + { + // Trim trailing newlines + // TODO: Ideally, we shouldn't be getting any newlines at all here, but + // DSPTool uses File::ReadFileToString, which opens the file in binary mode, + // so we get \r endings on Windows. This leads to bad results when printing the string. + std::string trimmed_line = loc.line_text; + // while (trimmed_line.ends_with('\r') || trimmed_line.ends_with('\n')) + while (!trimmed_line.empty() && (trimmed_line.back() == '\r' || trimmed_line.back() == '\n')) + trimmed_line.pop_back(); + + auto out = fmt::format_to(ctx.out(), "At line {}", loc.line_num); + + if (loc.included_file_path.has_value()) + out = fmt::format_to(out, " of included file {}", loc.included_file_path.value()); + + if (loc.opcode_type.has_value()) + { + if (loc.opcode_type.value() == DSP::DSPAssembler::OpcodeType::Primary) + out = fmt::format_to(out, ", main opcode"); + else if (loc.opcode_type.value() == DSP::DSPAssembler::OpcodeType::Extension) + out = fmt::format_to(out, ", extension opcode"); + + if (loc.opcode_param_number.has_value()) + out = fmt::format_to(out, " parameter {}", loc.opcode_param_number.value()); + } + + return fmt::format_to(out, ":\n{}", trimmed_line); + } +}; + namespace DSP { static const char* err_string[] = {"", @@ -88,31 +124,39 @@ bool DSPAssembler::Assemble(const std::string& text, std::vector& code, return true; } -void DSPAssembler::ShowError(AssemblerError err_code, const char* extra_info) +void DSPAssembler::ShowError(AssemblerError err_code) { if (!m_settings.force) m_failed = true; - std::string error = fmt::format("{} : {} ", m_code_line, m_cur_line); - if (!extra_info) - extra_info = "-"; - - const char* const error_string = err_string[static_cast(err_code)]; - - if (m_current_param == 0) - { - error += fmt::format("ERROR: {} Line: {} : {}\n", error_string, m_code_line, extra_info); - } - else - { - error += fmt::format("ERROR: {} Line: {} Param: {} : {}\n", error_string, m_code_line, - m_current_param, extra_info); - } - - m_last_error_str = std::move(error); + m_last_error_str = + fmt::format("{}\nERROR: {}\n\n", m_location, err_string[static_cast(err_code)]); + fmt::print(stderr, "{}", m_last_error_str); m_last_error = err_code; } +template +void DSPAssembler::ShowError(AssemblerError err_code, fmt::format_string format, + Args&&... args) +{ + if (!m_settings.force) + m_failed = true; + + const auto msg = fmt::format(format, std::forward(args)...); + + m_last_error_str = fmt::format("{}\nERROR: {}: {}\n\n", m_location, + err_string[static_cast(err_code)], msg); + fmt::print(stderr, "{}", m_last_error_str); + m_last_error = err_code; +} + +template +void DSPAssembler::ShowWarning(fmt::format_string format, Args&&... args) +{ + const auto msg = fmt::format(format, std::forward(args)...); + fmt::print(stderr, "{}\nWARNING: {}\n\n", m_location, msg); +} + static char* skip_spaces(char* ptr) { while (*ptr == ' ') @@ -147,7 +191,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '9') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectDecimal, str); + ShowError(AssemblerError::IncorrectDecimal, "{}", str); } } else @@ -165,7 +209,7 @@ s32 DSPAssembler::ParseValue(const char* str) else if (ptr[i] >= '0' && ptr[i] <= '9') val += (ptr[i] - '0'); else - ShowError(AssemblerError::IncorrectHex, str); + ShowError(AssemblerError::IncorrectHex, "{}", str); } break; case '\'': // binary @@ -175,7 +219,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '1') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectBinary, str); + ShowError(AssemblerError::IncorrectBinary, "{}", str); } break; default: @@ -196,7 +240,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '9') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectDecimal, str); + ShowError(AssemblerError::IncorrectDecimal, "{}", str); } } else // Everything else is a label. @@ -206,7 +250,7 @@ s32 DSPAssembler::ParseValue(const char* str) return *value; if (m_cur_pass == 2) - ShowError(AssemblerError::UnknownLabel, str); + ShowError(AssemblerError::UnknownLabel, "{}", str); } } if (negative) @@ -332,10 +376,10 @@ u32 DSPAssembler::ParseExpression(const char* ptr) val = ParseExpression(d_buffer) - ParseExpression(pbuf + 1); if (val < 0) { + ShowWarning("Number Underflow: {}", val); val = 0x10000 + (val & 0xffff); // ATTENTION: avoid a terrible bug!!! number cannot write with '-' in sprintf - fprintf(stderr, "WARNING: Number Underflow at Line: %d \n", m_code_line); } sprintf(d_buffer, "%d", val); } @@ -472,7 +516,7 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t { for (size_t i = 0; i < count; i++) { - const size_t current_param = i + 1; // just for display. + m_location.opcode_param_number = i + 1; if (opc->params[i].type != par[i].type || (par[i].type & P_REG)) { if (par[i].type == P_VAL && @@ -496,10 +540,6 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t if ((int)par[i].val < value || (int)par[i].val > value + get_mask_shifted_down(opc->params[i].mask)) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - - fprintf(stderr, "%s (param %zu)", m_cur_line.c_str(), current_param); ShowError(AssemblerError::InvalidRegister); } break; @@ -507,34 +547,19 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_PRG: if ((int)par[i].val < DSP_REG_AR0 || (int)par[i].val > DSP_REG_AR3) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - - fprintf(stderr, "%s (param %zu)", m_cur_line.c_str(), current_param); ShowError(AssemblerError::InvalidRegister); } break; case P_ACC: if ((int)par[i].val < DSP_REG_ACC0_FULL || (int)par[i].val > DSP_REG_ACC1_FULL) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { - fprintf(stderr, "%i : %s ", m_code_line, m_cur_line.c_str()); - fprintf(stderr, - "WARNING: $ACM%d register used instead of $ACC%d register Line: %d " - "Param: %zu Ext: %d\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param, - static_cast(type)); + ShowWarning("$ACM{0} register used instead of $ACC{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { - fprintf( - stderr, - "WARNING: $ACL%d register used instead of $ACC%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACL{0} register used instead of $ACC{0} register", (par[i].val & 1)); } else { @@ -545,22 +570,13 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_ACCM: if ((int)par[i].val < DSP_REG_ACM0 || (int)par[i].val > DSP_REG_ACM1) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { - fprintf( - stderr, - "WARNING: $ACL%d register used instead of $ACM%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACL{0} register used instead of $ACCM{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { - fprintf( - stderr, - "WARNING: $ACC%d register used instead of $ACM%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACC{0} register used instead of $ACM{0} register", (par[i].val & 1)); } else { @@ -572,24 +588,13 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_ACCL: if ((int)par[i].val < DSP_REG_ACL0 || (int)par[i].val > DSP_REG_ACL1) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { - fprintf(stderr, "%s ", m_cur_line.c_str()); - fprintf( - stderr, - "WARNING: $ACM%d register used instead of $ACL%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACM{0} register used instead of $ACL{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { - fprintf(stderr, "%s ", m_cur_line.c_str()); - fprintf( - stderr, - "WARNING: $ACC%d register used instead of $ACL%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACC{0} register used instead of $ACL{0} register", (par[i].val & 1)); } else { @@ -604,23 +609,15 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t switch (par[i].type & (P_REG | 7)) { case P_REG: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamReg); break; case P_MEM: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamMem); break; case P_VAL: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamVal); break; case P_IMM: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamImm); break; } @@ -634,40 +631,49 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t unsigned int valueu = 0xffff & ~(value >> 1); if ((int)par[i].val < 0) { - if (value == 7) // value 7 por sbclr/sbset + if (value == 7) // value 7 for sbclr/sbset { - fprintf(stderr, "Value must be from 0x0 to 0x%x\n", value); - ShowError(AssemblerError::NumberOutOfRange); + ShowError(AssemblerError::NumberOutOfRange, "Value must be from 0x0 to {:#x}, was {:#x}", + value, (int)par[i].val); } else if (opc->params[i].type == P_MEM) { if (value < 256) - fprintf(stderr, "Address value must be from 0x%x to 0x%x\n", valueu, (value >> 1)); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from {:#x} to {:#x}, was {:#x}", valueu, (value >> 1), + (int)par[i].val); + } else - fprintf(stderr, "Address value must be from 0x0 to 0x%x\n", value); - - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from 0x0 to {:#x}, was {:#x}", value, (int)par[i].val); + } } else if ((int)par[i].val < -((value >> 1) + 1)) { if (value < 128) - fprintf(stderr, "Value must be from -0x%x to 0x%x, is %i\n", (value >> 1) + 1, - value >> 1, par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}", -((value >> 1) + 1), + value >> 1, (int)par[i].val); + } else - fprintf(stderr, "Value must be from -0x%x to 0x%x or 0x0 to 0x%x, is %i\n", - (value >> 1) + 1, value >> 1, value, par[i].val); - - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x} or 0x0 to {:#x}, was {:#x}", + -((value >> 1) + 1), value >> 1, value, (int)par[i].val); + } } } else { - if (value == 7) // value 7 por sbclr/sbset + if (value == 7) // value 7 for sbclr/sbset { if (par[i].val > (unsigned)value) { - fprintf(stderr, "Value must be from 0x%x to 0x%x, is %i\n", valueu, value, par[i].val); - ShowError(AssemblerError::NumberOutOfRange); + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}\n", valueu, value, par[i].val); } } else if (opc->params[i].type == P_MEM) @@ -678,11 +684,17 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t (par[i].val < valueu || par[i].val > (unsigned)0xffff)) { if (value < 256) - fprintf(stderr, "Address value must be from 0x%x to 0x%x, is %04x\n", valueu, value, - par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from {:#x} to {:#x}, was {:04x}\n", valueu, value, + par[i].val); + } else - fprintf(stderr, "Address value must be minor of 0x%x\n", value + 1); - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be less than {:#x}, was {:04x}\n", value + 1, + par[i].val); + } } } else @@ -692,18 +704,23 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t if (par[i].val > (unsigned)value) { if (value < 64) - fprintf(stderr, "Value must be from -0x%x to 0x%x, is %i\n", (value + 1), value, - par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}\n", -(value + 1), value, + par[i].val); + } else - fprintf(stderr, "Value must be minor of 0x%x, is %i\n", value + 1, par[i].val); - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be less than {:#x}, was {:#x}\n", value + 1, par[i].val); + } } } } continue; } } - m_current_param = 0; + m_location.opcode_param_number = std::nullopt; return true; } @@ -755,7 +772,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) std::istringstream fsrc(text); - m_code_line = 0; + m_location.line_num = 0; m_cur_pass = pass; #define LINEBUF_SIZE 1024 @@ -767,8 +784,8 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) if (fsrc.fail()) break; - m_cur_line = line; - m_code_line++; + m_location.line_text = line; + m_location.line_num++; param_t params[10] = {{0, P_NONE, nullptr}}; param_t params_ext[10] = {{0, P_NONE, nullptr}}; @@ -905,23 +922,22 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) { if (params[0].type == P_STR) { - std::string include_file_path; - const u32 this_code_line = m_code_line; + const auto old_location = m_location; if (m_include_dir.empty()) { - include_file_path = params[0].str; + m_location.included_file_path = params[0].str; } else { - include_file_path = m_include_dir + '/' + params[0].str; + m_location.included_file_path = m_include_dir + '/' + params[0].str; } std::string included_text; - File::ReadFileToString(include_file_path, included_text); + File::ReadFileToString(m_location.included_file_path.value(), included_text); AssemblePass(included_text, pass); - m_code_line = this_code_line; + m_location = old_location; } else { @@ -959,9 +975,8 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) { if (m_cur_addr > params[0].val) { - const std::string msg = fmt::format("WARNPC at 0x{:04x}, expected 0x{:04x} or less", - m_cur_addr, params[0].val); - ShowError(AssemblerError::PCOutOfRange, msg.c_str()); + ShowError(AssemblerError::PCOutOfRange, "WARNPC at 0x{:04x}, expected 0x{:04x} or less", + m_cur_addr, params[0].val); } } else @@ -987,6 +1002,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) continue; } + m_location.opcode_type = OpcodeType::Primary; const DSPOPCTemplate* opc = FindOpcode(opcode, params_count, OpcodeType::Primary); if (!opc) opc = &cw; @@ -997,6 +1013,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) const DSPOPCTemplate* opc_ext = nullptr; // Check for opcode extensions. + m_location.opcode_type = OpcodeType::Extension; if (opc->extended) { if (opcode_ext) @@ -1016,6 +1033,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) if (params_count_ext) ShowError(AssemblerError::ExtensionParamsOnNonExtendableOpcode); } + m_location.opcode_type = std::nullopt; if (pass == 2) { diff --git a/Source/Core/Core/DSP/DSPAssembler.h b/Source/Core/Core/DSP/DSPAssembler.h index 4c1cdbcd94..29ccaeb557 100644 --- a/Source/Core/Core/DSP/DSPAssembler.h +++ b/Source/Core/Core/DSP/DSPAssembler.h @@ -6,9 +6,13 @@ #include #include +#include #include +#include #include +#include + #include "Common/CommonTypes.h" #include "Core/DSP/DSPDisassembler.h" @@ -85,6 +89,17 @@ private: Extension }; + struct LocationContext + { + u32 line_num = 0; + std::string line_text; + std::optional opcode_type; + std::optional opcode_param_number; + std::optional included_file_path; + }; + + friend struct ::fmt::formatter; + // Utility functions s32 ParseValue(const char* str); u32 ParseExpression(const char* ptr); @@ -94,7 +109,11 @@ private: void InitPass(int pass); bool AssemblePass(const std::string& text, int pass); - void ShowError(AssemblerError err_code, const char* extra_info = nullptr); + void ShowError(AssemblerError err_code); + template + void ShowError(AssemblerError err_code, fmt::format_string format, Args&&... args); + template + void ShowWarning(fmt::format_string format, Args&&... args); char* FindBrackets(char* src, char* dst); const DSPOPCTemplate* FindOpcode(std::string name, size_t par_count, OpcodeType type); @@ -104,7 +123,6 @@ private: std::vector m_output_buffer; std::string m_include_dir; - std::string m_cur_line; u32 m_cur_addr = 0; int m_total_size = 0; @@ -112,7 +130,6 @@ private: LabelMap m_labels; - u32 m_code_line = 0; bool m_failed = false; std::string m_last_error_str; AssemblerError m_last_error = AssemblerError::OK; @@ -122,7 +139,8 @@ private: segment_t m_cur_segment = SEGMENT_CODE; u32 m_segment_addr[SEGMENT_MAX] = {}; - int m_current_param = 0; const AssemblerSettings m_settings; + + LocationContext m_location; }; } // namespace DSP diff --git a/Source/Core/Core/DSP/DSPCodeUtil.cpp b/Source/Core/Core/DSP/DSPCodeUtil.cpp index 98e8c44d59..9a33edc9ba 100644 --- a/Source/Core/Core/DSP/DSPCodeUtil.cpp +++ b/Source/Core/Core/DSP/DSPCodeUtil.cpp @@ -35,13 +35,7 @@ bool Assemble(const std::string& text, std::vector& code, bool force) // TODO: fix the terrible api of the assembler. DSPAssembler assembler(settings); - if (!assembler.Assemble(text, code)) - { - std::cerr << assembler.GetErrorString() << std::endl; - return false; - } - - return true; + return assembler.Assemble(text, code); } bool Disassemble(const std::vector& code, bool line_numbers, std::string& text)