From d7d9860051a9a9eb2c6f11684535ac65cce38eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karel=20Ko=C4=8D=C3=AD?= Date: Mon, 15 Jan 2018 15:22:44 +0100 Subject: Cleanup some todos in code --- qtmips_machine/alu.cpp | 2 +- qtmips_machine/alu.h | 1 - qtmips_machine/cache.h | 10 ++-------- qtmips_machine/core.cpp | 7 ++----- qtmips_machine/programloader.cpp | 6 ++---- qtmips_machine/qtmipsmachine.cpp | 1 - qtmips_machine/qtmipsmachine.h | 1 - qtmips_machine/tests/testinstruction.cpp | 8 +++++--- 8 files changed, 12 insertions(+), 24 deletions(-) diff --git a/qtmips_machine/alu.cpp b/qtmips_machine/alu.cpp index f266c9b..24b1c02 100644 --- a/qtmips_machine/alu.cpp +++ b/qtmips_machine/alu.cpp @@ -25,7 +25,7 @@ std::uint32_t machine::alu_operate(enum AluOp operation, std::uint32_t s, std::u // Do nothing as we solve this when we are handling program counter in instruction decode (handle_pc) return 0; case ALU_OP_MOVZ: - // We do this just to implement valid alu operation but we have to evaluate comparison way before this to disable register write + // We do this just to implement valid alu operation but we have to evaluate comparison outside of this function to disable register write return t == 0 ? s : 0; case ALU_OP_MOVN: // Same note as for MOVZ applies here diff --git a/qtmips_machine/alu.h b/qtmips_machine/alu.h index 51cae28..13f3d89 100644 --- a/qtmips_machine/alu.h +++ b/qtmips_machine/alu.h @@ -8,7 +8,6 @@ namespace machine { -// TODO Any other operations? We seems to be missing a lot of them. enum AluOp : std::uint8_t { ALU_OP_SLL = 0, ALU_OP_SRL = 2, diff --git a/qtmips_machine/cache.h b/qtmips_machine/cache.h index 638aa10..1bf82bf 100644 --- a/qtmips_machine/cache.h +++ b/qtmips_machine/cache.h @@ -2,21 +2,15 @@ #define CACHE_H #include +#include namespace machine { class Cache : public MemoryAccess { public: - Cache(Memory *m); + Cache(Memory *m, MachineConfigCache *c); }; -class CacheAssociative : public Cache { -public: - CacheAssociative(Memory *m); -}; - -// TODO other chaches - } #endif // CACHE_H diff --git a/qtmips_machine/core.cpp b/qtmips_machine/core.cpp index 264e871..9ca7b9d 100644 --- a/qtmips_machine/core.cpp +++ b/qtmips_machine/core.cpp @@ -107,8 +107,7 @@ struct Core::dtDecode Core::decode(const struct dtFetch &dt) { emit instruction_decoded(dt.inst); const struct DecodeMap &dec = dmap[dt.inst.opcode()]; if (!(dec.flags & DM_SUPPORTED)) - // TODO message - throw QTMIPS_EXCEPTION(UnsupportedInstruction, "", ""); + throw QTMIPS_EXCEPTION(UnsupportedInstruction, "Instruction with following opcode is not supported", QString::number(dt.inst.opcode(), 16)); return { .inst = dt.inst, @@ -122,16 +121,14 @@ struct Core::dtDecode Core::decode(const struct dtFetch &dt) { .val_rs = regs->read_gp(dt.inst.rs()), .val_rt = regs->read_gp(dt.inst.rt()), }; - // TODO on jump there should be delay slot. Does processor addes it or compiler. And do we care? } struct Core::dtExecute Core::execute(const struct dtDecode &dt) { emit instruction_executed(dt.inst); // Handle conditional move (we have to change regwrite signal if conditional is not met) - // TODO can't we do this some cleaner way? bool regwrite = dt.regwrite; - if (dt.inst.opcode() == 0 && ((dt.inst.funct() == 10 && dt.val_rt != 0) || (dt.inst.funct() == 11 && dt.val_rt == 0))) + if (dt.inst.opcode() == 0 && ((dt.inst.funct() == ALU_OP_MOVZ && dt.val_rt != 0) || (dt.inst.funct() == ALU_OP_MOVN && dt.val_rt == 0))) regwrite = false; std::uint32_t alu_sec = dt.val_rt; diff --git a/qtmips_machine/programloader.cpp b/qtmips_machine/programloader.cpp index fd0679d..c83a30f 100644 --- a/qtmips_machine/programloader.cpp +++ b/qtmips_machine/programloader.cpp @@ -36,7 +36,7 @@ ProgramLoader::ProgramLoader(const char *file) { throw QTMIPS_EXCEPTION(Input, "Getting elf class failed", elf_errmsg(-1)); if (elf_class != ELFCLASS32) throw QTMIPS_EXCEPTION(Input, "Only supported architecture is 32bit", ""); - // TODO check endianity! + // TODO We should check in what endianity elf file is coded in // Get number of program sections in elf file if (elf_getphdrnum(this->elf, &this->n_secs)) @@ -46,7 +46,6 @@ ProgramLoader::ProgramLoader(const char *file) { throw QTMIPS_EXCEPTION(Input, "Elf program sections get failed", elf_errmsg(-1)); // We want only LOAD sections so we create map of those sections for (unsigned i = 1; i < this->n_secs; i++) { - // TODO handle endianity if (phdrs[i].p_type != PT_LOAD) continue; this->map.push_back(i); @@ -58,8 +57,7 @@ ProgramLoader::ProgramLoader(QString file) : ProgramLoader(file.toStdString().c_ ProgramLoader::~ProgramLoader() { // Close elf - // TODO fix (this results to segfault, there is probably somethig passed to it on stack or something) - //elf_end(this->elf); + elf_end(this->elf); // Close file close(this->fd); } diff --git a/qtmips_machine/qtmipsmachine.cpp b/qtmips_machine/qtmipsmachine.cpp index c60cdce..456640a 100644 --- a/qtmips_machine/qtmipsmachine.cpp +++ b/qtmips_machine/qtmipsmachine.cpp @@ -66,7 +66,6 @@ bool QtMipsMachine::exited() { // We don't allow to call control methods when machine exited or if it's busy // We rather silently fail. -// TODO wouldn't be error better? #define CTL_GUARD do { if (exited() || stat == ST_BUSY) return; } while(false) void QtMipsMachine::play() { diff --git a/qtmips_machine/qtmipsmachine.h b/qtmips_machine/qtmipsmachine.h index dba8932..05e2917 100644 --- a/qtmips_machine/qtmipsmachine.h +++ b/qtmips_machine/qtmipsmachine.h @@ -39,7 +39,6 @@ public: bool exited(); public slots: - // TODO handle speed void play(); void pause(); void step(); diff --git a/qtmips_machine/tests/testinstruction.cpp b/qtmips_machine/tests/testinstruction.cpp index 2cdf119..37fdd10 100644 --- a/qtmips_machine/tests/testinstruction.cpp +++ b/qtmips_machine/tests/testinstruction.cpp @@ -5,10 +5,10 @@ using namespace machine; // Test that we are correctly encoding instructions in constructor void MachineTests::instruction() { - QCOMPARE(Instruction(0x00), Instruction(0,0)); + QCOMPARE(Instruction(0x0), Instruction()); + QCOMPARE(Instruction(0x4432146), Instruction(1, 2, 3, 4, 5, 6)); + QCOMPARE(Instruction(0x4430004), Instruction(1, 2, 3, 4)); QCOMPARE(Instruction(0x4000002), Instruction(1, 2)); - // QCOMPARE(Instruction(0x4000002), Instruction(1, 2, 3, 4)); - // TODO other combinations } // Test that we are correctly decoding instruction fields @@ -25,3 +25,5 @@ void MachineTests::instruction_access() { QCOMPARE(i.immediate(), (std::uint16_t) 0xffff); QCOMPARE(i.address(), (std::uint32_t) 0x3ffffff); } + +// TODO test to_str -- cgit v1.2.3