From 71008605e1df07b98f42118e904ab37cb637953e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 23 Oct 2023 20:19:25 +0200 Subject: [PATCH 1/6] Autumn cleaning: types (#479) --- coreblocks/fu/div_unit.py | 4 +-- coreblocks/fu/mul_unit.py | 6 ++-- coreblocks/scheduler/scheduler.py | 2 +- coreblocks/stages/func_blocks_unifier.py | 2 +- coreblocks/structs_common/rs.py | 3 +- coreblocks/utils/_typing.py | 38 ++++++++++++------------ coreblocks/utils/utils.py | 3 +- transactron/_utils.py | 3 +- transactron/lib/transformers.py | 9 +++--- 9 files changed, 37 insertions(+), 33 deletions(-) diff --git a/coreblocks/fu/div_unit.py b/coreblocks/fu/div_unit.py index f78feadf8..c874d439f 100644 --- a/coreblocks/fu/div_unit.py +++ b/coreblocks/fu/div_unit.py @@ -1,6 +1,6 @@ from dataclasses import KW_ONLY, dataclass from enum import IntFlag, auto -from typing import Sequence, Tuple +from collections.abc import Sequence from amaranth import * @@ -34,7 +34,7 @@ def get_instructions(self) -> Sequence[tuple]: ] -def get_input(arg: Record) -> Tuple[Value, Value]: +def get_input(arg: Record) -> tuple[Value, Value]: return arg.s1_val, Mux(arg.imm, arg.imm, arg.s2_val) diff --git a/coreblocks/fu/mul_unit.py b/coreblocks/fu/mul_unit.py index a7230c774..3d4888cfa 100644 --- a/coreblocks/fu/mul_unit.py +++ b/coreblocks/fu/mul_unit.py @@ -1,5 +1,5 @@ from enum import IntFlag, IntEnum, auto -from typing import Sequence, Tuple +from collections.abc import Sequence from dataclasses import KW_ONLY, dataclass from amaranth import * @@ -45,7 +45,7 @@ def get_instructions(self) -> Sequence[tuple]: ] -def get_input(arg: Record) -> Tuple[Value, Value]: +def get_input(arg: Record) -> tuple[Value, Value]: """ Operation of getting two input values. @@ -56,7 +56,7 @@ def get_input(arg: Record) -> Tuple[Value, Value]: Returns ------- - return : Tuple[Value, Value] + return : tuple[Value, Value] Two input values. """ return arg.s1_val, Mux(arg.imm, arg.imm, arg.s2_val) diff --git a/coreblocks/scheduler/scheduler.py b/coreblocks/scheduler/scheduler.py index b85e6d052..eab680fe2 100644 --- a/coreblocks/scheduler/scheduler.py +++ b/coreblocks/scheduler/scheduler.py @@ -1,4 +1,4 @@ -from typing import Sequence +from collections.abc import Sequence from amaranth import * diff --git a/coreblocks/stages/func_blocks_unifier.py b/coreblocks/stages/func_blocks_unifier.py index 8bd97c423..8748bf5df 100644 --- a/coreblocks/stages/func_blocks_unifier.py +++ b/coreblocks/stages/func_blocks_unifier.py @@ -1,4 +1,4 @@ -from typing import Iterable +from collections.abc import Iterable from amaranth import * diff --git a/coreblocks/structs_common/rs.py b/coreblocks/structs_common/rs.py index 60bcdcce3..eb045c3a4 100644 --- a/coreblocks/structs_common/rs.py +++ b/coreblocks/structs_common/rs.py @@ -1,4 +1,5 @@ -from typing import Iterable, Optional +from collections.abc import Iterable +from typing import Optional from amaranth import * from amaranth.lib.coding import PriorityEncoder from transactron import Method, def_method, TModule diff --git a/coreblocks/utils/_typing.py b/coreblocks/utils/_typing.py index 124066124..5e27a229d 100644 --- a/coreblocks/utils/_typing.py +++ b/coreblocks/utils/_typing.py @@ -1,18 +1,14 @@ from typing import ( - ContextManager, Generic, NoReturn, Optional, Protocol, - Sequence, - Tuple, - Type, TypeAlias, - Iterable, - Mapping, TypeVar, runtime_checkable, ) +from collections.abc import Iterable, Mapping, Sequence +from contextlib import AbstractContextManager from enum import Enum from amaranth import * from amaranth.lib.data import View @@ -21,16 +17,18 @@ from amaranth.hdl.rec import Direction, Layout # Types representing Amaranth concepts -FragmentLike = Fragment | Elaboratable -ValueLike = Value | int | Enum | ValueCastable -ShapeLike = Shape | ShapeCastable | int | range | Type[Enum] +FragmentLike: TypeAlias = Fragment | Elaboratable +ValueLike: TypeAlias = Value | int | Enum | ValueCastable +ShapeLike: TypeAlias = Shape | ShapeCastable | int | range | type[Enum] StatementLike: TypeAlias = Statement | Iterable["StatementLike"] -LayoutLike = Layout | Sequence[Tuple[str, ShapeLike | "LayoutLike"] | Tuple[str, ShapeLike | "LayoutLike", Direction]] +LayoutLike: TypeAlias = ( + Layout | Sequence[tuple[str, "ShapeLike | LayoutLike"] | tuple[str, "ShapeLike | LayoutLike", Direction]] +) SwitchKey: TypeAlias = str | int | Enum # Internal Coreblocks types SignalBundle: TypeAlias = Signal | Record | View | Iterable["SignalBundle"] | Mapping[str, "SignalBundle"] -LayoutList = list[Tuple[str, ShapeLike | "LayoutList"]] +LayoutList: TypeAlias = list[tuple[str, "ShapeLike | LayoutList"]] class _ModuleBuilderDomainsLike(Protocol): @@ -55,28 +53,30 @@ class ModuleLike(Protocol, Generic[_T_ModuleBuilderDomains]): domains: _ModuleBuilderDomainSet d: _T_ModuleBuilderDomains - def If(self, cond: ValueLike) -> ContextManager[None]: # noqa: N802 + def If(self, cond: ValueLike) -> AbstractContextManager[None]: # noqa: N802 ... - def Elif(self, cond: ValueLike) -> ContextManager[None]: # noqa: N802 + def Elif(self, cond: ValueLike) -> AbstractContextManager[None]: # noqa: N802 ... - def Else(self) -> ContextManager[None]: # noqa: N802 + def Else(self) -> AbstractContextManager[None]: # noqa: N802 ... - def Switch(self, test: ValueLike) -> ContextManager[None]: # noqa: N802 + def Switch(self, test: ValueLike) -> AbstractContextManager[None]: # noqa: N802 ... - def Case(self, *patterns: SwitchKey) -> ContextManager[None]: # noqa: N802 + def Case(self, *patterns: SwitchKey) -> AbstractContextManager[None]: # noqa: N802 ... - def Default(self) -> ContextManager[None]: # noqa: N802 + def Default(self) -> AbstractContextManager[None]: # noqa: N802 ... - def FSM(self, reset: Optional[str] = ..., domain: str = ..., name: str = ...) -> ContextManager[FSM]: # noqa: N802 + def FSM( # noqa: N802 + self, reset: Optional[str] = ..., domain: str = ..., name: str = ... + ) -> AbstractContextManager[FSM]: ... - def State(self, name: str) -> ContextManager[None]: # noqa: N802 + def State(self, name: str) -> AbstractContextManager[None]: # noqa: N802 ... @property diff --git a/coreblocks/utils/utils.py b/coreblocks/utils/utils.py index 11a0df1b1..31fc830ab 100644 --- a/coreblocks/utils/utils.py +++ b/coreblocks/utils/utils.py @@ -1,6 +1,7 @@ from contextlib import contextmanager from enum import Enum -from typing import Iterable, Literal, Mapping, Optional, TypeAlias, cast, overload +from typing import Literal, Optional, TypeAlias, cast, overload +from collections.abc import Iterable, Mapping from amaranth import * from amaranth.hdl.ast import Assign, ArrayProxy from amaranth.lib import data diff --git a/transactron/_utils.py b/transactron/_utils.py index 0df18db2e..f86b0d647 100644 --- a/transactron/_utils.py +++ b/transactron/_utils.py @@ -1,7 +1,8 @@ import itertools import sys from inspect import Parameter, signature -from typing import Callable, Iterable, Optional, TypeAlias, TypeVar, Mapping +from typing import Optional, TypeAlias, TypeVar +from collections.abc import Callable, Iterable, Mapping from amaranth import * from coreblocks.utils._typing import LayoutLike from coreblocks.utils import OneHotSwitchDynamic diff --git a/transactron/lib/transformers.py b/transactron/lib/transformers.py index 23c385b53..e4b7aa0c0 100644 --- a/transactron/lib/transformers.py +++ b/transactron/lib/transformers.py @@ -1,7 +1,8 @@ from amaranth import * from ..core import * from ..core import RecordDict -from typing import Optional, Callable, Tuple +from typing import Optional +from collections.abc import Callable from coreblocks.utils import ValueLike, assign, AssignType from .connectors import Forwarder, ManyToOneConnectTrans, ConnectTrans @@ -35,8 +36,8 @@ def __init__( self, target: Method, *, - i_transform: Optional[Tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, - o_transform: Optional[Tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, + i_transform: Optional[tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, + o_transform: Optional[tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, ): """ Parameters @@ -132,7 +133,7 @@ class MethodProduct(Elaboratable): def __init__( self, targets: list[Method], - combiner: Optional[Tuple[MethodLayout, Callable[[TModule, list[Record]], RecordDict]]] = None, + combiner: Optional[tuple[MethodLayout, Callable[[TModule, list[Record]], RecordDict]]] = None, ): """Method product. From 122ac317b22136ba0a783fe1d4c1affbf1c541bd Mon Sep 17 00:00:00 2001 From: piotro888 Date: Mon, 23 Oct 2023 20:35:10 +0200 Subject: [PATCH 2/6] Fix memory protection in regression tests (#481) --- test/regression/cocotb.py | 2 +- test/regression/memory.py | 8 ++++---- test/regression/test.py | 8 +++++++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/test/regression/cocotb.py b/test/regression/cocotb.py index 9ca00c905..db7fe9aa8 100644 --- a/test/regression/cocotb.py +++ b/test/regression/cocotb.py @@ -147,7 +147,7 @@ async def run(self, mem_model: CoreMemoryModel, timeout_cycles: int = 5000) -> b instr_wb = WishboneSlave(self.dut, "wb_instr", self.dut.clk, mem_model, is_instr_bus=True) cocotb.start_soon(instr_wb.start()) - data_wb = WishboneSlave(self.dut, "wb_data", self.dut.clk, mem_model, is_instr_bus=True) + data_wb = WishboneSlave(self.dut, "wb_data", self.dut.clk, mem_model, is_instr_bus=False) cocotb.start_soon(data_wb.start()) res = await with_timeout(self.finish_event.wait(), timeout_cycles, "ns") diff --git a/test/regression/memory.py b/test/regression/memory.py index 1f0fa407f..68bda0616 100644 --- a/test/regression/memory.py +++ b/test/regression/memory.py @@ -137,7 +137,7 @@ def write(self, req: WriteRequest) -> WriteReply: return WriteReply(status=ReplyStatus.ERROR) -def load_segments_from_elf(file_path: str) -> list[RandomAccessMemory]: +def load_segments_from_elf(file_path: str, *, disable_write_protection: bool = False) -> list[RandomAccessMemory]: segments: list[RandomAccessMemory] = [] with open(file_path, "rb") as f: @@ -160,11 +160,11 @@ def align_down(n: int) -> int: data = b"\x00" * (paddr - seg_start) + segment.data() + b"\x00" * (seg_end - (paddr + len(segment.data()))) flags = SegmentFlags(0) - if flags_raw & P_FLAGS.PF_R == flags_raw & P_FLAGS.PF_R: + if flags_raw & P_FLAGS.PF_R: flags |= SegmentFlags.READ - if flags_raw & P_FLAGS.PF_W == flags_raw & P_FLAGS.PF_W: + if flags_raw & P_FLAGS.PF_W or disable_write_protection: flags |= SegmentFlags.WRITE - if flags_raw & P_FLAGS.PF_X == flags_raw & P_FLAGS.PF_X: + if flags_raw & P_FLAGS.PF_X: flags |= SegmentFlags.EXECUTABLE segments.append(RandomAccessMemory(range(seg_start, seg_end), flags, data)) diff --git a/test/regression/test.py b/test/regression/test.py index fa023f90f..cbe8067cd 100644 --- a/test/regression/test.py +++ b/test/regression/test.py @@ -7,6 +7,9 @@ test_dir = Path(__file__).parent.parent riscv_tests_dir = test_dir.joinpath("external/riscv-tests") +# disable write protection for specific tests with writes to .text section +exclude_write_protection = ["rv32uc-rvc"] + class MMIO(MemorySegment): def __init__(self, on_finish: Callable[[], None]): @@ -31,7 +34,10 @@ async def run_test(sim_backend: SimulationBackend, test_name: str): mmio = MMIO(lambda: sim_backend.stop()) mem_segments: list[MemorySegment] = [] - mem_segments += load_segments_from_elf(str(riscv_tests_dir.joinpath("test-" + test_name))) + mem_segments += load_segments_from_elf( + str(riscv_tests_dir.joinpath("test-" + test_name)), + disable_write_protection=test_name in exclude_write_protection, + ) mem_segments.append(mmio) mem_model = CoreMemoryModel(mem_segments) From 3da0f5c74bcb7c0b713f7b53bced690ef519270e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 24 Oct 2023 19:44:18 +0200 Subject: [PATCH 3/6] Bump Pyright (#485) --- constants/ecp5_platforms.py | 2 +- requirements-dev.txt | 2 +- test/regression/cocotb.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/constants/ecp5_platforms.py b/constants/ecp5_platforms.py index 8bca17145..0e3545690 100644 --- a/constants/ecp5_platforms.py +++ b/constants/ecp5_platforms.py @@ -55,7 +55,7 @@ def __init__(self, pins: Iterable[str]): def p(self, count: int = 1): return " ".join([self.pin_bag.pop() for _ in range(count)]) - def named_pin(self, names: list[str]): + def named_pin(self, names: Iterable[str]): for name in names: if name in self.pin_bag: self.pin_bag.remove(name) diff --git a/requirements-dev.txt b/requirements-dev.txt index 766fd7c8e..35c93666f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -9,7 +9,7 @@ myst-parser==0.18.0 numpydoc==1.5.0 parameterized==0.8.1 pre-commit==2.16.0 -pyright==1.1.308 +pyright==1.1.332 Sphinx==5.1.1 sphinx-rtd-theme==1.0.0 sphinxcontrib-mermaid==0.8.1 diff --git a/test/regression/cocotb.py b/test/regression/cocotb.py index db7fe9aa8..e59bcef03 100644 --- a/test/regression/cocotb.py +++ b/test/regression/cocotb.py @@ -73,7 +73,7 @@ async def start(self): while True: while not (self.bus.stb.value and self.bus.cyc.value): - await clock_edge_event + await clock_edge_event # type: ignore sig_m = WishboneMasterSignals() self.bus.sample(sig_m) @@ -124,10 +124,10 @@ async def start(self): ) for _ in range(self.delay): - await clock_edge_event + await clock_edge_event # type: ignore self.bus.drive(sig_s) - await clock_edge_event + await clock_edge_event # type: ignore self.bus.drive(WishboneSlaveSignals()) From a6282560786cc5952960addb168aa2345675973d Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 24 Oct 2023 20:04:44 +0200 Subject: [PATCH 4/6] Refactor def_helper (#486) --- test/common.py | 4 ++-- transactron/_utils.py | 35 ++++++++++++++++++++++++----------- transactron/core.py | 2 +- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/test/common.py b/test/common.py index f8cc2f4be..76ede88f4 100644 --- a/test/common.py +++ b/test/common.py @@ -12,7 +12,7 @@ from transactron.core import SignalBundle, Method, TransactionModule from transactron.lib import AdapterBase, AdapterTrans -from transactron._utils import method_def_helper +from transactron._utils import mock_def_helper from coreblocks.utils import ValueLike, HasElaborate, HasDebugSignals, auto_debug_signals, LayoutLike, ModuleConnector from .gtkw_extension import write_vcd_ext @@ -363,7 +363,7 @@ def method_handle( for _ in range(extra_settle_count + 1): yield Settle() - ret_out = method_def_helper(self, function, **arg) + ret_out = mock_def_helper(self, function, arg) yield from self.method_return(ret_out or {}) yield diff --git a/transactron/_utils.py b/transactron/_utils.py index f86b0d647..138c7222b 100644 --- a/transactron/_utils.py +++ b/transactron/_utils.py @@ -1,7 +1,7 @@ import itertools import sys from inspect import Parameter, signature -from typing import Optional, TypeAlias, TypeVar +from typing import Any, Concatenate, Optional, TypeAlias, TypeGuard, TypeVar from collections.abc import Callable, Iterable, Mapping from amaranth import * from coreblocks.utils._typing import LayoutLike @@ -15,11 +15,13 @@ "Graph", "GraphCC", "get_caller_class_name", + "def_helper", "method_def_helper", ] T = TypeVar("T") +U = TypeVar("U") class Scheduler(Elaboratable): @@ -122,24 +124,35 @@ def _graph_ccs(gr: ROGraph[T]) -> list[GraphCC[T]]: MethodLayout: TypeAlias = LayoutLike -def method_def_helper(method, func: Callable[..., T], arg=None, /, **kwargs) -> T: +def has_first_param(func: Callable[..., T], name: str, tp: type[U]) -> TypeGuard[Callable[Concatenate[U, ...], T]]: + parameters = signature(func).parameters + return ( + len(parameters) >= 1 + and next(iter(parameters)) == name + and parameters[name].kind in {Parameter.POSITIONAL_OR_KEYWORD, Parameter.POSITIONAL_ONLY} + and parameters[name].annotation in {Parameter.empty, tp} + ) + + +def def_helper(description, func: Callable[..., T], tp: type[U], arg: U, /, **kwargs) -> T: parameters = signature(func).parameters kw_parameters = set( n for n, p in parameters.items() if p.kind in {Parameter.POSITIONAL_OR_KEYWORD, Parameter.KEYWORD_ONLY} ) - if ( - len(parameters) == 1 - and "arg" in parameters - and parameters["arg"].kind in {Parameter.POSITIONAL_OR_KEYWORD, Parameter.POSITIONAL_ONLY} - and parameters["arg"].annotation in {Parameter.empty, Record} - ): - if arg is None: - arg = kwargs + if len(parameters) == 1 and has_first_param(func, "arg", tp): return func(arg) elif kw_parameters <= kwargs.keys(): return func(**kwargs) else: - raise TypeError(f"Invalid method definition/mock for {method}: {func}") + raise TypeError(f"Invalid {description}: {func}") + + +def mock_def_helper(tb, func: Callable[..., T], arg: Mapping[str, Any]) -> T: + return def_helper(f"mock definition for {tb}", func, Mapping[str, Any], arg, **arg) + + +def method_def_helper(method, func: Callable[..., T], arg: Record) -> T: + return def_helper(f"method definition for {method}", func, Record, arg, **arg.fields) def get_caller_class_name(default: Optional[str] = None) -> tuple[Optional[Elaboratable], str]: diff --git a/transactron/core.py b/transactron/core.py index 745b98b4d..035b3b588 100644 --- a/transactron/core.py +++ b/transactron/core.py @@ -1223,7 +1223,7 @@ def decorator(func: Callable[..., Optional[RecordDict]]): ret_out = None with method.body(m, ready=ready, out=out) as arg: - ret_out = method_def_helper(method, func, arg, **arg.fields) + ret_out = method_def_helper(method, func, arg) if ret_out is not None: m.d.top_comb += assign(out, ret_out, fields=AssignType.ALL) From fc8c9b9cbcecdd99636230d6d7bf0e012542ba33 Mon Sep 17 00:00:00 2001 From: Arusekk Date: Fri, 27 Oct 2023 14:15:29 +0200 Subject: [PATCH 5/6] Fix tests with binutils 2.41 (#490) --- test/test_core.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index 8a443dbda..aa03c64e3 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -263,7 +263,7 @@ def test_asm_source(self): self.base_dir = "test/asm/" self.bin_src = [] - with tempfile.NamedTemporaryFile() as asm_tmp: + with tempfile.NamedTemporaryFile() as asm_tmp, tempfile.NamedTemporaryFile() as bin_tmp: subprocess.check_call( [ "riscv64-unknown-elf-as", @@ -276,9 +276,10 @@ def test_asm_source(self): self.base_dir + self.source_file, ] ) - code = subprocess.check_output( - ["riscv64-unknown-elf-objcopy", "-O", "binary", "-j", ".text", asm_tmp.name, "/dev/stdout"] + subprocess.check_call( + ["riscv64-unknown-elf-objcopy", "-O", "binary", "-j", ".text", asm_tmp.name, bin_tmp.name] ) + code = bin_tmp.read() for word_idx in range(0, len(code), 4): word = code[word_idx : word_idx + 4] bin_instr = int.from_bytes(word, "little") From 99f3749b855d6a773932003213114fa5e07a0030 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Sun, 29 Oct 2023 18:15:57 +0100 Subject: [PATCH 6/6] Extend Transactron introduction (#491) --- docs/Transactions.md | 181 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 174 insertions(+), 7 deletions(-) diff --git a/docs/Transactions.md b/docs/Transactions.md index 1a7169774..41b5d5528 100644 --- a/docs/Transactions.md +++ b/docs/Transactions.md @@ -54,12 +54,6 @@ The transaction body `with` block works analogously to Amaranth's `with m.If():` This is implemented in hardware via multiplexers. Please remember that this is not a Python `if` statement -- the *Python code* inside the `with` block is always executed once. -If a transaction is not always ready for execution (for example, because of the dependence on some resource), a `request` parameter should be used. An Amaranth single-bit expression should be passed. - -```python - with Transaction().body(m, request=expr): -``` - ### Implementing methods As methods are used as a way to communicate with other `Elaboratable`s, they are typically declared in the `Elaboratable`'s constructor, and then defined in the `elaborate` method: @@ -114,7 +108,7 @@ Only methods should be passed around, not entire `Elaboratable`s! ```python class MyThing(Elaboratable): - def __init__(self, method): + def __init__(self, method: Method): self.method = method ... @@ -139,6 +133,114 @@ If in doubt, methods are preferred. This is because if a functionality is implemented as a method, and a transaction is needed, one can use a transaction which calls this method and does nothing else. Such a transaction is included in the library -- it's named `AdapterTrans`. +### Method argument passing conventions + +Even though method arguments are Amaranth records, their use can be avoided in many cases, which results in cleaner code. +Suppose we have the following layout, which is an input layout for a method called `method`: + +```python +layout = [("foo", 1), ("bar", 32)] +method = Method(input_layout=layout) +``` + +The method can be called in multiple ways. +The cleanest and recommended way is to pass each record field using a keyword argument: + +```python +method(m, foo=foo_expr, bar=bar_expr) +``` + +Another way is to pass the arguments using a `dict`: + +```python +method(m, {'foo': foo_expr, 'bar': bar_expr}) +``` + +Finally, one can directly pass an Amaranth record: + +```python +rec = Record(layout) +m.d.comb += rec.foo.eq(foo_expr) +m.d.comb += rec.bar.eq(bar_expr) +method(m, rec) +``` + +The `dict` convention can be used recursively when layouts are nested. +Take the following definitions: + +```python +layout2 = [("foobar", layout), ("baz", 42)] +method2 = Method(input_layout=layout2) +``` + +One can then pass the arguments using `dict`s in following ways: + +```python +# the preferred way +method2(m, foobar={'foo': foo_expr, 'bar': bar_expr}, baz=baz_expr) + +# the alternative way +method2(m, {'foobar': {'foo': foo_expr, 'bar': bar_expr}, 'baz': baz_expr}) +``` + +### Method definition conventions + +When defining methods, two conventions can be used. +The cleanest and recommended way is to create an argument for each record field: + +```python +@def_method(m, method) +def _(foo: Value, bar: Value): + ... +``` + +The other is to receive the argument record directly. The `arg` name is required: + +```python +def_method(m, method) +def _(arg: Record): + ... +``` + +### Method return value conventions + +The `dict` syntax can be used for returning values from methods. +Take the following method declaration: + +```python +method3 = Method(input_layout=layout, output_layout=layout2) +``` + +One can then define this method as follows: + +```python +@def_method(m, method3) +def _(foo: Value, bar: Value): + return {{'foo': foo, 'bar': foo + bar}, 'baz': foo - bar} +``` + +### Readiness signals + +If a transaction is not always ready for execution (for example, because of the dependence on some resource), a `request` parameter should be used. +An Amaranth single-bit expression should be passed. +When the `request` parameter is not passed, the transaction is always requesting execution. + +```python + with Transaction().body(m, request=expr): +``` + +Methods have a similar mechanism, which uses the `ready` parameter on `def_method`: + +```python + @def_method(m, self.my_method, ready=expr) + def _(arg): + ... +``` + +The `request` signal typically should only depend on the internal state of an `Elaboratable`. +Other dependencies risk introducing combinational loops. +In certain occasions, it is possible to relax this requirement; see e.g. [Scheduling order](#scheduling-order). + ## The library The transaction framework is designed to facilitate code re-use. @@ -152,6 +254,71 @@ The most useful ones are: ## Advanced concepts +### Special combinational domains + +Transactron defines its own variant of Amaranth modules, called `TModule`. +Its role is to allow to improve circuit performance by omitting unneeded multiplexers in combinational circuits. +This is done by adding two additional, special combinatorial domains, `av_comb` and `top_comb`. + +Statements added to the `av_comb` domain (the "avoiding" domain) are not executed when under a false `m.If`, but are executed when under a false `m.AvoidedIf`. +Transaction and method bodies are internally guarded by an `m.AvoidedIf` with the transaction `grant` or method `run` signal. +Therefore combinational assignments added to `av_comb` work even if the transaction or method definition containing the assignments are not running. +Because combinational signals usually don't induce state changes, this is often safe to do and improves performance. + +Statements added to the `top_comb` domain are always executed, even if the statement is under false conditions (including `m.If`, `m.Switch` etc.). +This allows for cleaner code, as combinational assignments which logically belong to some case, but aren't actually required to be there, can be as performant as if they were manually moved to the top level. + +An important caveat of the special domains is that, just like with normal domains, a signal assigned in one of them cannot be assigned in others. + +### Scheduling order + +When writing multiple methods and transactions in the same `Elaboratable`, sometimes some dependency between them needs to exist. +For example, in the `Forwarder` module in the library, forwarding can take place only if both `read` and `write` are executed simultaneously. +This requirement is handled by making the the `read` method's readiness depend on the execution of the `write` method. +If the `read` method was considered for execution before `write`, this would introduce a combinational loop into the circuit. +In order to avoid such issues, one can require a certain scheduling order between methods and transactions. + +`Method` and `Transaction` objects include a `schedule_before` method. +Its only argument is another `Method` or `Transaction`, which will be scheduled after the first one: + +```python +first_t_or_m.schedule_before(other_t_or_m) +``` + +Internally, scheduling orders exist only on transactions. +If a scheduling order is added to a `Method`, it is lifted to the transaction level. +For example, if `first_m` is scheduled before `other_t`, and is called by `t1` and `t2`, the added scheduling orderings will be the same as if the following calls were made: + +```python +t1.schedule_before(other_t) +t2.schedule_before(other_t) +``` + +### Conflicts + +In some situations it might be useful to make some methods or transactions mutually exclusive with others. +Two conflicting transactions or methods can't execute simultaneously: only one or the other runs in a given clock cycle. + +Conflicts are defined similarly to scheduling orders: + +```python +first_t_or_m.add_conflict(other_t_or_m) +``` + +Conflicts are lifted to the transaction level, just like scheduling orders. + +The `add_conflict` method has an optional argument `priority`, which allows to define a scheduling order between conflicting transactions or methods. +Possible values are `Priority.LEFT`, `Priority.RIGHT` and `Priority.UNDEFINED` (the default). +For example, the following code adds a conflict with a scheduling order, where `first_m` is scheduled before `other_m`: + +```python +first_m.add_conflict(other_m, priority = Priority.LEFT) +``` + +Scheduling conflicts come with a possible cost. +The conflicting transactions have a dependency in the transaction scheduler, which can increase the size and combinational delay of the scheduling circuit. +Therefore, use of this feature requires consideration. + ### Transaction and method nesting Transaction and method bodies can be nested. For example: