From ed851a24951417b2493211835f9e488448890be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karel=20Ko=C4=8D=C3=AD?= Date: Mon, 15 Jan 2018 14:36:31 +0100 Subject: Fix SRA and SRAV instructions This implementation is correct one but there is no guarantee that it will work with all compilers so we should always check on given platform that tests pass (and potentially fix it). --- instructions.md | 5 ++--- qtmips_machine/alu.cpp | 8 +++---- qtmips_machine/tests/testalu.cpp | 44 ++++++++++++++++++++++++++++++++++++++- qtmips_machine/tests/testcore.cpp | 2 +- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/instructions.md b/instructions.md index f9666ec..4340a7a 100644 --- a/instructions.md +++ b/instructions.md @@ -4,7 +4,6 @@ This is list of all MIPS1 instructions and their implementation status in QtMips Explanation of checkboxes: * [ ] Not tested -* [?] Somewhat tested but not sure about correctness of implementation * [x] Tested CPU Arithmetic Instruction @@ -98,8 +97,8 @@ CPU Shift Instructions ---------------------- * [x] SLL * [x] SLLV -* [?] SRA -* [?] SRAV +* [x] SRA +* [x] SRAV * [x] SRL * [x] SRLV diff --git a/qtmips_machine/alu.cpp b/qtmips_machine/alu.cpp index 0d40bf0..f266c9b 100644 --- a/qtmips_machine/alu.cpp +++ b/qtmips_machine/alu.cpp @@ -11,15 +11,15 @@ std::uint32_t machine::alu_operate(enum AluOp operation, std::uint32_t s, std::u case ALU_OP_SRL: return t >> sa; case ALU_OP_SRA: - // TODO is this correct implementation? (Should we be masking top most bit?) - return ((t & 0x7fffffff) >> sa) | (t & 0x80000000); + // Note: This might be broken with some compilers but works with gcc + return (std::int32_t)t >> sa; case ALU_OP_SLLV: return t << s; case ALU_OP_SRLV: return t >> s; case ALU_OP_SRAV: - // TODO is this correct implementation? (Should we be masking top most bit?) - return ((t & 0x7fffffff) >> s) | (t & 0x80000000); + // Note: same note as in case of SRA + return (std::int32_t)t >> s; case ALU_OP_JR: case ALU_OP_JALR: // Do nothing as we solve this when we are handling program counter in instruction decode (handle_pc) diff --git a/qtmips_machine/tests/testalu.cpp b/qtmips_machine/tests/testalu.cpp index 7668679..a701b64 100644 --- a/qtmips_machine/tests/testalu.cpp +++ b/qtmips_machine/tests/testalu.cpp @@ -13,7 +13,49 @@ void MachineTests::alu_data() { QTest::addColumn("regs_res"); QTest::addColumn("res"); - // TODO SLL-SRAV + QTest::newRow("SLL") << ALU_OP_SLL \ + << (std::uint32_t)0 \ + << (std::uint32_t)0x80000001 \ + << (std::uint8_t)3 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0x8; + QTest::newRow("SRL") << ALU_OP_SRL \ + << (std::uint32_t)0 \ + << (std::uint32_t)0x80000008 \ + << (std::uint8_t)3 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0x10000001; + QTest::newRow("SRA") << ALU_OP_SRA \ + << (std::uint32_t)0 \ + << (std::uint32_t)0x80000008 \ + << (std::uint8_t)3 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0xF0000001; + QTest::newRow("SLLV") << ALU_OP_SLLV \ + << (std::uint32_t)3 \ + << (std::uint32_t)0x80000001 \ + << (std::uint8_t)0 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0x8; + QTest::newRow("SRLV") << ALU_OP_SRLV \ + << (std::uint32_t)3 \ + << (std::uint32_t)0x80000008 \ + << (std::uint8_t)0 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0x10000001; + QTest::newRow("SRAV") << ALU_OP_SRAV \ + << (std::uint32_t)3 \ + << (std::uint32_t)0x80000008 \ + << (std::uint8_t)0 \ + << Registers() \ + << Registers() \ + << (std::uint32_t)0xF0000001; + // JR and JALR should have no effect and we test that in core (it really doesn't make sense to test it here) QTest::newRow("MOVZ") << ALU_OP_MOVZ \ << (std::uint32_t)22 \ << (std::uint32_t)0 \ diff --git a/qtmips_machine/tests/testcore.cpp b/qtmips_machine/tests/testcore.cpp index 1e5b502..c38eb26 100644 --- a/qtmips_machine/tests/testcore.cpp +++ b/qtmips_machine/tests/testcore.cpp @@ -82,7 +82,7 @@ static void core_regs_data() { regs_init.write_gp(24, 0x800000f0); regs_init.write_gp(25, 3); Registers regs_res(regs_init); - regs_res.write_gp(26, 0x8000001e); + regs_res.write_gp(26, 0xF000001e); QTest::newRow("SRA") << Instruction(0, 0, 24, 26, 3, 3) \ << regs_init \ << regs_res; -- cgit v1.2.3