retirement: Disable side effects on exception

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

coreblocks/lsu/dummyLsu.py

@@ -219,7 +219,10 @@ 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 +259,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)

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),
         ]
 
 

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):
+            with m.If(rob_entry.exception & side_fx):
+                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,24 @@ 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)
 

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
 

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

test/asm/exception.asm

@@ -0,0 +1,5 @@
+li x2, 2
+li x1, 1
+.4byte 0  /* should be unimp, but it would test nothing since unimp is system and stalls the fetcher >:( */
+li x2, 9
+

test/asm/exception_mem.asm

@@ -0,0 +1,11 @@
+# Data adress space:
+# 0x0 - one
+# 0x4 - two
+li x1, 1
+sw x1, 0(x0)
+li x2, 2
+sw x2, 4(x0)
+.4byte 0  /* should be unimp, but it would test nothing since unimp is system and stalls the fetcher >:( */
+sw x1, 4(x0)  /* TODO: actually check the side fx */
+li x2, 9
+

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)

test/stages/test_retirement.py

@@ -3,7 +3,7 @@
 from coreblocks.structs_common.csr_generic import GenericCSRRegisters
 
 from transactron.lib import FIFO, Adapter
-from coreblocks.structs_common.rat import RRAT
+from coreblocks.structs_common.rat import FRAT, RRAT
 from coreblocks.params import ROBLayouts, RFLayouts, GenParams, LSULayouts, SchedulerLayouts
 from coreblocks.params.configurations import test_core_config
 
@@ -26,6 +26,7 @@ def elaborate(self, platform):
         exception_layouts = self.gen_params.get(ExceptionRegisterLayouts)
 
         m.submodules.r_rat = self.rat = RRAT(gen_params=self.gen_params)
+        m.submodules.f_rat = self.frat = FRAT(gen_params=self.gen_params)
         m.submodules.free_rf_list = self.free_rf = FIFO(
             scheduler_layouts.free_rf_layout, 2**self.gen_params.phys_regs_bits
         )
@@ -51,6 +52,7 @@ def elaborate(self, platform):
             rf_free=self.mock_rf_free.adapter.iface,
             precommit=self.mock_precommit.adapter.iface,
             exception_cause_get=self.mock_exception_cause.adapter.iface,
+            frat_rename=self.frat.rename,
         )
 
         m.submodules.free_rf_fifo_adapter = self.free_rf_adapter = TestbenchIO(AdapterTrans(self.free_rf.read))
@@ -123,7 +125,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)

test/structs_common/test_csr.py

@@ -130,7 +130,7 @@ def process_test(self):
                 yield from self.dut.update.call(tag=op["exp"]["rs1"]["rp_s1"], value=op["exp"]["rs1"]["value"])
 
             yield from self.random_wait()
-            yield from self.dut.precommit.call()
+            yield from self.dut.precommit.call(side_fx=1)
 
             yield from self.random_wait()
             res = yield from self.dut.accept.call()
@@ -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()

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):