Precommit using validate_arguments (#677)

coreblocks/backend/retirement.py

@@ -55,11 +55,16 @@ def __init__(
             max_latency=2 * 2**gen_params.rob_entries_bits,
         )
 
+        layouts = self.gen_params.get(RetirementLayouts)
         self.dependency_manager = DependencyContext.get()
         self.core_state = Method(o=self.gen_params.get(RetirementLayouts).core_state, nonexclusive=True)
         self.dependency_manager.add_dependency(CoreStateKey(), self.core_state)
 
-        self.precommit = Method(o=self.gen_params.get(RetirementLayouts).precommit, nonexclusive=True)
+        # The argument is only used in argument validation, it is not needed in the method body.
+        # A dummy combiner is provided.
+        self.precommit = Method(
+            i=layouts.precommit_in, o=layouts.precommit_out, nonexclusive=True, combiner=lambda m, args, runs: 0
+        )
         self.dependency_manager.add_dependency(InstructionPrecommitKey(), self.precommit)
 
     def elaborate(self, platform):
@@ -228,9 +233,11 @@ def flush_instr(rob_entry):
         def _():
             return {"flushing": core_flushing}
 
-        @def_method(m, self.precommit)
-        def _():
-            rob_entry = self.rob_peek(m)
-            return {"rob_id": rob_entry.rob_id, "side_fx": side_fx}
+        rob_id_val = Signal(self.gen_params.rob_entries_bits)
+
+        @def_method(m, self.precommit, validate_arguments=lambda rob_id: rob_id == rob_id_val)
+        def _(rob_id):
+            m.d.top_comb += rob_id_val.eq(self.rob_peek(m).rob_id)
+            return {"side_fx": side_fx}
 
         return m

coreblocks/func_blocks/csr/csr.py

@@ -101,13 +101,12 @@ def elaborate(self, platform):
         done = Signal()
         call_resume = Signal()
         exception = Signal()
-        precommitting = Signal()
 
         current_result = Signal(self.gen_params.isa.xlen)
 
         instr = Signal(StructLayout(self.csr_layouts.rs.data_layout.members | {"valid": 1}))
 
-        m.d.comb += ready_to_process.eq(precommitting & instr.valid & (instr.rp_s1 == 0))
+        m.d.comb += ready_to_process.eq(instr.valid & (instr.rp_s1 == 0))
 
         # RISCV Zicsr spec Table 1.1
         should_read_csr = Signal()
@@ -134,6 +133,9 @@ def elaborate(self, platform):
 
         # 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)):
+            precommit = self.dependency_manager.get_dependency(InstructionPrecommitKey())
+            info = precommit(m, instr.rob_id)
+            m.d.top_comb += exe_side_fx.eq(info.side_fx)
             with m.Switch(instr.csr):
                 for csr_number, methods in self.regfile.items():
                     read, write = methods
@@ -257,14 +259,6 @@ def _():
             # CSR instructions are never compressed, PC+4 is always next instruction
             return {"pc": instr.pc + self.gen_params.isa.ilen_bytes}
 
-        # Generate precommitting signal from precommit
-        with Transaction().body(m):
-            precommit = self.dependency_manager.get_dependency(InstructionPrecommitKey())
-            info = precommit(m)
-            with m.If(instr.rob_id == info.rob_id):
-                m.d.comb += precommitting.eq(1)
-                m.d.comb += exe_side_fx.eq(info.side_fx)
-
         return m
 
 

coreblocks/func_blocks/fu/lsu/dummyLsu.py

@@ -15,7 +15,7 @@
 from coreblocks.func_blocks.interface.func_protocols import FuncUnit
 from coreblocks.func_blocks.fu.lsu.pma import PMAChecker
 from coreblocks.func_blocks.fu.lsu.lsu_requester import LSURequester
-from coreblocks.interface.keys import ExceptionReportKey, CommonBusDataKey, InstructionPrecommitKey
+from coreblocks.interface.keys import CoreStateKey, ExceptionReportKey, CommonBusDataKey, InstructionPrecommitKey
 
 __all__ = ["LSUDummy", "LSUComponent"]
 
@@ -55,6 +55,11 @@ def elaborate(self, platform):
         m = TModule()
         flush = Signal()  # exception handling, requests are not issued
 
+        with Transaction().body(m):
+            core_state = self.dependency_manager.get_dependency(CoreStateKey())
+            state = core_state(m)
+            m.d.comb += flush.eq(state.flushing)
+
         # Signals for handling issue logic
         request_rob_id = Signal(self.gen_params.rob_entries_bits)
         rob_id_match = Signal()
@@ -142,11 +147,8 @@ def _():
 
         with Transaction().body(m):
             precommit = self.dependency_manager.get_dependency(InstructionPrecommitKey())
-            info = precommit(m)
-            with m.If(info.rob_id == request_rob_id):
-                m.d.comb += rob_id_match.eq(1)
-            with m.If(~info.side_fx):
-                m.d.comb += flush.eq(1)
+            precommit(m, request_rob_id)
+            m.d.comb += rob_id_match.eq(1)
 
         return m
 

coreblocks/func_blocks/fu/priv.py

@@ -101,32 +101,31 @@ def _(arg):
 
         with Transaction().body(m, request=instr_valid & ~finished):
             precommit = self.dm.get_dependency(InstructionPrecommitKey())
-            info = precommit(m)
-            with m.If(info.rob_id == instr_rob):
-                m.d.sync += finished.eq(1)
-                self.perf_instr.incr(m, instr_fn, cond=info.side_fx)
-
-                priv_data = priv_mode.read(m).data
-
-                illegal_mret = (instr_fn == PrivilegedFn.Fn.MRET) & (priv_data != PrivilegeLevel.MACHINE)
-                # future todo: WFI should be illegal in U-Mode only if S-Mode is supported
-                illegal_wfi = (
-                    (instr_fn == PrivilegedFn.Fn.WFI)
-                    & (priv_data == PrivilegeLevel.USER)
-                    & csr.m_mode.mstatus_tw.read(m).data
-                )
-
-                with condition(m, nonblocking=True) as branch:
-                    with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.MRET) & ~illegal_mret):
-                        mret(m)
-                    with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.FENCEI)):
-                        flush_icache(m)
-                    with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.WFI) & ~illegal_wfi):
-                        # async_interrupt_active implies wfi_resume. WFI should continue normal execution
-                        # when interrupt is enabled in xie, but disabled via global mstatus.xIE
-                        m.d.sync += finished.eq(wfi_resume)
-
-                m.d.sync += illegal_instruction.eq(illegal_wfi | illegal_mret)
+            info = precommit(m, instr_rob)
+            m.d.sync += finished.eq(1)
+            self.perf_instr.incr(m, instr_fn, cond=info.side_fx)
+
+            priv_data = priv_mode.read(m).data
+
+            illegal_mret = (instr_fn == PrivilegedFn.Fn.MRET) & (priv_data != PrivilegeLevel.MACHINE)
+            # future todo: WFI should be illegal in U-Mode only if S-Mode is supported
+            illegal_wfi = (
+                (instr_fn == PrivilegedFn.Fn.WFI)
+                & (priv_data == PrivilegeLevel.USER)
+                & csr.m_mode.mstatus_tw.read(m).data
+            )
+
+            with condition(m, nonblocking=True) as branch:
+                with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.MRET) & ~illegal_mret):
+                    mret(m)
+                with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.FENCEI)):
+                    flush_icache(m)
+                with branch(info.side_fx & (instr_fn == PrivilegedFn.Fn.WFI) & ~illegal_wfi):
+                    # async_interrupt_active implies wfi_resume. WFI should continue normal execution
+                    # when interrupt is enabled in xie, but disabled via global mstatus.xIE
+                    m.d.sync += finished.eq(wfi_resume)
+
+            m.d.sync += illegal_instruction.eq(illegal_wfi | illegal_mret)
 
         @def_method(m, self.accept, ready=instr_valid & finished)
         def _():

coreblocks/interface/layouts.py

@@ -355,7 +355,9 @@ class RetirementLayouts:
     def __init__(self, gen_params: GenParams):
         fields = gen_params.get(CommonLayoutFields)
 
-        self.precommit = make_layout(fields.rob_id, fields.side_fx)
+        self.precommit_in = make_layout(fields.rob_id)
+
+        self.precommit_out = make_layout(fields.side_fx)
 
         self.flushing = ("flushing", 1)
         """ Core is currently flushed """
@@ -581,10 +583,6 @@ class LSULayouts:
     def __init__(self, gen_params: GenParams):
         fields = gen_params.get(CommonLayoutFields)
 
-        retirement = gen_params.get(RetirementLayouts)
-
-        self.precommit = retirement.precommit
-
         self.store: LayoutListField = ("store", 1)
 
         self.issue = make_layout(fields.addr, fields.data, fields.funct3, self.store)
@@ -638,10 +636,6 @@ def __init__(self, gen_params: GenParams):
 
         self.rs = gen_params.get(RSInterfaceLayouts, rs_entries_bits=self.rs_entries_bits, data_layout=data_layout)
 
-        retirement = gen_params.get(RetirementLayouts)
-
-        self.precommit = retirement.precommit
-
 
 class ExceptionRegisterLayouts:
     """Layouts used in the exception information register."""

test/backend/test_retirement.py

@@ -147,13 +147,10 @@ def rat_process(self):
         assert not self.rf_free_q
 
     def precommit_process(self):
-        yield from self.retc.precommit_adapter.call_init()
         while self.precommit_q:
-            yield Tick()
-            info = yield from self.retc.precommit_adapter.call_result()
+            info = yield from self.retc.precommit_adapter.call_try(rob_id=self.precommit_q[0])
             assert info is not None
             assert info["side_fx"]
-            assert self.precommit_q[0] == info["rob_id"]
             self.precommit_q.popleft()
 
     @def_method_mock(lambda self: self.retc.mock_rf_free, sched_prio=2)

test/func_blocks/csr/test_csr.py

@@ -23,7 +23,7 @@
 
 
 class CSRUnitTestCircuit(Elaboratable):
-    def __init__(self, gen_params, csr_count, only_legal=True):
+    def __init__(self, gen_params: GenParams, csr_count: int, only_legal=True):
         self.gen_params = gen_params
         self.csr_count = csr_count
         self.only_legal = only_legal
@@ -32,7 +32,12 @@ def elaborate(self, platform):
         m = Module()
 
         m.submodules.precommit = self.precommit = TestbenchIO(
-            Adapter(o=self.gen_params.get(RetirementLayouts).precommit, nonexclusive=True)
+            Adapter(
+                i=self.gen_params.get(RetirementLayouts).precommit_in,
+                o=self.gen_params.get(RetirementLayouts).precommit_out,
+                nonexclusive=True,
+                combiner=lambda m, args, runs: args[0],
+            ).set(with_validate_arguments=True)
         )
         DependencyContext.get().add_dependency(InstructionPrecommitKey(), self.precommit.adapter.iface)
 
@@ -142,7 +147,9 @@ def process_test(self):
                 yield from self.dut.update.call(reg_id=op["exp"]["rs1"]["rp_s1"], reg_val=op["exp"]["rs1"]["value"])
 
             yield from self.random_wait_geom()
-            yield from self.dut.precommit.call(side_fx=1)
+            yield from self.dut.precommit.method_handle(
+                function=lambda rob_id: {"side_fx": 1}, validate_arguments=lambda rob_id: True
+            )
 
             yield from self.random_wait_geom()
             res = yield from self.dut.accept.call()
@@ -185,7 +192,7 @@ def process_exception_test(self):
 
             yield from self.dut.select.call()
 
-            rob_id = random.randrange(2**self.gen_params.rob_entries_bits)
+            csr_rob_id = random.randrange(2**self.gen_params.rob_entries_bits)
             yield from self.dut.insert.call(
                 rs_data={
                     "exec_fn": {"op_type": OpType.CSR_REG, "funct3": Funct3.CSRRW, "funct7": 0},
@@ -195,21 +202,24 @@ def process_exception_test(self):
                     "rp_dst": 2,
                     "imm": 0,
                     "csr": csr,
-                    "rob_id": rob_id,
+                    "rob_id": csr_rob_id,
                 }
             )
 
             yield from self.random_wait_geom()
-            yield from self.dut.precommit.call(rob_id=rob_id, side_fx=1)
+            yield from self.dut.precommit.method_handle(
+                function=lambda rob_id: {"side_fx": 1}, validate_arguments=lambda rob_id: rob_id == csr_rob_id
+            )
 
             yield from self.random_wait_geom()
             res = yield from self.dut.accept.call()
 
             assert res["exception"] == 1
             report = yield from self.dut.exception_report.call_result()
+            assert report is not None
             assert isinstance(report, dict)
             report.pop("mtval")  # mtval tested in mtval.asm test
-            assert {"rob_id": rob_id, "cause": ExceptionCause.ILLEGAL_INSTRUCTION, "pc": 0} == report
+            assert {"rob_id": csr_rob_id, "cause": ExceptionCause.ILLEGAL_INSTRUCTION, "pc": 0} == report
 
     def test_exception(self):
         self.gen_params = GenParams(test_core_config)

test/func_blocks/lsu/test_dummylsu.py

@@ -9,7 +9,7 @@
 from coreblocks.func_blocks.fu.lsu.dummyLsu import LSUDummy
 from coreblocks.params.configurations import test_core_config
 from coreblocks.arch import *
-from coreblocks.interface.keys import ExceptionReportKey, InstructionPrecommitKey
+from coreblocks.interface.keys import CoreStateKey, ExceptionReportKey, InstructionPrecommitKey
 from transactron.utils.dependencies import DependencyContext
 from coreblocks.interface.layouts import ExceptionRegisterLayouts, RetirementLayouts
 from coreblocks.peripherals.wishbone import *
@@ -84,11 +84,20 @@ def elaborate(self, platform):
 
         DependencyContext.get().add_dependency(ExceptionReportKey(), self.exception_report.adapter.iface)
 
+        layouts = self.gen.get(RetirementLayouts)
         m.submodules.precommit = self.precommit = TestbenchIO(
-            Adapter(o=self.gen.get(RetirementLayouts).precommit, nonexclusive=True)
+            Adapter(
+                i=layouts.precommit_in,
+                o=layouts.precommit_out,
+                nonexclusive=True,
+                combiner=lambda m, args, runs: args[0],
+            ).set(with_validate_arguments=True)
         )
         DependencyContext.get().add_dependency(InstructionPrecommitKey(), self.precommit.adapter.iface)
 
+        m.submodules.core_state = self.core_state = TestbenchIO(Adapter(o=layouts.core_state, nonexclusive=True))
+        DependencyContext.get().add_dependency(CoreStateKey(), self.core_state.adapter.iface)
+
         m.submodules.func_unit = func_unit = LSUDummy(self.gen, self.bus_master_adapter)
 
         m.submodules.issue_mock = self.issue = TestbenchIO(AdapterTrans(func_unit.issue))
@@ -248,6 +257,14 @@ def test(self):
         def exception_consumer(arg):
             assert arg == self.exception_queue.pop()
 
+        @def_method_mock(lambda: self.test_module.precommit, validate_arguments=lambda rob_id: True)
+        def precommiter(rob_id):
+            return {"side_fx": 1}
+
+        @def_method_mock(lambda: self.test_module.core_state)
+        def core_state_process():
+            return {"flushing": 0}
+
         with self.run_simulation(self.test_module) as sim:
             sim.add_process(self.wishbone_slave)
             sim.add_process(self.inserter)
@@ -305,6 +322,10 @@ def test(self):
         def exception_consumer(arg):
             assert False
 
+        @def_method_mock(lambda: self.test_module.precommit, validate_arguments=lambda rob_id: True)
+        def precommiter(rob_id):
+            return {"side_fx": 1}
+
         with self.run_simulation(self.test_module) as sim:
             sim.add_process(self.one_instr_test)
 
@@ -397,13 +418,12 @@ def get_resulter(self):
             yield from self.random_wait(self.max_wait)
             self.precommit_data.pop()  # retire
 
-    def precommiter(self):
-        yield Passive()
-        while True:
-            while len(self.precommit_data) == 0:
-                yield Tick()
-            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, side_fx=1)
+    def precommit_validate(self, rob_id):
+        return len(self.precommit_data) > 0 and rob_id == self.precommit_data[-1]
+
+    @def_method_mock(lambda self: self.test_module.precommit, validate_arguments=precommit_validate)
+    def precommiter(self, rob_id):
+        return {"side_fx": 1}
 
     def test(self):
         @def_method_mock(lambda: self.test_module.exception_report)
@@ -448,5 +468,9 @@ def test_fence(self):
         def exception_consumer(arg):
             assert False
 
+        @def_method_mock(lambda: self.test_module.precommit, validate_arguments=lambda rob_id: True)
+        def precommiter(rob_id):
+            return {"side_fx": 1}
+
         with self.run_simulation(self.test_module) as sim:
             sim.add_process(self.process)