Reworking the instruction history

sources/CPU/CPU.cpp

@@ -211,86 +211,69 @@ namespace ComSquare::CPU
 		return cycles;
 	}
 
+	uint24_t CPU::_getValueAddr(Instruction &instruction)
+	{
+		switch (instruction.addressingMode) {
+		case Implied:
+			return 0;
+		case Immediate8bits:
+			return this->_getImmediateAddr8Bits();
+		case ImmediateForA:
+			return this->_getImmediateAddrForA();
+		case ImmediateForX:
+			return this->_getImmediateAddrForX();
+
+		case Absolute:
+			return this->_getAbsoluteAddr();
+		case AbsoluteLong:
+			return this->_getAbsoluteLongAddr();
+		case AbsoluteIndirect:
+			return this->_getAbsoluteIndirectAddr();
+		case AbsoluteIndirectLong:
+			return this->_getAbsoluteIndirectLongAddr();
+
+		case DirectPage:
+			return this->_getDirectAddr();
+		case DirectPageIndirect:
+			return this->_getDirectIndirectAddr();
+		case DirectPageIndirectLong:
+			return this->_getDirectIndirectLongAddr();
+
+		case DirectPageIndexedByX:
+			return this->_getDirectIndexedByXAddr();
+		case DirectPageIndexedByY:
+			return this->_getDirectIndexedByYAddr();
+		case DirectPageIndirectIndexedByX:
+			return this->_getDirectIndirectIndexedXAddr();
+		case DirectPageIndirectIndexedByY:
+			return this->_getDirectIndirectIndexedYAddr();
+		case DirectPageIndirectIndexedByYLong:
+			return this->_getDirectIndirectIndexedYLongAddr();
+
+		case AbsoluteIndexedByX:
+			return this->_getAbsoluteIndexedByXAddr();
+		case AbsoluteIndexedByXLong:
+			return this->_getAbsoluteIndexedByXLongAddr();
+		case AbsoluteIndexedByY:
+			return this->_getAbsoluteIndexedByYAddr();
+
+		case StackRelative:
+			return this->_getStackRelativeAddr();
+		case StackRelativeIndirectIndexedByY:
+			return this->_getStackRelativeIndirectIndexedYAddr();
+
+		case AbsoluteIndirectIndexedByX:
+			return this->_getAbsoluteIndirectIndexedByXAddr();
+		default:
+			return 0;
+		}
+	}
+
 	unsigned CPU::_executeInstruction(uint8_t opcode)
 	{
 		Instruction instruction = this->_instructions[opcode];
-		uint24_t valueAddr = 0;
-
 		this->_hasIndexCrossedPageBoundary = false;
-
-		switch (instruction.addressingMode) {
-		case Implied:
-			break;
-		case Immediate8bits:
-			valueAddr = this->_getImmediateAddr8Bits();
-			break;
-		case ImmediateForA:
-			valueAddr = this->_getImmediateAddrForA();
-			break;
-		case ImmediateForX:
-			valueAddr = this->_getImmediateAddrForX();
-			break;
-
-		case Absolute:
-			valueAddr = this->_getAbsoluteAddr();
-			break;
-		case AbsoluteLong:
-			valueAddr = this->_getAbsoluteLongAddr();
-			break;
-		case AbsoluteIndirect:
-			valueAddr = this->_getAbsoluteIndirectAddr();
-			break;
-		case AbsoluteIndirectLong:
-			valueAddr = this->_getAbsoluteIndirectLongAddr();
-			break;
-
-		case DirectPage:
-			valueAddr = this->_getDirectAddr();
-			break;
-		case DirectPageIndirect:
-			valueAddr = this->_getDirectIndirectAddr();
-			break;
-		case DirectPageIndirectLong:
-			valueAddr = this->_getDirectIndirectLongAddr();
-			break;
-
-		case DirectPageIndexedByX:
-			valueAddr = this->_getDirectIndexedByXAddr();
-			break;
-		case DirectPageIndexedByY:
-			valueAddr = this->_getDirectIndexedByYAddr();
-			break;
-		case DirectPageIndirectIndexedByX:
-			valueAddr = this->_getDirectIndirectIndexedXAddr();
-			break;
-		case DirectPageIndirectIndexedByY:
-			valueAddr = this->_getDirectIndirectIndexedYAddr();
-			break;
-		case DirectPageIndirectIndexedByYLong:
-			valueAddr = this->_getDirectIndirectIndexedYLongAddr();
-			break;
-
-		case AbsoluteIndexedByX:
-			valueAddr = this->_getAbsoluteIndexedByXAddr();
-			break;
-		case AbsoluteIndexedByXLong:
-			valueAddr = this->_getAbsoluteIndexedByXLongAddr();
-			break;
-		case AbsoluteIndexedByY:
-			valueAddr = this->_getAbsoluteIndexedByYAddr();
-			break;
-
-		case StackRelative:
-			valueAddr = this->_getStackRelativeAddr();
-			break;
-		case StackRelativeIndirectIndexedByY:
-			valueAddr = this->_getStackRelativeIndirectIndexedYAddr();
-			break;
-
-		case AbsoluteIndirectIndexedByX:
-			valueAddr = this->_getAbsoluteIndirectIndexedByXAddr();
-			break;
-		}
+		uint24_t valueAddr = this->_getValueAddr(instruction);
 
 		return instruction.cycleCount + (this->*instruction.call)(valueAddr, instruction.addressingMode);
 	}

sources/CPU/CPU.hpp

@@ -258,6 +258,11 @@ namespace ComSquare::CPU
 		//! @return The number of CPU cycles that the instruction took.
 		virtual unsigned _executeInstruction(uint8_t opcode);
 
+		//! @brief Get the parameter address of an instruction from it's addressing mode.
+		//! @info The current program counter should point to the instruction's opcode + 1.
+		//! @return The address of the data to read on the instruction.
+		uint24_t _getValueAddr(Instruction &instruction);
+
 		//! @brief Break instruction - Causes a software break. The PC is loaded from a vector table.
 		int BRK(uint24_t, AddressingMode);
 		//! @brief Co-Processor Enable instruction - Causes a software break. The PC is loaded from a vector table.

sources/Debugger/CPU/CPUDebug.cpp

@@ -42,6 +42,12 @@ namespace ComSquare::Debugger
 		this->_ui.stackView->verticalHeader()->setSectionResizeMode (QHeaderView::Fixed);
 		this->_ui.stackView->verticalHeader()->setHighlightSections(false);
 
+
+		this->_ui.history->setModel(&this->_historyModel);
+		this->_ui.history->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents);
+		this->_ui.history->verticalHeader()->setSectionResizeMode(QHeaderView::Fixed);
+		this->_ui.history->verticalHeader()->hide();
+
 		QMainWindow::connect(this->_ui.actionPause, &QAction::triggered, this, &CPUDebug::pause);
 		QMainWindow::connect(this->_ui.actionStep, &QAction::triggered, this, &CPUDebug::step);
 		QMainWindow::connect(this->_ui.actionNext, &QAction::triggered, this, &CPUDebug::next);
@@ -108,7 +114,10 @@ namespace ComSquare::Debugger
 		uint24_t pc = (this->_registers.pbr << 16u) | (this->_registers.pc - 1u);
 		DisassemblyContext ctx = this->_getDisassemblyContext();
 		DisassembledInstruction instruction = this->_parseInstruction(pc, ctx);
-		this->_ui.logger->append((instruction.toString() + " -  " + Utility::to_hex(opcode)).c_str());
+		this->_registers.pc--;
+		this->_historyModel.log({opcode, instruction.name, instruction.argument, this->getProceededParameters()});
+		this->_ui.history->scrollToBottom();
+		this->_registers.pc++;
 		unsigned ret = CPU::_executeInstruction(opcode);
 		this->_updateRegistersPanel();
 		return ret;
@@ -204,7 +213,7 @@ namespace ComSquare::Debugger
 
 	void CPUDebug::clearHistory()
 	{
-		this->_ui.logger->clear();
+		this->_historyModel.clear();
 	}
 
 	void CPUDebug::_updateDisassembly(uint24_t start, uint24_t refreshSize)
@@ -260,6 +269,19 @@ namespace ComSquare::Debugger
 	{
 		return this->_registers.s;
 	}
+
+	std::string CPUDebug::getProceededParameters()
+	{
+		uint24_t pac = this->_registers.pac;
+		this->_bus->forceSilence = true;
+		Instruction instruction = this->_instructions[this->readPC()];
+		uint24_t valueAddr = this->_getValueAddr(instruction);
+		this->_registers.pac = pac;
+		this->_bus->forceSilence = false;
+		if (instruction.size == 1)
+			return "";
+		return "[" + Utility::to_hex(valueAddr, Utility::AsmPrefix) + "]";
+	}
 }
 
 DisassemblyModel::DisassemblyModel(ComSquare::Debugger::CPUDebug &cpu) : QAbstractTableModel(), _cpu(cpu){ }
@@ -276,21 +298,28 @@ int DisassemblyModel::rowCount(const QModelIndex &) const
 
 QVariant DisassemblyModel::data(const QModelIndex &index, int role) const
 {
-	if (role != Qt::DisplayRole && role != Qt::DecorationRole)
-		return QVariant();
 	ComSquare::Debugger::DisassembledInstruction instruction = this->_cpu.disassembledInstructions[index.row()];
-	if (role == Qt::DecorationRole) {
-		if (index.column() == 3 && instruction.level == ComSquare::Debugger::TrustLevel::Unsafe)
+
+	switch (role) {
+	case Qt::DecorationRole:
+		if (index.column() == 2 && instruction.level == ComSquare::Debugger::TrustLevel::Unsafe)
 			return QColor(Qt::yellow);
-		if (index.column() == 3 && instruction.level == ComSquare::Debugger::TrustLevel::Compromised)
+		if (index.column() == 2 && instruction.level == ComSquare::Debugger::TrustLevel::Compromised)
 			return QColor(Qt::red);
 		return QVariant();
-	}
-	switch (index.column()) {
-	case 0:
-		return QString(instruction.name.c_str());
-	case 1:
-		return QString(instruction.argument.c_str());
+	case Qt::DisplayRole:
+		switch (index.column()) {
+		case 0:
+			return QString(instruction.name.c_str());
+		case 1:
+			return QString(instruction.argument.c_str());
+		case 3:
+			if (instruction.address != this->_cpu.getPC())
+				return QVariant();
+			return QString(this->_cpu.getProceededParameters().c_str());
+		default:
+			return QVariant();
+		}
 	default:
 		return QVariant();
 	}
@@ -377,3 +406,71 @@ QVariant StackModel::headerData(int section, Qt::Orientation orientation, int ro
 	uint16_t addr = section * 2;
 	return QString(ComSquare::Utility::to_hex(addr, ComSquare::Utility::HexString::NoPrefix).c_str());
 }
+
+HistoryModel::HistoryModel() = default;
+
+int HistoryModel::rowCount(const QModelIndex &) const
+{
+	return this->_instructions.size();
+}
+
+int HistoryModel::columnCount(const QModelIndex &) const
+{
+	return 4;
+}
+
+QVariant HistoryModel::data(const QModelIndex &index, int role) const
+{
+	if (role == Qt::TextAlignmentRole)
+		return Qt::AlignCenter;
+	if (role != Qt::DisplayRole)
+		return QVariant();
+
+	ComSquare::Debugger::ExecutedInstruction instruction = this->_instructions[index.row()];
+	switch (index.column()) {
+	case 0:
+		return QString(ComSquare::Utility::to_hex(instruction.opcode, ComSquare::Utility::NoPrefix).c_str());
+	case 1:
+		return QString(instruction.name.c_str());
+	case 2:
+		return QString(instruction.params.c_str());
+	case 3:
+		return QString(instruction.proceededParams.c_str());
+	default:
+		return QVariant();
+	}
+}
+
+QVariant HistoryModel::headerData(int section, Qt::Orientation orientation, int role) const
+{
+	if (orientation == Qt::Vertical || role != Qt::DisplayRole)
+		return QVariant();
+	switch (section) {
+	case 0:
+		return QString("OP");
+	case 1:
+		return QString("INST");
+	case 2:
+		return QString("Parameter");
+	case 3:
+		return QString("Data Addr");
+	default:
+		return QVariant();
+	}
+}
+
+void HistoryModel::log(const ComSquare::Debugger::ExecutedInstruction& instruction)
+{
+	int row = this->_instructions.size();
+	this->beginInsertRows(QModelIndex(), row, row);
+	this->_instructions.push_back(instruction);
+	this->insertRow(row);
+	this->endInsertRows();
+}
+
+void HistoryModel::clear()
+{
+	this->beginResetModel();
+	this->_instructions.clear();
+	this->endResetModel();
+}

sources/Debugger/CPU/CPUDebug.hpp

@@ -15,6 +15,18 @@
 namespace ComSquare::Debugger
 {
 	class CPUDebug;
+
+	//! @brief An instruction that has already been executed. Used for the history viewer
+	struct ExecutedInstruction {
+		//! @brief Opcode of the instruction
+		uint8_t opcode;
+		//! @brief The name of the instruction
+		std::string name;
+		//! @brief Readable parameters (disassembly style)
+		std::string params;
+		//! @brief The address to read from after processing the parameter.
+		std::string proceededParams;
+	};
 }
 
 //! @brief The qt model that show the stack.
@@ -40,6 +52,33 @@ public:
 	QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 };
 
+//! @brief The qt model that show the history.
+class HistoryModel : public QAbstractTableModel
+{
+Q_OBJECT
+private:
+	std::vector<ComSquare::Debugger::ExecutedInstruction> _instructions = {};
+public:
+	HistoryModel();
+	HistoryModel(const HistoryModel &) = delete;
+	const HistoryModel &operator=(const HistoryModel &) = delete;
+	~HistoryModel() override = default;
+
+	//! @brief Log a new instruction
+	void log(const ComSquare::Debugger::ExecutedInstruction &);
+	//! @brief Remove every instructions of the history.
+	void clear();
+
+	//! @brief The number of row the table has.
+	int rowCount(const QModelIndex &parent) const override;
+	//! @brief The number of column the table has.
+	int columnCount(const QModelIndex &parent) const override;
+	//! @brief Return a data representing the table cell.
+	QVariant data(const QModelIndex &index, int role) const override;
+	//! @brief Override the headers to use hex values.
+	QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
+};
+
 //! @brief The qt model that show the disassembly.
 class DisassemblyModel : public QAbstractTableModel
 {
@@ -139,6 +178,8 @@ namespace ComSquare::Debugger
 		RowPainter _painter;
 		//! @brief The stack viewer's model.
 		StackModel _stackModel;
+		//! @brief The history model.
+		HistoryModel _historyModel;
 		//! @brief If this is set to true, the execution of the CPU will be paused.
 		bool _isPaused = true;
 		//! @brief If this is set to true, the CPU will execute one instruction and pause itself.
@@ -220,6 +261,8 @@ namespace ComSquare::Debugger
 		std::vector<DisassembledInstruction> disassembledInstructions;
 		//! @brief The list of breakpoints the user has set.
 		std::vector<Breakpoint> breakpoints;
+		//! @brief Get a string representing the actual value of the arguments of the next instruction to execute.
+		std::string getProceededParameters();
 		//! @brief Return the current program counter of this CPU.
 		uint24_t getPC();
 		//! @brief Return the current stack pointer.

sources/Debugger/MemoryBusDebug.cpp

@@ -150,10 +150,14 @@ namespace ComSquare::Debugger
 
 	uint8_t MemoryBusDebug::read(uint24_t addr, bool silence)
 	{
-		if (!silence) {
+		if (!silence && !forceSilence) {
 			auto accessor = this->getAccessor(addr);
-			uint8_t value = accessor->read(addr - accessor->getStart());
-			this->_model.log(BusLog(false, addr, accessor, value, value));
+			if (!accessor) {
+				this->_model.log(BusLog(true, addr, accessor, this->_openBus, this->_openBus));
+			} else {
+				uint8_t value = accessor->read(addr - accessor->getStart());
+				this->_model.log(BusLog(false, addr, accessor, value, value));
+			}
 		}
 		return MemoryBus::read(addr);
 	}
@@ -167,7 +171,8 @@ namespace ComSquare::Debugger
 		} catch (InvalidAddress &) {
 			value = 0;
 		}
-		this->_model.log(BusLog(true, addr, accessor, value, data));
+		if (!forceSilence)
+			this->_model.log(BusLog(true, addr, accessor, value, data));
 		MemoryBus::write(addr, data);
 	}
 

sources/Memory/MemoryBus.hpp

@@ -22,20 +22,22 @@ namespace ComSquare
 			//! @brief The list of components registered inside the bus. Every components that can read/write to a public address should be in this vector.
 			std::vector<std::shared_ptr<AMemory>> _memoryAccessors;
 
-			//! @brief The last value read via the memory bus.
-			uint8_t _openBus = 0;
-
 			//! @brief WRam, CPU, PPU & APU registers are mirrored to all banks of Q1 & Q3. This function is used for the mirroring.
 			//! @param console All the components.
 			//! @param i Base address for the mirrors.
 			void _mirrorComponents(SNES &console, unsigned i);
-
+		protected:
+			//! @brief The last value read via the memory bus.
+			uint8_t _openBus = 0;
 		public:
 			MemoryBus() = default;
 			MemoryBus(const MemoryBus &) = default;
 			MemoryBus &operator=(const MemoryBus &) = default;
 			~MemoryBus() = default;
 
+			//! @brief Force silencing read to the bus.
+			bool forceSilence = false;
+
 			//! @brief Read data at a global address.
 			//! @param addr The address to read from.
 			//! @param silence Disable login to the memory bus's debugger (if enabled). Should only be used by other debuggers.

ui/cpu.ui

@@ -181,16 +181,6 @@
     </item>
     <item row="1" column="1">
      <layout class="QGridLayout" name="gridLayout">
-      <item row="1" column="0">
-       <widget class="QTextBrowser" name="logger"/>
-      </item>
-      <item row="2" column="0">
-       <widget class="QPushButton" name="clear">
-        <property name="text">
-         <string>Clear History</string>
-        </property>
-       </widget>
-      </item>
       <item row="0" column="0">
        <widget class="QLabel" name="loggerLabel">
         <property name="text">
@@ -201,6 +191,16 @@
         </property>
        </widget>
       </item>
+      <item row="2" column="0">
+       <widget class="QPushButton" name="clear">
+        <property name="text">
+         <string>Clear History</string>
+        </property>
+       </widget>
+      </item>
+      <item row="1" column="0">
+       <widget class="QTableView" name="history"/>
+      </item>
      </layout>
     </item>
     <item row="1" column="2">

ui/ui_cpu.h

@@ -23,7 +23,6 @@
 #include <QtWidgets/QMainWindow>
 #include <QtWidgets/QPushButton>
 #include <QtWidgets/QTableView>
-#include <QtWidgets/QTextBrowser>
 #include <QtWidgets/QToolBar>
 #include <QtWidgets/QWidget>
 
@@ -61,9 +60,9 @@ public:
     QLabel *emulationModeLabel;
     QCheckBox *emulationModeCheckBox;
     QGridLayout *gridLayout;
-    QTextBrowser *logger;
-    QPushButton *clear;
     QLabel *loggerLabel;
+    QPushButton *clear;
+    QTableView *history;
     QGroupBox *formGroupBox;
     QFormLayout *formLayout_2;
     QLabel *negativeLabel;
@@ -245,21 +244,21 @@ public:
 
         gridLayout = new QGridLayout();
         gridLayout->setObjectName(QString::fromUtf8("gridLayout"));
-        logger = new QTextBrowser(centralwidget);
-        logger->setObjectName(QString::fromUtf8("logger"));
+        loggerLabel = new QLabel(centralwidget);
+        loggerLabel->setObjectName(QString::fromUtf8("loggerLabel"));
+        loggerLabel->setAlignment(Qt::AlignCenter);
 
-        gridLayout->addWidget(logger, 1, 0, 1, 1);
+        gridLayout->addWidget(loggerLabel, 0, 0, 1, 1);
 
         clear = new QPushButton(centralwidget);
         clear->setObjectName(QString::fromUtf8("clear"));
 
         gridLayout->addWidget(clear, 2, 0, 1, 1);
 
-        loggerLabel = new QLabel(centralwidget);
-        loggerLabel->setObjectName(QString::fromUtf8("loggerLabel"));
-        loggerLabel->setAlignment(Qt::AlignCenter);
+        history = new QTableView(centralwidget);
+        history->setObjectName(QString::fromUtf8("history"));
 
-        gridLayout->addWidget(loggerLabel, 0, 0, 1, 1);
+        gridLayout->addWidget(history, 1, 0, 1, 1);
 
 
         gridLayout_3->addLayout(gridLayout, 1, 1, 1, 1);
@@ -419,8 +418,8 @@ public:
         xIndexLabel->setText(QCoreApplication::translate("CPUView", "X Index", nullptr));
         yIndexLabel->setText(QCoreApplication::translate("CPUView", "Y Index", nullptr));
         emulationModeLabel->setText(QCoreApplication::translate("CPUView", "Emulation mode", nullptr));
-        clear->setText(QCoreApplication::translate("CPUView", "Clear History", nullptr));
         loggerLabel->setText(QCoreApplication::translate("CPUView", "Instructions History", nullptr));
+        clear->setText(QCoreApplication::translate("CPUView", "Clear History", nullptr));
         formGroupBox->setTitle(QCoreApplication::translate("CPUView", "Flags", nullptr));
         negativeLabel->setText(QCoreApplication::translate("CPUView", "Memory/Accumulator Select (M)", nullptr));
         zeroLabel->setText(QCoreApplication::translate("CPUView", "Index Select (X) / Break (B)", nullptr));