From d62c9c8e6c487072cd7a1e2270e164f3d17b6357 Mon Sep 17 00:00:00 2001 From: Arusekk Date: Sun, 29 Oct 2023 00:34:54 +0200 Subject: [PATCH] retirement: Disable side effects on exception --- coreblocks/core.py | 1 + coreblocks/lsu/dummyLsu.py | 9 ++++++--- coreblocks/params/layouts.py | 7 ++++++- coreblocks/stages/retirement.py | 28 ++++++++++++++++++++++------ coreblocks/structs_common/csr.py | 20 +++++++++++++------- coreblocks/structs_common/rat.py | 5 +++-- test/lsu/test_dummylsu.py | 2 +- test/stages/test_retirement.py | 2 +- test/structs_common/test_csr.py | 2 +- test/test_core.py | 2 ++ 10 files changed, 56 insertions(+), 22 deletions(-) diff --git a/coreblocks/core.py b/coreblocks/core.py index 409ab3238..4da2a62eb 100644 --- a/coreblocks/core.py +++ b/coreblocks/core.py @@ -136,6 +136,7 @@ def elaborate(self, platform): rf_free=rf.free, precommit=self.func_blocks_unifier.get_extra_method(InstructionPrecommitKey()), exception_cause_get=self.exception_cause_register.get, + frat_rename=frat.rename, ) m.submodules.csr_generic = self.csr_generic diff --git a/coreblocks/lsu/dummyLsu.py b/coreblocks/lsu/dummyLsu.py index 497ef3355..195f25aeb 100644 --- a/coreblocks/lsu/dummyLsu.py +++ b/coreblocks/lsu/dummyLsu.py @@ -219,7 +219,8 @@ def elaborate(self, platform): m.submodules.internal = internal = LSUDummyInternals(self.gen_params, self.bus, current_instr) - result_ready = internal.result_ready | ((current_instr.exec_fn.op_type == OpType.FENCE) & current_instr.valid) + result_ready = Signal() + m.d.comb += result_ready.eq(internal.result_ready | ((current_instr.exec_fn.op_type == OpType.FENCE) & current_instr.valid)) @def_method(m, self.select, ~reserved) def _(): @@ -256,8 +257,10 @@ def _(): } @def_method(m, self.precommit) - def _(rob_id: Value): - with m.If( + def _(rob_id: Value, side_fx: Value): + with m.If(~side_fx): + m.d.comb += result_ready.eq(1) + with m.Elif( current_instr.valid & (rob_id == current_instr.rob_id) & (current_instr.exec_fn.op_type != OpType.FENCE) ): m.d.comb += internal.execute.eq(1) diff --git a/coreblocks/params/layouts.py b/coreblocks/params/layouts.py index 9386d11e0..3b6916285 100644 --- a/coreblocks/params/layouts.py +++ b/coreblocks/params/layouts.py @@ -111,7 +111,11 @@ def __init__(self, gen_params: GenParams): ] self.rat_rename_out = [("rp_s1", gen_params.phys_regs_bits), ("rp_s2", gen_params.phys_regs_bits)] - self.rat_commit_in = [("rl_dst", gen_params.isa.reg_cnt_log), ("rp_dst", gen_params.phys_regs_bits)] + self.rat_commit_in = [ + ("rl_dst", gen_params.isa.reg_cnt_log), + ("rp_dst", gen_params.phys_regs_bits), + ("side_fx", 1), + ] self.rat_commit_out = [("old_rp_dst", gen_params.phys_regs_bits)] @@ -175,6 +179,7 @@ class RetirementLayouts: def __init__(self, gen_params: GenParams): self.precommit = [ ("rob_id", gen_params.rob_entries_bits), + ("side_fx", 1), ] diff --git a/coreblocks/stages/retirement.py b/coreblocks/stages/retirement.py index f8f4aeac7..071d86fff 100644 --- a/coreblocks/stages/retirement.py +++ b/coreblocks/stages/retirement.py @@ -18,7 +18,8 @@ def __init__( free_rf_put: Method, rf_free: Method, precommit: Method, - exception_cause_get: Method + exception_cause_get: Method, + frat_rename: Method, ): self.gen_params = gen_params self.rob_peek = rob_peek @@ -28,6 +29,7 @@ def __init__( self.rf_free = rf_free self.precommit = precommit self.exception_cause_get = exception_cause_get + self.rename = frat_rename self.instret_csr = DoubleCounterCSR(gen_params, CSRAddress.INSTRET, CSRAddress.INSTRETH) @@ -36,18 +38,23 @@ def elaborate(self, platform): m.submodules.instret_csr = self.instret_csr + side_fx = Signal(reset=1) + with Transaction().body(m): # TODO: do we prefer single precommit call per instruction? # If so, the precommit method should send an acknowledge signal here. # Just calling once is not enough, because the insn might not be in relevant unit yet. rob_entry = self.rob_peek(m) - self.precommit(m, rob_id=rob_entry.rob_id) + self.precommit(m, rob_id=rob_entry.rob_id, side_fx=side_fx) with Transaction().body(m): rob_entry = self.rob_retire(m) # TODO: Trigger InterruptCoordinator (handle exception) when rob_entry.exception is set. with m.If(rob_entry.exception): + m.d.sync += side_fx.eq(0) + # TODO: only set mcause/trigger IC if cause is actual exception and not e.g. + # misprediction or pipeline flush after some fence.i or changing ISA mcause = self.gen_params.get(DependencyManager).get_dependency(GenericCSRRegistersKey()).mcause cause = self.exception_cause_get(m).cause entry = Signal(self.gen_params.isa.xlen) @@ -56,13 +63,22 @@ def elaborate(self, platform): mcause.write(m, entry) # set rl_dst -> rp_dst in R-RAT - rat_out = self.r_rat_commit(m, rl_dst=rob_entry.rob_data.rl_dst, rp_dst=rob_entry.rob_data.rp_dst) + rat_out = self.r_rat_commit(m, rl_dst=rob_entry.rob_data.rl_dst, rp_dst=rob_entry.rob_data.rp_dst, side_fx=side_fx) + + rp_freed = Signal(self.gen_params.phys_regs_bits) + with m.If(side_fx): + m.d.comb += rp_freed.eq(rat_out.old_rp_dst) + with m.Else(): + m.d.comb += rp_freed.eq(rob_entry.rob_data.rp_dst) + # free the phys_reg with computed value and restore old reg into FRAT as well + # TODO: are method priorities enough? + self.rename(m, rl_s1=0, rl_s2=0, rl_dst=rob_entry.rob_data.rl_dst, rp_dst=rat_out.old_rp_dst) - self.rf_free(m, rat_out.old_rp_dst) + self.rf_free(m, rp_freed) # put old rp_dst to free RF list - with m.If(rat_out.old_rp_dst): # don't put rp0 to free list - reserved to no-return instructions - self.free_rf_put(m, rat_out.old_rp_dst) + with m.If(rp_freed): # don't put rp0 to free list - reserved to no-return instructions + self.free_rf_put(m, rp_freed) self.instret_csr.increment(m) diff --git a/coreblocks/structs_common/csr.py b/coreblocks/structs_common/csr.py index 0f8eba862..1d9d74ec3 100644 --- a/coreblocks/structs_common/csr.py +++ b/coreblocks/structs_common/csr.py @@ -219,13 +219,13 @@ def elaborate(self, platform): done = Signal() accepted = Signal() exception = Signal() - rob_sfx_empty = Signal() + precommitting = Signal() current_result = Signal(self.gen_params.isa.xlen) instr = Record(self.csr_layouts.rs_data_layout + [("valid", 1)]) - m.d.comb += ready_to_process.eq(rob_sfx_empty & instr.valid & (instr.rp_s1 == 0)) + m.d.comb += ready_to_process.eq(precommitting & instr.valid & (instr.rp_s1 == 0)) # RISCV Zicsr spec Table 1.1 should_read_csr = Signal() @@ -251,6 +251,8 @@ def elaborate(self, platform): # Temporary, until privileged spec is implemented priv_level = Signal(PrivilegeLevel, reset=PrivilegeLevel.MACHINE) + exe_side_fx = Signal() + # Methods used within this Tranaction are CSRRegister internal _fu_(read|write) handlers which are always ready with Transaction().body(m, request=(ready_to_process & ~done)): with m.Switch(instr.csr): @@ -266,7 +268,8 @@ def elaborate(self, platform): with m.If(priv_valid): read_val = Signal(self.gen_params.isa.xlen) with m.If(should_read_csr & ~done): - m.d.comb += read_val.eq(read(m)) + with m.If(exe_side_fx): + m.d.comb += read_val.eq(read(m)) m.d.sync += current_result.eq(read_val) if read_only: @@ -283,7 +286,8 @@ def elaborate(self, platform): m.d.comb += write_val.eq(read_val | instr.s1_val) with m.Case(Funct3.CSRRC, Funct3.CSRRCI): m.d.comb += write_val.eq(read_val & (~instr.s1_val)) - write(m, write_val) + with m.If(exe_side_fx): + write(m, write_val) with m.Else(): # Missing privilege @@ -338,10 +342,12 @@ def _(): def _(): return {"from_pc": instr.pc, "next_pc": instr.pc + self.gen_params.isa.ilen_bytes} - # Generate rob_sfx_empty signal from precommit + # Generate precommitting signal from precommit @def_method(m, self.precommit) - def _(rob_id): - m.d.comb += rob_sfx_empty.eq(instr.rob_id == rob_id) + def _(rob_id: Value, side_fx: Value): + with m.If(instr.rob_id == rob_id): + m.d.comb += precommitting.eq(1) + m.d.comb += exe_side_fx.eq(side_fx) return m diff --git a/coreblocks/structs_common/rat.py b/coreblocks/structs_common/rat.py index 4902c1b99..1ed3f1a8f 100644 --- a/coreblocks/structs_common/rat.py +++ b/coreblocks/structs_common/rat.py @@ -42,8 +42,9 @@ def elaborate(self, platform): m = TModule() @def_method(m, self.commit) - def _(rp_dst: Value, rl_dst: Value): - m.d.sync += self.entries[rl_dst].eq(rp_dst) + def _(rp_dst: Value, rl_dst: Value, side_fx: Value): + with m.If(side_fx): + m.d.sync += self.entries[rl_dst].eq(rp_dst) return {"old_rp_dst": self.entries[rl_dst]} return m diff --git a/test/lsu/test_dummylsu.py b/test/lsu/test_dummylsu.py index d269049a3..d51edddc1 100644 --- a/test/lsu/test_dummylsu.py +++ b/test/lsu/test_dummylsu.py @@ -430,7 +430,7 @@ def precommiter(self): while len(self.precommit_data) == 0: yield rob_id = self.precommit_data[-1] # precommit is called continously until instruction is retired - yield from self.test_module.precommit.call(rob_id=rob_id) + yield from self.test_module.precommit.call(rob_id=rob_id, side_fx=1) def test(self): @def_method_mock(lambda: self.test_module.exception_report) diff --git a/test/stages/test_retirement.py b/test/stages/test_retirement.py index 05f4cd411..673279e06 100644 --- a/test/stages/test_retirement.py +++ b/test/stages/test_retirement.py @@ -123,7 +123,7 @@ def rf_free_process(reg_id): self.assertEqual(reg_id, self.rf_free_q.popleft()) @def_method_mock(lambda: retc.mock_precommit, sched_prio=2) - def precommit_process(rob_id): + def precommit_process(rob_id, side_fx): self.assertEqual(rob_id, self.precommit_q.popleft()) @def_method_mock(lambda: retc.mock_exception_cause) diff --git a/test/structs_common/test_csr.py b/test/structs_common/test_csr.py index d1b1e64e4..493617a39 100644 --- a/test/structs_common/test_csr.py +++ b/test/structs_common/test_csr.py @@ -183,7 +183,7 @@ def process_exception_test(self): ) yield from self.random_wait() - yield from self.dut.precommit.call(rob_id=rob_id) + yield from self.dut.precommit.call(rob_id=rob_id, side_fx=1) yield from self.random_wait() res = yield from self.dut.accept.call() diff --git a/test/test_core.py b/test/test_core.py index aa03c64e3..28f3a9196 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -243,6 +243,8 @@ def test_randomized(self): ("fibonacci", "fibonacci.asm", 1200, {2: 2971215073}, basic_core_config), ("fibonacci_mem", "fibonacci_mem.asm", 610, {3: 55}, basic_core_config), ("csr", "csr.asm", 200, {1: 1, 2: 4}, full_core_config), + ("exception", "exception.asm", 200, {1: 1, 2: 2}, basic_core_config), + ("exception_mem", "exception_mem.asm", 200, {1: 1, 2: 2}, basic_core_config), ], ) class TestCoreAsmSource(TestCoreBase):