Merge pull request #5667 from ligfx/cleanupdspdisassembler

DSPDisassembler: various code cleanups

Source/Core/Core/DSP/DSPCodeUtil.cpp

@@ -56,7 +56,7 @@ bool Disassemble(const std::vector<u16>& code, bool line_numbers, std::string& t
   settings.decode_registers = true;
 
   DSPDisassembler disasm(settings);
-  bool success = disasm.Disassemble(0, code, 0x0000, text);
+  bool success = disasm.Disassemble(code, text);
   return success;
 }
 
@@ -79,9 +79,9 @@ bool Compare(const std::vector<u16>& code1, const std::vector<u16>& code2)
     {
       std::string line1, line2;
       u16 pc = i;
-      disassembler.DisassembleOpcode(&code1[0], 0x0000, 2, &pc, line1);
+      disassembler.DisassembleOpcode(&code1[0], &pc, line1);
       pc = i;
-      disassembler.DisassembleOpcode(&code2[0], 0x0000, 2, &pc, line2);
+      disassembler.DisassembleOpcode(&code2[0], &pc, line2);
       printf("!! %04x : %04x vs %04x - %s  vs  %s\n", i, code1[i], code2[i], line1.c_str(),
              line2.c_str());
     }
@@ -94,7 +94,7 @@ bool Compare(const std::vector<u16>& code1, const std::vector<u16>& code2)
     {
       u16 pc = i;
       std::string line;
-      disassembler.DisassembleOpcode(&longest[0], 0x0000, 2, &pc, line);
+      disassembler.DisassembleOpcode(&longest[0], &pc, line);
       printf("!! %s\n", line.c_str());
     }
   }

Source/Core/Core/DSP/DSPDisassembler.cpp

@@ -25,49 +25,21 @@ DSPDisassembler::DSPDisassembler(const AssemblerSettings& settings) : settings_(
 {
 }
 
-DSPDisassembler::~DSPDisassembler()
+bool DSPDisassembler::Disassemble(const std::vector<u16>& code, std::string& text)
 {
-  // Some old code for logging unknown ops.
+  if (code.size() > std::numeric_limits<u16>::max())
-  std::string filename = File::GetUserPath(D_DUMPDSP_IDX) + "UnkOps.txt";
-  std::ofstream uo(filename);
-  if (!uo)
-    return;
-
-  int count = 0;
-  for (const auto& entry : unk_opcodes)
   {
-    if (entry.second > 0)
+    text.append("; code too large for 16-bit addressing\n");
-    {
+    return false;
-      count++;
-      uo << StringFromFormat("OP%04x\t%d", entry.first, entry.second);
-      for (int j = 15; j >= 0; j--)  // print op bits
-      {
-        if ((j & 0x3) == 3)
-          uo << "\tb";
-
-        uo << StringFromFormat("%d", (entry.first >> j) & 0x1);
   }
 
-      uo << "\n";
+  for (u16 pc = 0; pc < code.size();)
-    }
-  }
-
-  uo << StringFromFormat("Unknown opcodes count: %d\n", count);
-}
-
-bool DSPDisassembler::Disassemble(int start_pc, const std::vector<u16>& code, int base_addr,
-                                  std::string& text)
   {
-  const char* tmp1 = "tmp1.bin";
+    if (!DisassembleOpcode(code.data(), &pc, text))
-
+      return false;
-  // First we have to dump the code to a bin file.
+    text.append("\n");
-  {
-    File::IOFile f(tmp1, "wb");
-    f.WriteArray(&code[0], code.size());
   }
-
+  return true;
-  // Run the two passes.
-  return DisassembleFile(tmp1, base_addr, 1, text) && DisassembleFile(tmp1, base_addr, 2, text);
 }
 
 std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2)
@@ -163,18 +135,8 @@ std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u1
   return buf;
 }
 
-static std::string MakeLowerCase(std::string in)
+bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest)
 {
-  std::transform(in.begin(), in.end(), in.begin(), ::tolower);
-
-  return in;
-}
-
-bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pass, u16* pc,
-                                        std::string& dest)
-{
-  std::string buf(" ");
-
   if ((*pc & 0x7fff) >= 0x1000)
   {
     ++pc;
@@ -186,10 +148,9 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa
 
   // Find main opcode
   const DSPOPCTemplate* opc = FindOpInfoByOpcode(op1);
-  const DSPOPCTemplate fake_op = {"CW",    0x0000, 0x0000, DSP::Interpreter::nop,
+  const DSPOPCTemplate fake_op = {
-                                  nullptr, 1,      1,      {{P_VAL, 2, 0, 0, 0xffff}},
+      "CW",  0x0000, 0x0000, nullptr, nullptr, 1, 1, {{P_VAL, 2, 0, 0, 0xffff}},
-                                  false,   false,  false,  false,
+      false, false,  false,  false,   false};
-                                  false};
   if (!opc)
     opc = &fake_op;
 
@@ -217,7 +178,7 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa
   // printing
 
   if (settings_.show_pc)
-    buf += StringFromFormat("%04x ", *pc);
+    dest += StringFromFormat("%04x ", *pc);
 
   u16 op2;
 
@@ -226,87 +187,48 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa
   {
     op2 = binbuf[(*pc + 1) & 0x0fff];
     if (settings_.show_hex)
-      buf += StringFromFormat("%04x %04x ", op1, op2);
+      dest += StringFromFormat("%04x %04x ", op1, op2);
   }
   else
   {
     op2 = 0;
     if (settings_.show_hex)
-      buf += StringFromFormat("%04x      ", op1);
+      dest += StringFromFormat("%04x      ", op1);
   }
 
   std::string opname = opc->name;
-  if (settings_.lower_case_ops)
-    opname = MakeLowerCase(opname);
-
-  std::string ext_buf;
   if (is_extended)
-    ext_buf = StringFromFormat("%s%c%s", opname.c_str(), settings_.ext_separator, opc_ext->name);
+    opname += StringFromFormat("%c%s", settings_.ext_separator, opc_ext->name);
-  else
-    ext_buf = opname;
   if (settings_.lower_case_ops)
-    ext_buf = MakeLowerCase(ext_buf);
+    std::transform(opname.begin(), opname.end(), opname.begin(), ::tolower);
 
   if (settings_.print_tabs)
-    buf += StringFromFormat("%s\t", ext_buf.c_str());
+    dest += StringFromFormat("%s\t", opname.c_str());
   else
-    buf += StringFromFormat("%-12s", ext_buf.c_str());
+    dest += StringFromFormat("%-12s", opname.c_str());
 
   if (opc->param_count > 0)
-    buf += DisassembleParameters(*opc, op1, op2);
+    dest += DisassembleParameters(*opc, op1, op2);
 
   // Handle opcode extension.
   if (is_extended)
   {
     if (opc->param_count > 0)
-      buf += " ";
+      dest += " ";
 
-    buf += ": ";
+    dest += ": ";
 
     if (opc_ext->param_count > 0)
-      buf += DisassembleParameters(*opc_ext, op1, op2);
+      dest += DisassembleParameters(*opc_ext, op1, op2);
   }
 
   if (opc->opcode_mask == 0)
   {
     // unknown opcode
-    unk_opcodes[op1]++;
+    dest += "\t\t; *** UNKNOWN OPCODE ***";
-    buf += "\t\t; *** UNKNOWN OPCODE ***";
   }
 
-  if (is_extended)
+  *pc += is_extended ? opc_ext->size : opc->size;
-    *pc += opc_ext->size;
-  else
-    *pc += opc->size;
-
-  if (pass == 2)
-    dest.append(buf);
-
-  return true;
-}
-
-bool DSPDisassembler::DisassembleFile(const std::string& name, int base_addr, int pass,
-                                      std::string& output)
-{
-  File::IOFile in(name, "rb");
-  if (!in)
-  {
-    printf("gd_dis_file: No input\n");
-    return false;
-  }
-
-  const int size = ((int)in.GetSize() & ~1) / 2;
-  std::vector<u16> binbuf(size);
-  in.ReadArray(binbuf.data(), size);
-  in.Close();
-
-  // Actually do the disassembly.
-  for (u16 pc = 0; pc < size;)
-  {
-    DisassembleOpcode(binbuf.data(), base_addr, pass, &pc, output);
-    if (pass == 2)
-      output.append("\n");
-  }
 
   return true;
 }

Source/Core/Core/DSP/DSPDisassembler.h

@@ -12,7 +12,6 @@
 #include "Common/CommonTypes.h"
 
 #include "Core/DSP/DSPTables.h"
-#include "Core/DSP/LabelMap.h"
 
 namespace DSP
 {
@@ -34,23 +33,15 @@ class DSPDisassembler
 {
 public:
   explicit DSPDisassembler(const AssemblerSettings& settings);
-  ~DSPDisassembler();
 
-  bool Disassemble(int start_pc, const std::vector<u16>& code, int base_addr, std::string& text);
+  bool Disassemble(const std::vector<u16>& code, std::string& text);
 
   // Warning - this one is trickier to use right.
-  // Use pass == 2 if you're just using it by itself.
+  bool DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest);
-  bool DisassembleOpcode(const u16* binbuf, int base_addr, int pass, u16* pc, std::string& dest);
 
 private:
-  // Moves PC forward and writes the result to dest.
-  bool DisassembleFile(const std::string& name, int base_addr, int pass, std::string& output);
-
   std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2);
-  std::map<u16, int> unk_opcodes;
 
   const AssemblerSettings settings_;
-
-  LabelMap labels;
 };
 }  // namespace DSP

Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp

@@ -230,7 +230,7 @@ void AutoDisassembly(u16 start_addr, u16 end_addr)
     addr_to_line[addr] = line_counter;
 
     std::string buf;
-    if (!disasm.DisassembleOpcode(ptr, 0, 2, &addr, buf))
+    if (!disasm.DisassembleOpcode(ptr, &addr, buf))
     {
       ERROR_LOG(DSPLLE, "disasm failed at %04x", addr);
       break;

Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp

@@ -23,7 +23,7 @@ static bool RoundTrippableDissassemble(const std::vector<u16>& code, std::string
   settings.show_pc = false;
   DSP::DSPDisassembler disasm(settings);
 
-  return disasm.Disassemble(0x0000, code, 0x0000, text);
+  return disasm.Disassemble(code, text);
 }
 
 // This test goes from text ASM to binary to text ASM and once again back to binary.