From 78c115041223cea8430393d781162cb730c74d53 Mon Sep 17 00:00:00 2001 From: Amneesh Singh Date: Tue, 11 Jun 2024 23:03:44 +0530 Subject: [PATCH] cpu/{arm|thumb}(chore): change how branch disassembly happens Signed-off-by: Amneesh Singh --- include/cpu/arm/instruction.hh | 2 +- include/cpu/thumb/instruction.hh | 2 +- src/cpu/arm/disassembler.cc | 9 +++++---- src/cpu/arm/exec.cc | 7 ++----- src/cpu/arm/instruction.cc | 9 +++------ src/cpu/thumb/disassembler.cc | 10 +++++----- tests/cpu/arm/exec.cc | 13 ++++++++++--- tests/cpu/arm/instruction.cc | 9 +++++---- tests/cpu/thumb/exec.cc | 6 +++--- tests/cpu/thumb/instruction.cc | 6 ++++-- 10 files changed, 39 insertions(+), 34 deletions(-) diff --git a/include/cpu/arm/instruction.hh b/include/cpu/arm/instruction.hh index 027bbe2..a2bb12c 100644 --- a/include/cpu/arm/instruction.hh +++ b/include/cpu/arm/instruction.hh @@ -26,7 +26,7 @@ struct BranchAndExchange { struct Branch { bool link; - uint32_t offset; + int32_t offset; }; struct Multiply { diff --git a/include/cpu/thumb/instruction.hh b/include/cpu/thumb/instruction.hh index 19c4a55..cc25c2d 100644 --- a/include/cpu/thumb/instruction.hh +++ b/include/cpu/thumb/instruction.hh @@ -282,7 +282,7 @@ struct Instruction { void exec(Cpu& cpu); #ifdef DISASSEMBLER - std::string disassemble(uint32_t pc = 0); + std::string disassemble(); #endif InstructionData data; diff --git a/src/cpu/arm/disassembler.cc b/src/cpu/arm/disassembler.cc index 1c9850e..092a0ed 100644 --- a/src/cpu/arm/disassembler.cc +++ b/src/cpu/arm/disassembler.cc @@ -1,7 +1,6 @@ #include "cpu/arm/instruction.hh" #include "util/bits.hh" #include -#include namespace matar::arm { std::string @@ -13,9 +12,11 @@ Instruction::disassemble() { [condition](BranchAndExchange& data) { return std::format("BX{} R{:d}", condition, data.rn); }, - [condition](Branch& data) { - return std::format( - "B{}{} 0x{:06X}", (data.link ? "L" : ""), condition, data.offset); + [condition](Branch& data) { + return std::format("B{}{} {:#06x}", + (data.link ? "L" : ""), + condition, + static_cast(data.offset + 2 * INSTRUCTION_SIZE)); }, [condition](Multiply& data) { if (data.acc) { diff --git a/src/cpu/arm/exec.cc b/src/cpu/arm/exec.cc index a6c2048..d63371e 100644 --- a/src/cpu/arm/exec.cc +++ b/src/cpu/arm/exec.cc @@ -40,17 +40,14 @@ Instruction::exec(Cpu& cpu) { if (state == State::Arm) rst_bit(cpu.pc, 1); - // pc is affected so flush the pipeline + // PC is affected so flush the pipeline cpu.is_flushed = true; }, [&cpu](Branch& data) { if (data.link) cpu.gpr[14] = cpu.pc - INSTRUCTION_SIZE; - // data.offset accounts for two instructions ahead when - // disassembling, so need to adjust - cpu.pc = - static_cast(cpu.pc) - 2 * INSTRUCTION_SIZE + data.offset; + cpu.pc += data.offset; // pc is affected so flush the pipeline cpu.is_flushed = true; diff --git a/src/cpu/arm/instruction.cc b/src/cpu/arm/instruction.cc index eb77310..8015390 100644 --- a/src/cpu/arm/instruction.cc +++ b/src/cpu/arm/instruction.cc @@ -1,6 +1,5 @@ #include "cpu/arm/instruction.hh" #include "util/bits.hh" -#include namespace matar::arm { Instruction::Instruction(uint32_t insn) @@ -13,13 +12,11 @@ Instruction::Instruction(uint32_t insn) // Branch } else if ((insn & 0x0E000000) == 0x0A000000) { - bool link = get_bit(insn, 24); - uint32_t offset = bit_range(insn, 0, 23); + bool link = get_bit(insn, 24); + int32_t offset = static_cast(bit_range(insn, 0, 23)); // lsh 2 and sign extend the 26 bit offset to 32 bits - offset = (static_cast(offset) << 8) >> 6; - - offset += 2 * INSTRUCTION_SIZE; + offset = (offset << 8) >> 6; data = Branch{ .link = link, .offset = offset }; diff --git a/src/cpu/thumb/disassembler.cc b/src/cpu/thumb/disassembler.cc index f8f325b..f180fc4 100644 --- a/src/cpu/thumb/disassembler.cc +++ b/src/cpu/thumb/disassembler.cc @@ -4,7 +4,7 @@ namespace matar::thumb { std::string -Instruction::disassemble(uint32_t pc) { +Instruction::disassemble() { return std::visit( overloaded{ [](MoveShiftedRegister& data) { @@ -133,16 +133,16 @@ Instruction::disassemble(uint32_t pc) { [](SoftwareInterrupt& data) { return std::format("SWI {:d}", data.vector); }, - [pc](ConditionalBranch& data) { + [](ConditionalBranch& data) { return std::format( "B{} #{:d}", stringify(data.condition), - static_cast(data.offset + pc + 2 * INSTRUCTION_SIZE)); + static_cast(data.offset + 2 * INSTRUCTION_SIZE)); }, - [pc](UnconditionalBranch& data) { + [](UnconditionalBranch& data) { return std::format( "B #{:d}", - static_cast(data.offset + pc + 2 * INSTRUCTION_SIZE)); + static_cast(data.offset + 2 * INSTRUCTION_SIZE)); }, [](LongBranchWithLink& data) { // duh this manual be empty for H = 0 diff --git a/tests/cpu/arm/exec.cc b/tests/cpu/arm/exec.cc index 579f04d..ab8d8bb 100644 --- a/tests/cpu/arm/exec.cc +++ b/tests/cpu/arm/exec.cc @@ -24,18 +24,25 @@ TEST_CASE_METHOD(CpuFixture, "Branch", TAG) { InstructionData data = Branch{ .link = false, .offset = 3489748 }; Branch* branch = std::get_if(&data); + // set PC to 48 + setr(15, 48); + exec(data); - CHECK(getr(15) == 3489748); + // 48 + offset + CHECK(getr(15) == 3489796); CHECK(getr(14) == 0); // with link reset(); + setr(15, 48); branch->link = true; exec(data); - CHECK(getr(15) == 3489748); - CHECK(getr(14) == 0 + INSTRUCTION_SIZE); + // 48 + offset + CHECK(getr(15) == 3489796); + // pc was set to 48 + CHECK(getr(14) == 48 - INSTRUCTION_SIZE); } TEST_CASE_METHOD(CpuFixture, "Multiply", TAG) { diff --git a/tests/cpu/arm/instruction.cc b/tests/cpu/arm/instruction.cc index c571e6b..713619f 100644 --- a/tests/cpu/arm/instruction.cc +++ b/tests/cpu/arm/instruction.cc @@ -31,15 +31,16 @@ TEST_CASE("Branch", TAG) { // last 24 bits = 8748995 // (8748995 << 8) >> 6 sign extended = 0xFE15FF0C - // Also +8 since PC is two instructions ahead - CHECK(b->offset == 0xFE15FF14); + CHECK(b->offset == static_cast(0xfe15ff0c)); CHECK(b->link == true); #ifdef DISASSEMBLER - CHECK(instruction.disassemble() == "BL 0xFE15FF14"); + // take prefetch into account + // offset + 8 = 0xfe15ff0c + 8 = -0x1ea00e4 + 8 + CHECK(instruction.disassemble() == "BL -0x1ea00ec"); b->link = false; - CHECK(instruction.disassemble() == "B 0xFE15FF14"); + CHECK(instruction.disassemble() == "B -0x1ea00ec"); #endif } diff --git a/tests/cpu/thumb/exec.cc b/tests/cpu/thumb/exec.cc index 6227966..980dca8 100644 --- a/tests/cpu/thumb/exec.cc +++ b/tests/cpu/thumb/exec.cc @@ -1,6 +1,5 @@ #include "cpu/cpu-fixture.hh" #include "cpu/thumb/instruction.hh" -#include "util/bits.hh" #include using namespace matar; @@ -531,8 +530,9 @@ TEST_CASE_METHOD(CpuFixture, "PC Relative Load", TAG) { InstructionData data = PcRelativeLoad{ .word = 0x578, .rd = 0 }; setr(15, 0x3003FD5); - // 0x3003FD5 + 0x578 - bus.write_word(0x300454D, 489753492); + // setting bit 0 for 0x3003FD5, we get 0x3003FD4 + // 0x3003FD4 + 0x578 + bus.write_word(0x300454C, 489753492); CHECK(getr(0) == 0); exec(data); diff --git a/tests/cpu/thumb/instruction.cc b/tests/cpu/thumb/instruction.cc index 49f0e11..a97182a 100644 --- a/tests/cpu/thumb/instruction.cc +++ b/tests/cpu/thumb/instruction.cc @@ -412,7 +412,8 @@ TEST_CASE("Conditional Branch", TAG) { CHECK(b->condition == Condition::LS); #ifdef DISASSEMBLER - // (-76 << 1) + PC (0) + 4 + // take prefetch into account + // offset + 4 = -152 + 4 CHECK(instruction.disassemble() == "BLS #-148"); #endif } @@ -439,7 +440,8 @@ TEST_CASE("Unconditional Branch") { REQUIRE(b->offset == -410); #ifdef DISASSEMBLER - // (2147483443 << 1) + PC(0) + 4 + // take prefetch into account + // offset + 4 = -410 + 4 CHECK(instruction.disassemble() == "B #-406"); #endif }