From 6b200fd00e9ba0fc403d1373657ce24fcc2d741c Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Sat, 16 Dec 2023 15:33:12 +0100 Subject: [PATCH 01/35] Source locations for transactions and methods --- transactron/core.py | 25 +++++++++++++++++-------- transactron/utils/_typing.py | 2 ++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/transactron/core.py b/transactron/core.py index 6410ab773..59c015f2c 100644 --- a/transactron/core.py +++ b/transactron/core.py @@ -481,15 +481,15 @@ def print_info( print("") print("Calling transactions per method") for m, ts in method_map.transactions_by_method.items(): - print(f"\t{m.owned_name}") + print(f"\t{m.owned_name}: {m.src_loc[0]}:{m.src_loc[1]}") for t in ts: - print(f"\t\t{t.name}") + print(f"\t\t{t.name}: {t.src_loc[0]}:{t.src_loc[1]}") print("") print("Called methods per transaction") for t, ms in method_map.methods_by_transaction.items(): - print(f"\t{t.name}") + print(f"\t{t.name}: {t.src_loc[0]}:{t.src_loc[1]}") for m in ms: - print(f"\t\t{m.owned_name}") + print(f"\t\t{m.owned_name}: {m.src_loc[0]}:{m.src_loc[1]}") print("") def visual_graph(self, fragment): @@ -727,12 +727,14 @@ class TransactionBase(Owned, Protocol): def_order: int defined: bool = False name: str + src_loc: SrcLoc method_uses: dict["Method", Tuple[Record, ValueLike]] relations: list[RelationBase] simultaneous_list: list[TransactionOrMethod] independent_list: list[TransactionOrMethod] - def __init__(self): + def __init__(self, *, src_loc_at: int): + self.src_loc = tracer.get_src_loc(src_loc_at) self.method_uses: dict["Method", Tuple[Record, ValueLike]] = dict() self.relations: list[RelationBase] = [] self.simultaneous_list: list[TransactionOrMethod] = [] @@ -899,7 +901,9 @@ class Transaction(TransactionBase): and all used methods are called. """ - def __init__(self, *, name: Optional[str] = None, manager: Optional[TransactionManager] = None): + def __init__( + self, *, name: Optional[str] = None, manager: Optional[TransactionManager] = None, src_loc_at: int = 0 + ): """ Parameters ---------- @@ -911,8 +915,10 @@ def __init__(self, *, name: Optional[str] = None, manager: Optional[TransactionM manager: TransactionManager The `TransactionManager` controlling this `Transaction`. If omitted, the manager is received from `TransactionContext`. + src_loc_at: int + How many stack frames deep the source location is taken from. """ - super().__init__() + super().__init__(src_loc_at=1 + src_loc_at) self.owner, owner_name = get_caller_class_name(default="$transaction") self.name = name or tracer.get_var_name(depth=2, default=owner_name) if manager is None: @@ -1004,6 +1010,7 @@ def __init__( o: MethodLayout = (), nonexclusive: bool = False, single_caller: bool = False, + src_loc_at: int = 0, ): """ Parameters @@ -1026,8 +1033,10 @@ def __init__( If true, this method is intended to be called from a single transaction. An error will be thrown if called from multiple transactions. + src_loc_at: int + How many stack frames deep the source location is taken from. """ - super().__init__() + super().__init__(src_loc_at=1 + src_loc_at) self.owner, owner_name = get_caller_class_name(default="$method") self.name = name or tracer.get_var_name(depth=2, default=owner_name) self.ready = Signal(name=self.owned_name + "_ready") diff --git a/transactron/utils/_typing.py b/transactron/utils/_typing.py index e4a532697..fc7d9e59d 100644 --- a/transactron/utils/_typing.py +++ b/transactron/utils/_typing.py @@ -25,6 +25,7 @@ "LayoutLike", "SwitchKey", "MethodLayout", + "SrcLoc", "SignalBundle", "LayoutListField", "LayoutList", @@ -50,6 +51,7 @@ ) SwitchKey: TypeAlias = str | int | Enum MethodLayout: TypeAlias = LayoutLike +SrcLoc: TypeAlias = tuple[str, int] # Internal Coreblocks types SignalBundle: TypeAlias = Signal | Record | View | Iterable["SignalBundle"] | Mapping[str, "SignalBundle"] From a5d9e32f9965cb92a265e84c8be42fe0ceaef72c Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 19 Dec 2023 10:42:33 +0100 Subject: [PATCH 02/35] More useful source locations --- transactron/core.py | 18 +++++++------ transactron/lib/connectors.py | 33 ++++++++++++++++-------- transactron/lib/transformers.py | 13 +++++++--- transactron/utils/transactron_helpers.py | 8 +++++- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/transactron/core.py b/transactron/core.py index 59c015f2c..8ab4622e1 100644 --- a/transactron/core.py +++ b/transactron/core.py @@ -733,8 +733,8 @@ class TransactionBase(Owned, Protocol): simultaneous_list: list[TransactionOrMethod] independent_list: list[TransactionOrMethod] - def __init__(self, *, src_loc_at: int): - self.src_loc = tracer.get_src_loc(src_loc_at) + def __init__(self, *, src_loc: int | SrcLoc): + self.src_loc = get_src_loc(src_loc) self.method_uses: dict["Method", Tuple[Record, ValueLike]] = dict() self.relations: list[RelationBase] = [] self.simultaneous_list: list[TransactionOrMethod] = [] @@ -902,7 +902,7 @@ class Transaction(TransactionBase): """ def __init__( - self, *, name: Optional[str] = None, manager: Optional[TransactionManager] = None, src_loc_at: int = 0 + self, *, name: Optional[str] = None, manager: Optional[TransactionManager] = None, src_loc: int | SrcLoc = 0 ): """ Parameters @@ -915,10 +915,11 @@ def __init__( manager: TransactionManager The `TransactionManager` controlling this `Transaction`. If omitted, the manager is received from `TransactionContext`. - src_loc_at: int + src_loc: int | SrcLoc How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - super().__init__(src_loc_at=1 + src_loc_at) + super().__init__(src_loc=get_src_loc(src_loc)) self.owner, owner_name = get_caller_class_name(default="$transaction") self.name = name or tracer.get_var_name(depth=2, default=owner_name) if manager is None: @@ -1010,7 +1011,7 @@ def __init__( o: MethodLayout = (), nonexclusive: bool = False, single_caller: bool = False, - src_loc_at: int = 0, + src_loc: int | SrcLoc = 0, ): """ Parameters @@ -1033,10 +1034,11 @@ def __init__( If true, this method is intended to be called from a single transaction. An error will be thrown if called from multiple transactions. - src_loc_at: int + src_loc: int | SrcLoc How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - super().__init__(src_loc_at=1 + src_loc_at) + super().__init__(src_loc=get_src_loc(src_loc)) self.owner, owner_name = get_caller_class_name(default="$method") self.name = name or tracer.get_var_name(depth=2, default=owner_name) self.ready = Signal(name=self.owned_name + "_ready") diff --git a/transactron/lib/connectors.py b/transactron/lib/connectors.py index 3b9b18b83..f2a0496ab 100644 --- a/transactron/lib/connectors.py +++ b/transactron/lib/connectors.py @@ -1,6 +1,7 @@ from amaranth import * import amaranth.lib.fifo from ..core import * +from ..utils import SrcLoc, get_src_loc __all__ = [ "FIFO", @@ -44,8 +45,8 @@ def __init__(self, layout: MethodLayout, depth: int, fifo_type=amaranth.lib.fifo self.depth = depth self.fifoType = fifo_type - self.read = Method(o=layout) - self.write = Method(i=layout) + self.read = Method(o=layout, src_loc=1) + self.write = Method(i=layout, src_loc=1) def elaborate(self, platform): m = TModule() @@ -97,9 +98,9 @@ def __init__(self, layout: MethodLayout): layout: record layout The format of records forwarded. """ - self.read = Method(o=layout) - self.write = Method(i=layout) - self.clear = Method() + self.read = Method(o=layout, src_loc=1) + self.write = Method(i=layout, src_loc=1) + self.clear = Method(src_loc=1) self.head = Record.like(self.read.data_out) self.clear.add_conflict(self.read, Priority.LEFT) @@ -164,8 +165,8 @@ def __init__(self, layout: MethodLayout = (), rev_layout: MethodLayout = ()): rev_layout: record layout The format of records forwarded in the reverse direction. """ - self.read = Method(o=layout, i=rev_layout) - self.write = Method(i=layout, o=rev_layout) + self.read = Method(o=layout, i=rev_layout, src_loc=1) + self.write = Method(i=layout, o=rev_layout, src_loc=1) def elaborate(self, platform): m = TModule() @@ -197,7 +198,7 @@ class ConnectTrans(Elaboratable): layouts. """ - def __init__(self, method1: Method, method2: Method): + def __init__(self, method1: Method, method2: Method, *, src_loc: int | SrcLoc = 0): """ Parameters ---------- @@ -205,14 +206,18 @@ def __init__(self, method1: Method, method2: Method): First method. method2: Method Second method. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.method1 = method1 self.method2 = method2 + self.src_loc = get_src_loc(src_loc) def elaborate(self, platform): m = TModule() - with Transaction().body(m): + with Transaction(src_loc=self.src_loc).body(m): data1 = Record.like(self.method1.data_out) data2 = Record.like(self.method2.data_out) @@ -229,7 +234,7 @@ class ManyToOneConnectTrans(Elaboratable): transactions. Equivalent to a set of `ConnectTrans`. """ - def __init__(self, *, get_results: list[Method], put_result: Method): + def __init__(self, *, get_results: list[Method], put_result: Method, src_loc: int | SrcLoc = 0): """ Parameters ---------- @@ -237,16 +242,22 @@ def __init__(self, *, get_results: list[Method], put_result: Method): Methods to be connected to the `put_result` method. put_result: Method Common method for each of the connections created. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.get_results = get_results self.m_put_result = put_result self.count = len(self.get_results) + self.src_loc = get_src_loc(src_loc) def elaborate(self, platform): m = TModule() for i in range(self.count): - m.submodules[f"ManyToOneConnectTrans_input_{i}"] = ConnectTrans(self.m_put_result, self.get_results[i]) + m.submodules[f"ManyToOneConnectTrans_input_{i}"] = ConnectTrans( + self.m_put_result, self.get_results[i], src_loc=self.src_loc + ) return m diff --git a/transactron/lib/transformers.py b/transactron/lib/transformers.py index 8053180bd..ca0d1344e 100644 --- a/transactron/lib/transformers.py +++ b/transactron/lib/transformers.py @@ -1,7 +1,10 @@ from abc import ABC from amaranth import * + +from transactron.utils.transactron_helpers import get_src_loc from ..core import * from ..core import RecordDict +from ..utils import SrcLoc from typing import Optional from collections.abc import Callable from transactron.utils import ValueLike, assign, AssignType, ModuleLike @@ -289,16 +292,20 @@ class Collector(Transformer): Method which returns single result of provided methods. """ - def __init__(self, targets: list[Method]): + def __init__(self, targets: list[Method], *, src_loc: int | SrcLoc = 0): """ Parameters ---------- method_list: list[Method] List of methods from which results will be collected. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.method_list = targets layout = targets[0].data_out.layout - self.method = Method(o=layout) + self.src_loc = get_src_loc(src_loc) + self.method = Method(o=layout, src_loc=self.src_loc) for method in targets: if layout != method.data_out.layout: @@ -310,7 +317,7 @@ def elaborate(self, platform): m.submodules.forwarder = forwarder = Forwarder(self.method.data_out.layout) m.submodules.connect = ManyToOneConnectTrans( - get_results=[get for get in self.method_list], put_result=forwarder.write + get_results=[get for get in self.method_list], put_result=forwarder.write, src_loc=self.src_loc ) self.method.proxy(m, forwarder.read) diff --git a/transactron/utils/transactron_helpers.py b/transactron/utils/transactron_helpers.py index d7e610d6c..3b271ed27 100644 --- a/transactron/utils/transactron_helpers.py +++ b/transactron/utils/transactron_helpers.py @@ -2,9 +2,10 @@ from contextlib import contextmanager from typing import Optional, Any, Concatenate, TypeGuard, TypeVar from collections.abc import Callable, Mapping -from ._typing import ROGraph, GraphCC +from ._typing import ROGraph, GraphCC, SrcLoc from inspect import Parameter, signature from amaranth import * +from amaranth import tracer __all__ = [ @@ -13,6 +14,7 @@ "def_helper", "method_def_helper", "mock_def_helper", + "get_src_loc", ] T = TypeVar("T") @@ -104,3 +106,7 @@ def silence_mustuse(elaboratable: Elaboratable): except Exception: elaboratable._MustUse__silence = True # type: ignore raise + + +def get_src_loc(src_loc_at: int | SrcLoc) -> SrcLoc: + return tracer.get_src_loc(1 + src_loc_at) if isinstance(src_loc_at, int) else src_loc_at From c658fc73baf25607fbb85d309f5a1b16360b96ca Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 19 Dec 2023 11:32:37 +0100 Subject: [PATCH 03/35] Source locations in Transactron lib --- transactron/core.py | 7 ++-- transactron/lib/adapters.py | 20 ++++++++--- transactron/lib/buttons.py | 17 +++++++--- transactron/lib/connectors.py | 34 +++++++++++++------ transactron/lib/fifo.py | 16 +++++---- transactron/lib/reqres.py | 32 +++++++++++++----- transactron/lib/simultaneous.py | 5 ++- transactron/lib/storage.py | 23 +++++++++---- transactron/lib/transformers.py | 43 ++++++++++++++++++++---- transactron/utils/transactron_helpers.py | 4 +-- 10 files changed, 151 insertions(+), 50 deletions(-) diff --git a/transactron/core.py b/transactron/core.py index 8ab4622e1..c0698de32 100644 --- a/transactron/core.py +++ b/transactron/core.py @@ -1052,7 +1052,7 @@ def __init__( assert len(self.data_in) == 0 @staticmethod - def like(other: "Method", *, name: Optional[str] = None) -> "Method": + def like(other: "Method", *, name: Optional[str] = None, src_loc: int | SrcLoc = 0) -> "Method": """Constructs a new `Method` based on another. The returned `Method` has the same input/output data layouts as the @@ -1064,13 +1064,16 @@ def like(other: "Method", *, name: Optional[str] = None) -> "Method": The `Method` which serves as a blueprint for the new `Method`. name : str, optional Name of the new `Method`. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. Returns ------- Method The freshly constructed `Method`. """ - return Method(name=name, i=other.data_in.layout, o=other.data_out.layout) + return Method(name=name, i=other.data_in.layout, o=other.data_out.layout, src_loc=get_src_loc(src_loc)) def proxy(self, m: TModule, method: "Method"): """Define as a proxy for another method. diff --git a/transactron/lib/adapters.py b/transactron/lib/adapters.py index d367fe577..5cd69e804 100644 --- a/transactron/lib/adapters.py +++ b/transactron/lib/adapters.py @@ -1,4 +1,6 @@ from amaranth import * + +from ..utils import SrcLoc, get_src_loc from ..core import * from ..core import SignalBundle from typing import Optional @@ -42,14 +44,18 @@ class AdapterTrans(AdapterBase): Data returned from the `iface` method. """ - def __init__(self, iface: Method): + def __init__(self, iface: Method, *, src_loc: int | SrcLoc = 0): """ Parameters ---------- iface: Method The method to be called by the transaction. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ super().__init__(iface) + self.src_loc = get_src_loc(src_loc) self.data_in = Record.like(iface.data_in) self.data_out = Record.like(iface.data_out) @@ -60,7 +66,7 @@ def elaborate(self, platform): data_in = Signal.like(self.data_in) m.d.comb += data_in.eq(self.data_in) - with Transaction(name=f"AdapterTrans_{self.iface.name}").body(m, request=self.en): + with Transaction(name=f"AdapterTrans_{self.iface.name}", src_loc=self.src_loc).body(m, request=self.en): data_out = self.iface(m, data_in) m.d.top_comb += self.data_out.eq(data_out) m.d.comb += self.done.eq(1) @@ -86,7 +92,9 @@ class Adapter(AdapterBase): Data passed as argument to the defined method. """ - def __init__(self, *, name: Optional[str] = None, i: MethodLayout = (), o: MethodLayout = ()): + def __init__( + self, *, name: Optional[str] = None, i: MethodLayout = (), o: MethodLayout = (), src_loc: int | SrcLoc = 0 + ): """ Parameters ---------- @@ -94,8 +102,12 @@ def __init__(self, *, name: Optional[str] = None, i: MethodLayout = (), o: Metho The input layout of the defined method. o: record layout The output layout of the defined method. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - super().__init__(Method(name=name, i=i, o=o)) + src_loc = get_src_loc(src_loc) + super().__init__(Method(name=name, i=i, o=o, src_loc=src_loc)) self.data_in = Record.like(self.iface.data_out) self.data_out = Record.like(self.iface.data_in) diff --git a/transactron/lib/buttons.py b/transactron/lib/buttons.py index 82955779f..ec3c796ce 100644 --- a/transactron/lib/buttons.py +++ b/transactron/lib/buttons.py @@ -1,5 +1,6 @@ from amaranth import * from ..core import * +from ..utils import SrcLoc, get_src_loc __all__ = ["ClickIn", "ClickOut"] @@ -23,14 +24,18 @@ class ClickIn(Elaboratable): The data input. """ - def __init__(self, layout: MethodLayout): + def __init__(self, layout: MethodLayout, src_loc: int | SrcLoc = 0): """ Parameters ---------- layout: record layout The data format for the input. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - self.get = Method(o=layout) + src_loc = get_src_loc(src_loc) + self.get = Method(o=layout, src_loc=src_loc) self.btn = Signal() self.dat = Record(layout) @@ -76,14 +81,18 @@ class ClickOut(Elaboratable): The data output. """ - def __init__(self, layout: MethodLayout): + def __init__(self, layout: MethodLayout, *, src_loc: int | SrcLoc = 0): """ Parameters ---------- layout: record layout The data format for the output. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - self.put = Method(i=layout) + src_loc = get_src_loc(src_loc) + self.put = Method(i=layout, src_loc=src_loc) self.btn = Signal() self.dat = Record(layout) diff --git a/transactron/lib/connectors.py b/transactron/lib/connectors.py index f2a0496ab..81918c2a6 100644 --- a/transactron/lib/connectors.py +++ b/transactron/lib/connectors.py @@ -29,7 +29,9 @@ class FIFO(Elaboratable): The write method. Accepts a `Record`, returns empty result. """ - def __init__(self, layout: MethodLayout, depth: int, fifo_type=amaranth.lib.fifo.SyncFIFO): + def __init__( + self, layout: MethodLayout, depth: int, fifo_type=amaranth.lib.fifo.SyncFIFO, *, src_loc: int | SrcLoc = 0 + ): """ Parameters ---------- @@ -40,13 +42,17 @@ def __init__(self, layout: MethodLayout, depth: int, fifo_type=amaranth.lib.fifo fifoType: Elaboratable FIFO module conforming to Amaranth library FIFO interface. Defaults to SyncFIFO. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.width = len(Record(layout)) self.depth = depth self.fifoType = fifo_type - self.read = Method(o=layout, src_loc=1) - self.write = Method(i=layout, src_loc=1) + src_loc = get_src_loc(src_loc) + self.read = Method(o=layout, src_loc=src_loc) + self.write = Method(i=layout, src_loc=src_loc) def elaborate(self, platform): m = TModule() @@ -91,16 +97,20 @@ class Forwarder(Elaboratable): The write method. Accepts a `Record`, returns empty result. """ - def __init__(self, layout: MethodLayout): + def __init__(self, layout: MethodLayout, *, src_loc: int | SrcLoc = 0): """ Parameters ---------- layout: record layout The format of records forwarded. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - self.read = Method(o=layout, src_loc=1) - self.write = Method(i=layout, src_loc=1) - self.clear = Method(src_loc=1) + src_loc = get_src_loc(src_loc) + self.read = Method(o=layout, src_loc=src_loc) + self.write = Method(i=layout, src_loc=src_loc) + self.clear = Method(src_loc=src_loc) self.head = Record.like(self.read.data_out) self.clear.add_conflict(self.read, Priority.LEFT) @@ -156,7 +166,7 @@ class Connect(Elaboratable): `Record`. """ - def __init__(self, layout: MethodLayout = (), rev_layout: MethodLayout = ()): + def __init__(self, layout: MethodLayout = (), rev_layout: MethodLayout = (), *, src_loc: int | SrcLoc = 0): """ Parameters ---------- @@ -164,9 +174,13 @@ def __init__(self, layout: MethodLayout = (), rev_layout: MethodLayout = ()): The format of records forwarded. rev_layout: record layout The format of records forwarded in the reverse direction. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ - self.read = Method(o=layout, i=rev_layout, src_loc=1) - self.write = Method(i=layout, o=rev_layout, src_loc=1) + src_loc = get_src_loc(src_loc) + self.read = Method(o=layout, i=rev_layout, src_loc=src_loc) + self.write = Method(i=layout, o=rev_layout, src_loc=src_loc) def elaborate(self, platform): m = TModule() diff --git a/transactron/lib/fifo.py b/transactron/lib/fifo.py index a39d006a9..3c74dad04 100644 --- a/transactron/lib/fifo.py +++ b/transactron/lib/fifo.py @@ -1,7 +1,8 @@ from amaranth import * from transactron import Method, def_method, Priority, TModule -from transactron.utils._typing import ValueLike, MethodLayout +from transactron.utils._typing import ValueLike, MethodLayout, SrcLoc from transactron.utils.amaranth_ext import mod_incr +from transactron.utils.transactron_helpers import get_src_loc class BasicFifo(Elaboratable): @@ -21,7 +22,7 @@ class BasicFifo(Elaboratable): """ - def __init__(self, layout: MethodLayout, depth: int) -> None: + def __init__(self, layout: MethodLayout, depth: int, *, src_loc: int | SrcLoc = 0) -> None: """ Parameters ---------- @@ -30,15 +31,18 @@ def __init__(self, layout: MethodLayout, depth: int) -> None: If integer is given, Record with field `data` and width of this paramter is used as internal layout. depth: int Size of the FIFO. - + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.layout = layout self.width = len(Record(self.layout)) self.depth = depth - self.read = Method(o=self.layout) - self.write = Method(i=self.layout) - self.clear = Method() + src_loc = get_src_loc(src_loc) + self.read = Method(o=self.layout, src_loc=src_loc) + self.write = Method(i=self.layout, src_loc=src_loc) + self.clear = Method(src_loc=src_loc) self.head = Record(self.layout) self.buff = Memory(width=self.width, depth=self.depth) diff --git a/transactron/lib/reqres.py b/transactron/lib/reqres.py index 5f2e20ba6..9bfcdee2b 100644 --- a/transactron/lib/reqres.py +++ b/transactron/lib/reqres.py @@ -1,5 +1,6 @@ from amaranth import * from ..core import * +from ..utils import SrcLoc, get_src_loc from .connectors import Forwarder, FIFO from transactron.lib import BasicFifo from amaranth.utils import * @@ -47,7 +48,7 @@ class ArgumentsToResultsZipper(Elaboratable): record with two fields: 'args' and 'results'. """ - def __init__(self, args_layout: MethodLayout, results_layout: MethodLayout): + def __init__(self, args_layout: MethodLayout, results_layout: MethodLayout, src_loc: int | SrcLoc = 0): """ Parameters ---------- @@ -55,20 +56,24 @@ def __init__(self, args_layout: MethodLayout, results_layout: MethodLayout): The format of arguments. results_layout: record layout The format of results. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ + self.src_loc = get_src_loc(src_loc) self.results_layout = results_layout self.args_layout = args_layout self.output_layout = [("args", self.args_layout), ("results", results_layout)] - self.write_args = Method(i=self.args_layout) - self.write_results = Method(i=self.results_layout) - self.read = Method(o=self.output_layout) + self.write_args = Method(i=self.args_layout, src_loc=self.src_loc) + self.write_results = Method(i=self.results_layout, src_loc=self.src_loc) + self.read = Method(o=self.output_layout, src_loc=self.src_loc) def elaborate(self, platform): m = TModule() - fifo = FIFO(self.args_layout, depth=2) - forwarder = Forwarder(self.results_layout) + fifo = FIFO(self.args_layout, depth=2, src_loc=self.src_loc) + forwarder = Forwarder(self.results_layout, src_loc=self.src_loc) m.submodules.fifo = fifo m.submodules.forwarder = forwarder @@ -117,6 +122,7 @@ def __init__( serialized_req_method: Method, serialized_resp_method: Method, depth: int = 4, + src_loc: int | SrcLoc = 0 ): """ Parameters @@ -130,7 +136,11 @@ def __init__( depth: int Number of requests which can be forwarded to server, before server provides first response. Describe the resistance of `Serializer` to latency of server in case when server is fully pipelined. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ + self.src_loc = get_src_loc(src_loc) self.port_count = port_count self.serialized_req_method = serialized_req_method self.serialized_resp_method = serialized_resp_method @@ -140,13 +150,17 @@ def __init__( self.id_layout = [("id", log2_int(self.port_count))] self.clear = Method() - self.serialize_in = [Method.like(self.serialized_req_method) for _ in range(self.port_count)] - self.serialize_out = [Method.like(self.serialized_resp_method) for _ in range(self.port_count)] + self.serialize_in = [ + Method.like(self.serialized_req_method, src_loc=self.src_loc) for _ in range(self.port_count) + ] + self.serialize_out = [ + Method.like(self.serialized_resp_method, src_loc=self.src_loc) for _ in range(self.port_count) + ] def elaborate(self, platform) -> TModule: m = TModule() - pending_requests = BasicFifo(self.id_layout, self.depth) + pending_requests = BasicFifo(self.id_layout, self.depth, src_loc=self.src_loc) m.submodules.pending_requests = pending_requests for i in range(self.port_count): diff --git a/transactron/lib/simultaneous.py b/transactron/lib/simultaneous.py index 48814cde8..7d4955ba7 100644 --- a/transactron/lib/simultaneous.py +++ b/transactron/lib/simultaneous.py @@ -1,4 +1,6 @@ from amaranth import * + +from ..utils import get_src_loc from ..core import * from ..core import TransactionBase from contextlib import contextmanager @@ -59,6 +61,7 @@ def condition(m: TModule, *, nonblocking: bool = False, priority: bool = True): this = TransactionBase.get() transactions = list[Transaction]() last = False + src_loc = get_src_loc(0) @contextmanager def branch(cond: Optional[ValueLike] = None): @@ -67,7 +70,7 @@ def branch(cond: Optional[ValueLike] = None): raise RuntimeError("Condition clause added after catch-all") req = cond if cond is not None else 1 name = f"{this.name}_cond{len(transactions)}" - with (transaction := Transaction(name=name)).body(m, request=req): + with (transaction := Transaction(name=name, src_loc=src_loc)).body(m, request=req): yield if transactions and priority: transactions[-1].schedule_before(transaction) diff --git a/transactron/lib/storage.py b/transactron/lib/storage.py index 9d95f6a83..4b33f8eb1 100644 --- a/transactron/lib/storage.py +++ b/transactron/lib/storage.py @@ -1,6 +1,7 @@ from amaranth import * from amaranth.utils import * from ..core import * +from ..utils import SrcLoc, get_src_loc from typing import Optional from transactron.utils import assign, AssignType from .reqres import ArgumentsToResultsZipper @@ -28,7 +29,13 @@ class MemoryBank(Elaboratable): """ def __init__( - self, *, data_layout: MethodLayout, elem_count: int, granularity: Optional[int] = None, safe_writes: bool = True + self, + *, + data_layout: MethodLayout, + elem_count: int, + granularity: Optional[int] = None, + safe_writes: bool = True, + src_loc: int | SrcLoc = 0 ): """ Parameters @@ -44,7 +51,11 @@ def __init__( Set to `False` if an optimisation can be done to increase throughput of writes. This will cause that writes will be reordered with respect to reads eg. in sequence "read A, write A X", read can return "X" even when write was called later. By default `True`, which disable optimisation. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ + self.src_loc = get_src_loc(src_loc) self.data_layout = data_layout self.elem_count = elem_count self.granularity = granularity @@ -57,9 +68,9 @@ def __init__( if self.granularity is not None: self.write_layout.append(("mask", self.width // self.granularity)) - self.read_req = Method(i=self.read_req_layout) - self.read_resp = Method(o=self.data_layout) - self.write = Method(i=self.write_layout) + self.read_req = Method(i=self.read_req_layout, src_loc=self.src_loc) + self.read_resp = Method(o=self.data_layout, src_loc=self.src_loc) + self.write = Method(i=self.write_layout, src_loc=self.src_loc) self._internal_read_resp_trans = None def elaborate(self, platform) -> TModule: @@ -79,12 +90,12 @@ def elaborate(self, platform) -> TModule: zipper = ArgumentsToResultsZipper([("valid", 1)], self.data_layout) m.submodules.zipper = zipper - self._internal_read_resp_trans = Transaction() + self._internal_read_resp_trans = Transaction(src_loc=self.src_loc) with self._internal_read_resp_trans.body(m, request=read_output_valid): m.d.sync += read_output_valid.eq(0) zipper.write_results(m, read_port.data) - write_trans = Transaction() + write_trans = Transaction(src_loc=self.src_loc) with write_trans.body(m, request=write_req | (~read_output_valid & write_pending)): if self.safe_writes: with m.If(write_pending): diff --git a/transactron/lib/transformers.py b/transactron/lib/transformers.py index ca0d1344e..71c783679 100644 --- a/transactron/lib/transformers.py +++ b/transactron/lib/transformers.py @@ -70,6 +70,7 @@ def __init__( *, i_transform: Optional[tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, o_transform: Optional[tuple[MethodLayout, Callable[[TModule, Record], RecordDict]]] = None, + src_loc: int | SrcLoc = 0 ): """ Parameters @@ -84,6 +85,9 @@ def __init__( Output mapping function. If specified, it should be a pair of a function and a output layout for the transformed method. If not present, output is passed unmodified. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ if i_transform is None: i_transform = (target.data_in.layout, lambda _, x: x) @@ -91,7 +95,8 @@ def __init__( o_transform = (target.data_out.layout, lambda _, x: x) self.target = target - self.method = Method(i=i_transform[0], o=o_transform[0]) + src_loc = get_src_loc(src_loc) + self.method = Method(i=i_transform[0], o=o_transform[0], src_loc=src_loc) self.i_fun = i_transform[1] self.o_fun = o_transform[1] @@ -129,7 +134,9 @@ def __init__( target: Method, condition: Callable[[TModule, Record], ValueLike], default: Optional[RecordDict] = None, + *, use_condition: bool = False, + src_loc: int | SrcLoc = 0 ): """ Parameters @@ -145,13 +152,19 @@ def __init__( use_condition : bool Instead of `m.If` use simultaneus `condition` which allow to execute this filter if the condition is False and target is not ready. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ if default is None: default = Record.like(target.data_out) self.target = target self.use_condition = use_condition - self.method = Method(i=target.data_in.layout, o=target.data_out.layout, single_caller=self.use_condition) + src_loc = get_src_loc(src_loc) + self.method = Method( + i=target.data_in.layout, o=target.data_out.layout, single_caller=self.use_condition, src_loc=src_loc + ) self.condition = condition self.default = default @@ -184,6 +197,8 @@ def __init__( self, targets: list[Method], combiner: Optional[tuple[MethodLayout, Callable[[TModule, list[Record]], RecordDict]]] = None, + *, + src_loc: int | SrcLoc = 0 ): """Method product. @@ -202,6 +217,9 @@ def __init__( A pair of the output layout and the combiner function. The combiner function takes two parameters: a `Module` and a list of outputs of the target methods. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. Attributes ---------- @@ -212,7 +230,8 @@ def __init__( combiner = (targets[0].data_out.layout, lambda _, x: x[0]) self.targets = targets self.combiner = combiner - self.method = Method(i=targets[0].data_in.layout, o=combiner[0]) + src_loc = get_src_loc(src_loc) + self.method = Method(i=targets[0].data_in.layout, o=combiner[0], src_loc=src_loc) def elaborate(self, platform): m = TModule() @@ -232,6 +251,8 @@ def __init__( self, targets: list[Method], combiner: Optional[tuple[MethodLayout, Callable[[TModule, list[tuple[Value, Record]]], RecordDict]]] = None, + *, + src_loc: int | SrcLoc = 0 ): """Method product with optional calling. @@ -251,6 +272,9 @@ def __init__( combiner function takes two parameters: a `Module` and a list of pairs. Each pair contains a bit which signals that a given call succeeded, and the result of the call. + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. Attributes ---------- @@ -261,7 +285,8 @@ def __init__( combiner = ([], lambda _, __: {}) self.targets = targets self.combiner = combiner - self.method = Method(i=targets[0].data_in.layout, o=combiner[0]) + self.src_loc = get_src_loc(src_loc) + self.method = Method(i=targets[0].data_in.layout, o=combiner[0], src_loc=self.src_loc) def elaborate(self, platform): m = TModule() @@ -271,7 +296,7 @@ def _(arg): results: list[tuple[Value, Record]] = [] for target in self.targets: success = Signal() - with Transaction().body(m): + with Transaction(src_loc=self.src_loc).body(m): m.d.comb += success.eq(1) results.append((success, target(m, arg))) return self.combiner[1](m, results) @@ -314,7 +339,7 @@ def __init__(self, targets: list[Method], *, src_loc: int | SrcLoc = 0): def elaborate(self, platform): m = TModule() - m.submodules.forwarder = forwarder = Forwarder(self.method.data_out.layout) + m.submodules.forwarder = forwarder = Forwarder(self.method.data_out.layout, src_loc=self.src_loc) m.submodules.connect = ManyToOneConnectTrans( get_results=[get for get in self.method_list], put_result=forwarder.write, src_loc=self.src_loc @@ -378,6 +403,7 @@ def __init__( *, i_fun: Optional[Callable[[TModule, Record], RecordDict]] = None, o_fun: Optional[Callable[[TModule, Record], RecordDict]] = None, + src_loc: int | SrcLoc = 0 ): """ Parameters @@ -390,11 +416,15 @@ def __init__( Input transformation (`method1` to `method2`). o_fun: function or Method, optional Output transformation (`method2` to `method1`). + src_loc: int | SrcLoc + How many stack frames deep the source location is taken from. + Alternatively, the source location to use instead of the default. """ self.method1 = method1 self.method2 = method2 self.i_fun = i_fun or (lambda _, x: x) self.o_fun = o_fun or (lambda _, x: x) + self.src_loc = get_src_loc(src_loc) def elaborate(self, platform): m = TModule() @@ -403,6 +433,7 @@ def elaborate(self, platform): self.method2, i_transform=(self.method1.data_out.layout, self.i_fun), o_transform=(self.method1.data_in.layout, self.o_fun), + src_loc=self.src_loc, ) m.submodules.connect = ConnectTrans(self.method1, transformer.method) diff --git a/transactron/utils/transactron_helpers.py b/transactron/utils/transactron_helpers.py index 3b271ed27..13dd827fe 100644 --- a/transactron/utils/transactron_helpers.py +++ b/transactron/utils/transactron_helpers.py @@ -108,5 +108,5 @@ def silence_mustuse(elaboratable: Elaboratable): raise -def get_src_loc(src_loc_at: int | SrcLoc) -> SrcLoc: - return tracer.get_src_loc(1 + src_loc_at) if isinstance(src_loc_at, int) else src_loc_at +def get_src_loc(src_loc: int | SrcLoc) -> SrcLoc: + return tracer.get_src_loc(1 + src_loc) if isinstance(src_loc, int) else src_loc From edbd3fd7939247845ae7c0d98eff4f887cb927d3 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Wed, 20 Dec 2023 17:25:49 +0100 Subject: [PATCH 04/35] Profiler --- test/common/__init__.py | 1 + test/common/infrastructure.py | 14 +++++- test/common/profiler.py | 81 +++++++++++++++++++++++++++++++++++ transactron/core.py | 5 +++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 test/common/profiler.py diff --git a/test/common/__init__.py b/test/common/__init__.py index 4b44c291f..46eb4b5a1 100644 --- a/test/common/__init__.py +++ b/test/common/__init__.py @@ -2,4 +2,5 @@ from .infrastructure import * # noqa: F401 from .sugar import * # noqa: F401 from .testbenchio import * # noqa: F401 +from .profiler import * # noqa: F401 from transactron.utils import data_layout # noqa: F401 diff --git a/test/common/infrastructure.py b/test/common/infrastructure.py index e1141a0eb..2efdfb5d1 100644 --- a/test/common/infrastructure.py +++ b/test/common/infrastructure.py @@ -7,6 +7,7 @@ from amaranth import * from amaranth.sim import * from .testbenchio import TestbenchIO +from .profiler import profiler_process, Profile from ..gtkw_extension import write_vcd_ext from transactron import Method from transactron.lib import AdapterTrans @@ -101,7 +102,7 @@ def elaborate(self, platform) -> HasElaborate: class PysimSimulator(Simulator): def __init__(self, module: HasElaborate, max_cycles: float = 10e4, add_transaction_module=True, traces_file=None): - test_module = TestModule(module, add_transaction_module) + self.test_module = TestModule(module, add_transaction_module) tested_module = test_module.tested_module super().__init__(test_module) @@ -151,8 +152,19 @@ def run_simulation(self, module: HasElaborate, max_cycles: float = 10e4, add_tra module, max_cycles=max_cycles, add_transaction_module=add_transaction_module, traces_file=traces_file ) yield sim + + profile = None + if "__TRANSACTRON_PROFILE" in os.environ and isinstance(sim.test_module.tested_module, TransactionModule): + profile = Profile() + sim.add_sync_process(profiler_process(sim.test_module.tested_module.transactionManager, profile)) + res = sim.run() + if profile is not None: + profile_dir = "test/__profiles__" + profile_file = unittest.TestCase.id(self) + profile.encode(f"{profile_dir}/{profile_file}.json") + self.assertTrue(res, "Simulation time limit exceeded") def tick(self, cycle_cnt=1): diff --git a/test/common/profiler.py b/test/common/profiler.py new file mode 100644 index 000000000..e79a87a6d --- /dev/null +++ b/test/common/profiler.py @@ -0,0 +1,81 @@ +import json +from typing import Optional +from dataclasses import dataclass, field, is_dataclass, asdict +from transactron.core import MethodMap, TransactionManager, Transaction, Method +from transactron.lib import SrcLoc +from amaranth.sim import * + +__all__ = ["profiler_process"] + + +class ProfileJSONEncoder(json.JSONEncoder): + def default(self, o): + if is_dataclass(o): + return asdict(o) + return super().default(o) + + +@dataclass +class ProfileInfo: + name: str + src_loc: SrcLoc + is_transaction: bool + + +@dataclass +class Profile: + transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) + waiting_transactions: dict[int, dict[int, int]] = field(default_factory=dict) + running: dict[int, dict[int, Optional[int]]] = field(default_factory=dict) + + def encode(self, file_name: str): + with open(file_name, "w") as fp: + json.dump(self, fp, cls=ProfileJSONEncoder) + + +def profiler_process(transaction_manager: TransactionManager, profile: Profile): + method_map = MethodMap(transaction_manager.transactions) + cgr, _, _ = TransactionManager._conflict_graph(method_map) + + for transaction in method_map.transactions: + profile.transactions_and_methods[id(transaction)] = ProfileInfo( + transaction.owned_name, transaction.src_loc, True + ) + + for method in method_map.methods: + profile.transactions_and_methods[id(method)] = ProfileInfo(method.name, method.src_loc, False) + + cycle = 0 + + yield Passive() + while True: + yield Settle() + + profile.waiting_transactions[cycle] = {} + profile.running[cycle] = {} + + for transaction in method_map.transactions: + request = yield transaction.request + runnable = yield transaction.runnable + grant = yield transaction.grant + + if grant: + profile.running[cycle][id(transaction)] = None + elif request and runnable: + for transaction2 in cgr[transaction]: + if (yield transaction2.grant): + profile.waiting_transactions[cycle][id(transaction)] = id(transaction2) + + for method in method_map.methods: + if (yield method.run): + for t_or_m in method_map.method_parents[method]: + if ( + isinstance(t_or_m, Transaction) + and (yield t_or_m.grant) + or isinstance(t_or_m, Method) + and (yield t_or_m.run) + ): + profile.running[cycle][id(method)] = id(t_or_m) + + yield + cycle = cycle + 1 diff --git a/transactron/core.py b/transactron/core.py index c0698de32..9f1a59622 100644 --- a/transactron/core.py +++ b/transactron/core.py @@ -75,6 +75,7 @@ def __init__(self, transactions: Iterable["Transaction"]): self.methods_by_transaction = dict[Transaction, list[Method]]() self.transactions_by_method = defaultdict[Method, list[Transaction]](list) self.readiness_by_method_and_transaction = dict[tuple[Transaction, Method], ValueLike]() + self.method_parents = defaultdict[Method, list[TransactionBase]](list) def rec(transaction: Transaction, source: TransactionBase): for method, (arg_rec, _) in source.method_uses.items(): @@ -91,6 +92,10 @@ def rec(transaction: Transaction, source: TransactionBase): self.methods_by_transaction[transaction] = [] rec(transaction, transaction) + for transaction_or_method in self.methods_and_transactions: + for method in transaction_or_method.method_uses.keys(): + self.method_parents[method].append(transaction_or_method) + def transactions_for(self, elem: TransactionOrMethod) -> Collection["Transaction"]: if isinstance(elem, Transaction): return [elem] From b3f160d164b7853b68a02b60236063832528dca8 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Wed, 20 Dec 2023 18:00:35 +0100 Subject: [PATCH 05/35] Make lint pass --- test/common/infrastructure.py | 8 ++-- test/common/profiler.py | 88 ++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/test/common/infrastructure.py b/test/common/infrastructure.py index 7c8568f37..c0eb87bc3 100644 --- a/test/common/infrastructure.py +++ b/test/common/infrastructure.py @@ -138,8 +138,8 @@ def _wrapping_function(self): class PysimSimulator(Simulator): def __init__(self, module: HasElaborate, max_cycles: float = 10e4, add_transaction_module=True, traces_file=None): - self.test_module = TestModule(module, add_transaction_module) - tested_module = test_module.tested_module + test_module = TestModule(module, add_transaction_module) + self.tested_module = tested_module = test_module.tested_module super().__init__(test_module) clk_period = 1e-6 @@ -194,9 +194,9 @@ def run_simulation(self, module: HasElaborate, max_cycles: float = 10e4, add_tra yield sim profile = None - if "__TRANSACTRON_PROFILE" in os.environ and isinstance(sim.test_module.tested_module, TransactionModule): + if "__TRANSACTRON_PROFILE" in os.environ and isinstance(sim.tested_module, TransactionModule): profile = Profile() - sim.add_sync_process(profiler_process(sim.test_module.tested_module.transactionManager, profile)) + sim.add_sync_process(profiler_process(sim.tested_module.transactionManager, profile)) res = sim.run() diff --git a/test/common/profiler.py b/test/common/profiler.py index e79a87a6d..2286d655e 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -1,9 +1,10 @@ import json from typing import Optional from dataclasses import dataclass, field, is_dataclass, asdict +from amaranth.sim import * from transactron.core import MethodMap, TransactionManager, Transaction, Method from transactron.lib import SrcLoc -from amaranth.sim import * +from .functions import TestGen __all__ = ["profiler_process"] @@ -34,48 +35,51 @@ def encode(self, file_name: str): def profiler_process(transaction_manager: TransactionManager, profile: Profile): - method_map = MethodMap(transaction_manager.transactions) - cgr, _, _ = TransactionManager._conflict_graph(method_map) - - for transaction in method_map.transactions: - profile.transactions_and_methods[id(transaction)] = ProfileInfo( - transaction.owned_name, transaction.src_loc, True - ) - - for method in method_map.methods: - profile.transactions_and_methods[id(method)] = ProfileInfo(method.name, method.src_loc, False) - - cycle = 0 - - yield Passive() - while True: - yield Settle() - - profile.waiting_transactions[cycle] = {} - profile.running[cycle] = {} + def process() -> TestGen: + method_map = MethodMap(transaction_manager.transactions) + cgr, _, _ = TransactionManager._conflict_graph(method_map) for transaction in method_map.transactions: - request = yield transaction.request - runnable = yield transaction.runnable - grant = yield transaction.grant - - if grant: - profile.running[cycle][id(transaction)] = None - elif request and runnable: - for transaction2 in cgr[transaction]: - if (yield transaction2.grant): - profile.waiting_transactions[cycle][id(transaction)] = id(transaction2) + profile.transactions_and_methods[id(transaction)] = ProfileInfo( + transaction.owned_name, transaction.src_loc, True + ) for method in method_map.methods: - if (yield method.run): - for t_or_m in method_map.method_parents[method]: - if ( - isinstance(t_or_m, Transaction) - and (yield t_or_m.grant) - or isinstance(t_or_m, Method) - and (yield t_or_m.run) - ): - profile.running[cycle][id(method)] = id(t_or_m) - - yield - cycle = cycle + 1 + profile.transactions_and_methods[id(method)] = ProfileInfo(method.name, method.src_loc, False) + + cycle = 0 + + yield Passive() + while True: + yield Settle() + + profile.waiting_transactions[cycle] = {} + profile.running[cycle] = {} + + for transaction in method_map.transactions: + request = yield transaction.request + runnable = yield transaction.runnable + grant = yield transaction.grant + + if grant: + profile.running[cycle][id(transaction)] = None + elif request and runnable: + for transaction2 in cgr[transaction]: + if (yield transaction2.grant): + profile.waiting_transactions[cycle][id(transaction)] = id(transaction2) + + for method in method_map.methods: + if (yield method.run): + for t_or_m in method_map.method_parents[method]: + if ( + isinstance(t_or_m, Transaction) + and (yield t_or_m.grant) + or isinstance(t_or_m, Method) + and (yield t_or_m.run) + ): + profile.running[cycle][id(method)] = id(t_or_m) + + yield + cycle = cycle + 1 + + return process From 2bf63f7ad153a37a71c04075da1b5a2dcca7d357 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 15:19:29 +0100 Subject: [PATCH 06/35] Generating profiles --- .gitignore | 1 + scripts/run_tests.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index b545b086c..38daa07cb 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ venv.bak/ # Tests outputs test/__traces__ +test/__profiles__ # cocotb build /test/regression/cocotb/build diff --git a/scripts/run_tests.py b/scripts/run_tests.py index 0f310703a..264daa707 100755 --- a/scripts/run_tests.py +++ b/scripts/run_tests.py @@ -104,6 +104,7 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument("-l", "--list", action="store_true", help="List all tests") parser.add_argument("-t", "--trace", action="store_true", help="Dump waveforms") + parser.add_argument("-p", "--profile", action="store_true", help="Write execution profiles") parser.add_argument("-v", "--verbose", action="store_true", help="Verbose output") parser.add_argument("-a", "--all", action="store_true", default=False, help="Run all tests") parser.add_argument( @@ -127,6 +128,9 @@ def main(): if args.trace: os.environ["__COREBLOCKS_DUMP_TRACES"] = "1" + if args.profile: + os.environ["__TRANSACTRON_PROFILE"] = "1" + if args.test_name: pattern = re.compile(args.test_name) unit_tests = {name: test for name, test in unit_tests.items() if pattern.search(name)} From 110a2e984712f7469746efa3bba6ef29c142f5f4 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 15:28:51 +0100 Subject: [PATCH 07/35] Shorter ids in profiles --- test/common/profiler.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index 2286d655e..11689351e 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -38,14 +38,25 @@ def profiler_process(transaction_manager: TransactionManager, profile: Profile): def process() -> TestGen: method_map = MethodMap(transaction_manager.transactions) cgr, _, _ = TransactionManager._conflict_graph(method_map) + id_map = dict[int, int]() + id_seq = 0 + + def get_id(obj): + try: + return id_map[id(obj)] + except KeyError: + nonlocal id_seq + id_seq = id_seq + 1 + id_map[id(obj)] = id_seq + return id_seq for transaction in method_map.transactions: - profile.transactions_and_methods[id(transaction)] = ProfileInfo( + profile.transactions_and_methods[get_id(transaction)] = ProfileInfo( transaction.owned_name, transaction.src_loc, True ) for method in method_map.methods: - profile.transactions_and_methods[id(method)] = ProfileInfo(method.name, method.src_loc, False) + profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.name, method.src_loc, False) cycle = 0 @@ -62,11 +73,11 @@ def process() -> TestGen: grant = yield transaction.grant if grant: - profile.running[cycle][id(transaction)] = None + profile.running[cycle][get_id(transaction)] = None elif request and runnable: for transaction2 in cgr[transaction]: if (yield transaction2.grant): - profile.waiting_transactions[cycle][id(transaction)] = id(transaction2) + profile.waiting_transactions[cycle][get_id(transaction)] = get_id(transaction2) for method in method_map.methods: if (yield method.run): @@ -77,7 +88,7 @@ def process() -> TestGen: or isinstance(t_or_m, Method) and (yield t_or_m.run) ): - profile.running[cycle][id(method)] = id(t_or_m) + profile.running[cycle][get_id(method)] = get_id(t_or_m) yield cycle = cycle + 1 From 40cc5519a05bd00041de5cb9e73419dd81f4fddf Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 15:43:13 +0100 Subject: [PATCH 08/35] Change one test to get a sensible profile --- test/transactions/test_methods.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/transactions/test_methods.py b/test/transactions/test_methods.py index df05d259f..bcd014a9d 100644 --- a/test/transactions/test_methods.py +++ b/test/transactions/test_methods.py @@ -589,7 +589,6 @@ def process(): yield self.circ.req_t2.eq(req_t2) yield self.circ.ready.eq(m_ready) yield Settle() - yield Delay(1e-8) out_m = yield self.circ.out_m out_t1 = yield self.circ.out_t1 @@ -609,8 +608,10 @@ def process(): self.assertTrue(in1 != self.bad_number or not out_t1) self.assertTrue(in2 != self.bad_number or not out_t2) + yield + with self.run_simulation(self.circ, 100) as sim: - sim.add_process(process) + sim.add_sync_process(process) def test_random_arg(self): self.base_random(lambda arg: arg.data != self.bad_number) From 8bc4946a4ff9f084f75c72bfd8b61c6402b8d51e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 15:50:18 +0100 Subject: [PATCH 09/35] Split out profile dataclasses --- test/common/profiler.py | 30 +----------------------------- transactron/profiler.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 29 deletions(-) create mode 100644 transactron/profiler.py diff --git a/test/common/profiler.py b/test/common/profiler.py index 11689351e..37c4cd172 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -1,39 +1,11 @@ -import json -from typing import Optional -from dataclasses import dataclass, field, is_dataclass, asdict from amaranth.sim import * from transactron.core import MethodMap, TransactionManager, Transaction, Method -from transactron.lib import SrcLoc +from transactron.profiler import Profile, ProfileInfo from .functions import TestGen __all__ = ["profiler_process"] -class ProfileJSONEncoder(json.JSONEncoder): - def default(self, o): - if is_dataclass(o): - return asdict(o) - return super().default(o) - - -@dataclass -class ProfileInfo: - name: str - src_loc: SrcLoc - is_transaction: bool - - -@dataclass -class Profile: - transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) - waiting_transactions: dict[int, dict[int, int]] = field(default_factory=dict) - running: dict[int, dict[int, Optional[int]]] = field(default_factory=dict) - - def encode(self, file_name: str): - with open(file_name, "w") as fp: - json.dump(self, fp, cls=ProfileJSONEncoder) - - def profiler_process(transaction_manager: TransactionManager, profile: Profile): def process() -> TestGen: method_map = MethodMap(transaction_manager.transactions) diff --git a/transactron/profiler.py b/transactron/profiler.py new file mode 100644 index 000000000..7c9a7d20f --- /dev/null +++ b/transactron/profiler.py @@ -0,0 +1,32 @@ +import json +from typing import Optional +from dataclasses import dataclass, is_dataclass, asdict, field +from transactron.utils import SrcLoc + + +__ALL__ = ["ProfileInfo", "Profile"] + + +class ProfileJSONEncoder(json.JSONEncoder): + def default(self, o): + if is_dataclass(o): + return asdict(o) + return super().default(o) + + +@dataclass +class ProfileInfo: + name: str + src_loc: SrcLoc + is_transaction: bool + + +@dataclass +class Profile: + transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) + waiting_transactions: dict[int, dict[int, int]] = field(default_factory=dict) + running: dict[int, dict[int, Optional[int]]] = field(default_factory=dict) + + def encode(self, file_name: str): + with open(file_name, "w") as fp: + json.dump(self, fp, cls=ProfileJSONEncoder) From c098d20b69d985ed75ab0e8c0899946fc1052c5a Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 16:37:10 +0100 Subject: [PATCH 10/35] Use dataclasses_json. Start profiler script --- requirements-dev.txt | 1 + scripts/tprof.py | 26 ++++++++++++++++++++++++++ transactron/profiler.py | 22 +++++++++++----------- 3 files changed, 38 insertions(+), 11 deletions(-) create mode 100755 scripts/tprof.py diff --git a/requirements-dev.txt b/requirements-dev.txt index 35c93666f..7b36bde8d 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -17,3 +17,4 @@ cocotb==1.7.2 cocotb-bus==0.2.1 pytest==7.2.2 pyelftools==0.29 +dataclasses-json==0.6.3 diff --git a/scripts/tprof.py b/scripts/tprof.py new file mode 100755 index 000000000..905631d3e --- /dev/null +++ b/scripts/tprof.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python3 + +import argparse +import sys +from pathlib import Path + +topdir = Path(__file__).parent.parent +sys.path.insert(0, str(topdir)) + + +from transactron.profiler import Profile # noqa: E402 + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("file_name", nargs=1) + + args = parser.parse_args() + + profile = Profile.decode(args.file_name[0]) + + print(profile) + + +if __name__ == "__main__": + main() diff --git a/transactron/profiler.py b/transactron/profiler.py index 7c9a7d20f..de74c5c63 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -1,19 +1,13 @@ -import json from typing import Optional -from dataclasses import dataclass, is_dataclass, asdict, field +from dataclasses import dataclass, field from transactron.utils import SrcLoc +from dataclasses_json import dataclass_json -__ALL__ = ["ProfileInfo", "Profile"] - - -class ProfileJSONEncoder(json.JSONEncoder): - def default(self, o): - if is_dataclass(o): - return asdict(o) - return super().default(o) +__all__ = ["ProfileInfo", "Profile"] +@dataclass_json @dataclass class ProfileInfo: name: str @@ -21,6 +15,7 @@ class ProfileInfo: is_transaction: bool +@dataclass_json @dataclass class Profile: transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) @@ -29,4 +24,9 @@ class Profile: def encode(self, file_name: str): with open(file_name, "w") as fp: - json.dump(self, fp, cls=ProfileJSONEncoder) + fp.write(self.to_json()) # type: ignore + + @staticmethod + def decode(file_name: str): + with open(file_name, "r") as fp: + return Profile.from_json(fp.read()) # type: ignore From 740948f8c0a8199224e1eaa5bc63cf46d81e6c30 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 21 Dec 2023 17:03:00 +0100 Subject: [PATCH 11/35] Basic profile statistics --- requirements-dev.txt | 1 + scripts/tprof.py | 5 ++++- test/common/profiler.py | 4 +++- transactron/profiler.py | 29 ++++++++++++++++++++++++++++- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 7b36bde8d..3e90e45be 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -18,3 +18,4 @@ cocotb-bus==0.2.1 pytest==7.2.2 pyelftools==0.29 dataclasses-json==0.6.3 +tabulate==0.9.0 diff --git a/scripts/tprof.py b/scripts/tprof.py index 905631d3e..d6b969ed4 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -3,6 +3,7 @@ import argparse import sys from pathlib import Path +from tabulate import tabulate topdir = Path(__file__).parent.parent sys.path.insert(0, str(topdir)) @@ -19,7 +20,9 @@ def main(): profile = Profile.decode(args.file_name[0]) - print(profile) + transactions = profile.analyze_transactions() + + print(tabulate(transactions, headers=["name", "source location", "unused", "runnable", "grant"])) if __name__ == "__main__": diff --git a/test/common/profiler.py b/test/common/profiler.py index 37c4cd172..fbde23f9e 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -34,7 +34,9 @@ def get_id(obj): yield Passive() while True: - yield Settle() + # TODO: wait for the end of cycle + for _ in range(3): + yield Settle() profile.waiting_transactions[cycle] = {} profile.running[cycle] = {} diff --git a/transactron/profiler.py b/transactron/profiler.py index de74c5c63..a6a75d89d 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -4,7 +4,7 @@ from dataclasses_json import dataclass_json -__all__ = ["ProfileInfo", "Profile"] +__all__ = ["ProfileInfo", "Profile", "TransactionStat"] @dataclass_json @@ -15,6 +15,15 @@ class ProfileInfo: is_transaction: bool +@dataclass +class TransactionStat: + name: str + src_loc: str + unused: int = 0 + runnable: int = 0 + grant: int = 0 + + @dataclass_json @dataclass class Profile: @@ -30,3 +39,21 @@ def encode(self, file_name: str): def decode(file_name: str): with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore + + def analyze_transactions(self: "Profile") -> list[TransactionStat]: + stats = { + i: TransactionStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") + for i, info in self.transactions_and_methods.items() + if info.is_transaction + } + + for k in self.running.keys(): + for i in stats: + if i in self.waiting_transactions[k]: + stats[i].runnable = stats[i].runnable + 1 + elif i in self.running[k]: + stats[i].grant = stats[i].grant + 1 + else: + stats[i].unused = stats[i].unused + 1 + + return list(stats.values()) From dc58151e31ef825eaa264e691d42d20f5b5cdf91 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Fri, 29 Dec 2023 00:06:11 +0100 Subject: [PATCH 12/35] Start method profiler --- transactron/profiler.py | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/transactron/profiler.py b/transactron/profiler.py index a6a75d89d..3593575b8 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -24,6 +24,19 @@ class TransactionStat: grant: int = 0 +@dataclass +class RunStat: + name: str + src_loc: str + run: int = 0 + + +@dataclass +class RunStatNode: + stat: RunStat + callers: dict[int, "RunStatNode"] = {} + + @dataclass_json @dataclass class Profile: @@ -40,7 +53,7 @@ def decode(file_name: str): with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore - def analyze_transactions(self: "Profile") -> list[TransactionStat]: + def analyze_transactions(self) -> list[TransactionStat]: stats = { i: TransactionStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") for i, info in self.transactions_and_methods.items() @@ -50,10 +63,30 @@ def analyze_transactions(self: "Profile") -> list[TransactionStat]: for k in self.running.keys(): for i in stats: if i in self.waiting_transactions[k]: - stats[i].runnable = stats[i].runnable + 1 + stats[i].runnable += 1 elif i in self.running[k]: - stats[i].grant = stats[i].grant + 1 + stats[i].grant += 1 else: - stats[i].unused = stats[i].unused + 1 + stats[i].unused += 1 return list(stats.values()) + + def analyze_methods(self, recursive=True) -> list[RunStat]: + stats: dict[int, RunStatNode] = { + i: RunStatNode(RunStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}")) + for i, info in self.transactions_and_methods.items() + } + + def rec(k: int, caller: int): + stats[caller].stat.run += 1 + caller1 = self.running[k][caller] + if caller1 is not None: + rec(k, caller1) + + for k, d in self.running.items(): + for i, caller in d.items(): + stats[i].stat.run += 1 + if recursive and caller is not None: + rec(k, caller) + + return list(map(lambda node: node.stat, stats.values())) From b58e2626a0ba3b269472d144a7158053d2c8c2eb Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Sat, 30 Dec 2023 00:52:37 +0100 Subject: [PATCH 13/35] Refactor --- test/common/profiler.py | 15 +++++++-------- transactron/profiler.py | 29 ++++++++++++++++++----------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index fbde23f9e..180c59281 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -1,6 +1,6 @@ from amaranth.sim import * from transactron.core import MethodMap, TransactionManager, Transaction, Method -from transactron.profiler import Profile, ProfileInfo +from transactron.profiler import CycleProfile, Profile, ProfileInfo from .functions import TestGen __all__ = ["profiler_process"] @@ -38,8 +38,7 @@ def get_id(obj): for _ in range(3): yield Settle() - profile.waiting_transactions[cycle] = {} - profile.running[cycle] = {} + cprof = CycleProfile() for transaction in method_map.transactions: request = yield transaction.request @@ -47,22 +46,22 @@ def get_id(obj): grant = yield transaction.grant if grant: - profile.running[cycle][get_id(transaction)] = None + cprof.running[get_id(transaction)] = None elif request and runnable: for transaction2 in cgr[transaction]: if (yield transaction2.grant): - profile.waiting_transactions[cycle][get_id(transaction)] = get_id(transaction2) + cprof.waiting_transactions[get_id(transaction)] = get_id(transaction2) for method in method_map.methods: - if (yield method.run): - for t_or_m in method_map.method_parents[method]: + for t_or_m in method_map.method_parents[method]: + if (yield method.run): if ( isinstance(t_or_m, Transaction) and (yield t_or_m.grant) or isinstance(t_or_m, Method) and (yield t_or_m.run) ): - profile.running[cycle][get_id(method)] = get_id(t_or_m) + cprof.running[get_id(method)] = get_id(t_or_m) yield cycle = cycle + 1 diff --git a/transactron/profiler.py b/transactron/profiler.py index 3593575b8..94e8ed17a 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -37,12 +37,19 @@ class RunStatNode: callers: dict[int, "RunStatNode"] = {} +@dataclass_json +@dataclass +class CycleProfile: + waiting_transactions: dict[int, int] = field(default_factory=dict) + locked_methods: dict[int, int] = field(default_factory=dict) + running: dict[int, Optional[int]] = field(default_factory=dict) + + @dataclass_json @dataclass class Profile: transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) - waiting_transactions: dict[int, dict[int, int]] = field(default_factory=dict) - running: dict[int, dict[int, Optional[int]]] = field(default_factory=dict) + cycles: list[CycleProfile] = field(default_factory=list) def encode(self, file_name: str): with open(file_name, "w") as fp: @@ -60,11 +67,11 @@ def analyze_transactions(self) -> list[TransactionStat]: if info.is_transaction } - for k in self.running.keys(): + for c in self.cycles: for i in stats: - if i in self.waiting_transactions[k]: + if i in c.waiting_transactions: stats[i].runnable += 1 - elif i in self.running[k]: + elif i in c.running: stats[i].grant += 1 else: stats[i].unused += 1 @@ -77,16 +84,16 @@ def analyze_methods(self, recursive=True) -> list[RunStat]: for i, info in self.transactions_and_methods.items() } - def rec(k: int, caller: int): + def rec(c: CycleProfile, caller: int): stats[caller].stat.run += 1 - caller1 = self.running[k][caller] + caller1 = c.running[caller] if caller1 is not None: - rec(k, caller1) + rec(c, caller1) - for k, d in self.running.items(): - for i, caller in d.items(): + for c in self.cycles: + for i, caller in c.running.items(): stats[i].stat.run += 1 if recursive and caller is not None: - rec(k, caller) + rec(c, caller) return list(map(lambda node: node.stat, stats.values())) From 91490c00755e80df40a5dab3c2f0b256d34151be Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Sun, 31 Dec 2023 19:58:19 +0100 Subject: [PATCH 14/35] Towards locked method counter --- test/common/profiler.py | 10 +++++++-- transactron/profiler.py | 47 +++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index 180c59281..28f1f0b90 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -53,8 +53,8 @@ def get_id(obj): cprof.waiting_transactions[get_id(transaction)] = get_id(transaction2) for method in method_map.methods: - for t_or_m in method_map.method_parents[method]: - if (yield method.run): + if (yield method.run): + for t_or_m in method_map.method_parents[method]: if ( isinstance(t_or_m, Transaction) and (yield t_or_m.grant) @@ -62,6 +62,12 @@ def get_id(obj): and (yield t_or_m.run) ): cprof.running[get_id(method)] = get_id(t_or_m) + else: + if any( + get_id(transaction) in cprof.running + for transaction in method_map.transactions_by_method[method] + ): + cprof.locked_methods.add(get_id(method)) yield cycle = cycle + 1 diff --git a/transactron/profiler.py b/transactron/profiler.py index 94e8ed17a..0fc45b339 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -23,25 +23,38 @@ class TransactionStat: runnable: int = 0 grant: int = 0 + @staticmethod + def make(info: ProfileInfo): + return TransactionStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") + @dataclass class RunStat: name: str src_loc: str run: int = 0 + locked: int = 0 + + @staticmethod + def make(info: ProfileInfo): + return RunStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") @dataclass class RunStatNode: stat: RunStat - callers: dict[int, "RunStatNode"] = {} + callers: dict[int, "RunStatNode"] = field(default_factory=dict) + + @staticmethod + def make(info: ProfileInfo): + return RunStatNode(RunStat.make(info)) @dataclass_json @dataclass class CycleProfile: waiting_transactions: dict[int, int] = field(default_factory=dict) - locked_methods: dict[int, int] = field(default_factory=dict) + locked_methods: set[int] = field(default_factory=set) running: dict[int, Optional[int]] = field(default_factory=dict) @@ -62,9 +75,7 @@ def decode(file_name: str): def analyze_transactions(self) -> list[TransactionStat]: stats = { - i: TransactionStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") - for i, info in self.transactions_and_methods.items() - if info.is_transaction + i: TransactionStat.make(info) for i, info in self.transactions_and_methods.items() if info.is_transaction } for c in self.cycles: @@ -79,21 +90,21 @@ def analyze_transactions(self) -> list[TransactionStat]: return list(stats.values()) def analyze_methods(self, recursive=True) -> list[RunStat]: - stats: dict[int, RunStatNode] = { - i: RunStatNode(RunStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}")) - for i, info in self.transactions_and_methods.items() - } + stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} - def rec(c: CycleProfile, caller: int): - stats[caller].stat.run += 1 - caller1 = c.running[caller] - if caller1 is not None: - rec(c, caller1) + def rec(c: CycleProfile, node: RunStatNode, i: int): + node.stat.run += 1 + caller = c.running[i] + if recursive and caller is not None: + if caller not in stats[i].callers: + node.callers[caller] = RunStatNode.make(self.transactions_and_methods[caller]) + rec(c, node.callers[caller], caller) for c in self.cycles: - for i, caller in c.running.items(): - stats[i].stat.run += 1 - if recursive and caller is not None: - rec(c, caller) + for i in c.running: + rec(c, stats[i], i) + + for i in c.locked_methods: + stats[i].stat.locked += 1 return list(map(lambda node: node.stat, stats.values())) From 3538aee38eaab3d65fcc18ad5c05e49a4bf14c11 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 2 Jan 2024 11:01:43 +0100 Subject: [PATCH 15/35] Some changes --- .gitignore | 2 +- scripts/tprof.py | 7 ++++++- test/common/profiler.py | 3 ++- transactron/profiler.py | 5 +---- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 38daa07cb..a27638d1d 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,7 @@ venv.bak/ # Tests outputs test/__traces__ -test/__profiles__ +test/__profiles__/*.json # cocotb build /test/regression/cocotb/build diff --git a/scripts/tprof.py b/scripts/tprof.py index d6b969ed4..ef2188a72 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -22,7 +22,12 @@ def main(): transactions = profile.analyze_transactions() - print(tabulate(transactions, headers=["name", "source location", "unused", "runnable", "grant"])) + print(tabulate(transactions, headers=["name", "source location", "runnable", "grant"])) + print() + + methods = profile.analyze_methods(recursive=False) + + print(tabulate(methods, headers=["name", "source location", "locked", "run"])) if __name__ == "__main__": diff --git a/test/common/profiler.py b/test/common/profiler.py index 28f1f0b90..f812099a0 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -28,7 +28,7 @@ def get_id(obj): ) for method in method_map.methods: - profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.name, method.src_loc, False) + profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.owned_name, method.src_loc, False) cycle = 0 @@ -39,6 +39,7 @@ def get_id(obj): yield Settle() cprof = CycleProfile() + profile.cycles.append(cprof) for transaction in method_map.transactions: request = yield transaction.request diff --git a/transactron/profiler.py b/transactron/profiler.py index 0fc45b339..94465e6a8 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -19,7 +19,6 @@ class ProfileInfo: class TransactionStat: name: str src_loc: str - unused: int = 0 runnable: int = 0 grant: int = 0 @@ -32,8 +31,8 @@ def make(info: ProfileInfo): class RunStat: name: str src_loc: str - run: int = 0 locked: int = 0 + run: int = 0 @staticmethod def make(info: ProfileInfo): @@ -84,8 +83,6 @@ def analyze_transactions(self) -> list[TransactionStat]: stats[i].runnable += 1 elif i in c.running: stats[i].grant += 1 - else: - stats[i].unused += 1 return list(stats.values()) From 285fe11f73dffa08bd0f03675bd4e3323003e6b3 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 2 Jan 2024 11:08:39 +0100 Subject: [PATCH 16/35] Shorter locations --- test/common/profiler.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index f812099a0..31cbd2097 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -1,6 +1,8 @@ +import os.path from amaranth.sim import * from transactron.core import MethodMap, TransactionManager, Transaction, Method from transactron.profiler import CycleProfile, Profile, ProfileInfo +from transactron.utils import SrcLoc from .functions import TestGen __all__ = ["profiler_process"] @@ -22,13 +24,16 @@ def get_id(obj): id_map[id(obj)] = id_seq return id_seq + def local_src_loc(src_loc: SrcLoc): + return (os.path.relpath(src_loc[0]), src_loc[1]) + for transaction in method_map.transactions: profile.transactions_and_methods[get_id(transaction)] = ProfileInfo( - transaction.owned_name, transaction.src_loc, True + transaction.owned_name, local_src_loc(transaction.src_loc), True ) for method in method_map.methods: - profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.owned_name, method.src_loc, False) + profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.owned_name, local_src_loc(method.src_loc), False) cycle = 0 From 873ceae6baf34430ac5659b870015b28b57f17aa Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 2 Jan 2024 12:07:38 +0100 Subject: [PATCH 17/35] Unify transaction and method stats --- scripts/tprof.py | 5 ----- test/common/profiler.py | 29 ++++++++++++++++------------- transactron/profiler.py | 31 ++----------------------------- 3 files changed, 18 insertions(+), 47 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index ef2188a72..8b3dc9375 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -20,11 +20,6 @@ def main(): profile = Profile.decode(args.file_name[0]) - transactions = profile.analyze_transactions() - - print(tabulate(transactions, headers=["name", "source location", "runnable", "grant"])) - print() - methods = profile.analyze_methods(recursive=False) print(tabulate(methods, headers=["name", "source location", "locked", "run"])) diff --git a/test/common/profiler.py b/test/common/profiler.py index 31cbd2097..9e5f8a1c9 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -56,24 +56,27 @@ def local_src_loc(src_loc: SrcLoc): elif request and runnable: for transaction2 in cgr[transaction]: if (yield transaction2.grant): - cprof.waiting_transactions[get_id(transaction)] = get_id(transaction2) + cprof.locked[get_id(transaction)] = get_id(transaction2) + running = set(cprof.running) for method in method_map.methods: if (yield method.run): + running.add(get_id(method)) + + locked_methods = set[int]() + for method in method_map.methods: + if get_id(method) not in running: + if any(get_id(transaction) in running for transaction in method_map.transactions_by_method[method]): + locked_methods.add(get_id(method)) + + for method in method_map.methods: + if get_id(method) in running: for t_or_m in method_map.method_parents[method]: - if ( - isinstance(t_or_m, Transaction) - and (yield t_or_m.grant) - or isinstance(t_or_m, Method) - and (yield t_or_m.run) - ): + if get_id(t_or_m) in running: cprof.running[get_id(method)] = get_id(t_or_m) - else: - if any( - get_id(transaction) in cprof.running - for transaction in method_map.transactions_by_method[method] - ): - cprof.locked_methods.add(get_id(method)) + elif get_id(method) in locked_methods: + caller = next(get_id(t_or_m) for t_or_m in method_map.method_parents[method] if get_id(t_or_m) in running or get_id(t_or_m) in locked_methods) + cprof.locked[get_id(method)] = caller yield cycle = cycle + 1 diff --git a/transactron/profiler.py b/transactron/profiler.py index 94465e6a8..356e2f90a 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -15,18 +15,6 @@ class ProfileInfo: is_transaction: bool -@dataclass -class TransactionStat: - name: str - src_loc: str - runnable: int = 0 - grant: int = 0 - - @staticmethod - def make(info: ProfileInfo): - return TransactionStat(info.name, f"{info.src_loc[0]}:{info.src_loc[1]}") - - @dataclass class RunStat: name: str @@ -52,8 +40,7 @@ def make(info: ProfileInfo): @dataclass_json @dataclass class CycleProfile: - waiting_transactions: dict[int, int] = field(default_factory=dict) - locked_methods: set[int] = field(default_factory=set) + locked: dict[int, int] = field(default_factory=dict) running: dict[int, Optional[int]] = field(default_factory=dict) @@ -72,20 +59,6 @@ def decode(file_name: str): with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore - def analyze_transactions(self) -> list[TransactionStat]: - stats = { - i: TransactionStat.make(info) for i, info in self.transactions_and_methods.items() if info.is_transaction - } - - for c in self.cycles: - for i in stats: - if i in c.waiting_transactions: - stats[i].runnable += 1 - elif i in c.running: - stats[i].grant += 1 - - return list(stats.values()) - def analyze_methods(self, recursive=True) -> list[RunStat]: stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} @@ -101,7 +74,7 @@ def rec(c: CycleProfile, node: RunStatNode, i: int): for i in c.running: rec(c, stats[i], i) - for i in c.locked_methods: + for i in c.locked: stats[i].stat.locked += 1 return list(map(lambda node: node.stat, stats.values())) From 23431d01a9c1f04756fa894ca2236ae507cf3538 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 2 Jan 2024 12:38:57 +0100 Subject: [PATCH 18/35] Some documentation --- transactron/profiler.py | 67 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/transactron/profiler.py b/transactron/profiler.py index 356e2f90a..785dcd063 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -4,12 +4,24 @@ from dataclasses_json import dataclass_json -__all__ = ["ProfileInfo", "Profile", "TransactionStat"] +__all__ = ["ProfileInfo", "Profile"] @dataclass_json @dataclass class ProfileInfo: + """Information about transactions and methods. In `Profile`, transactions + and methods are referred to by their unique ID numbers. + + Attributes + ---------- + name : str + The name. + src_loc : SrcLoc + Source location. + is_transaction : bool + If true, this object describes a transaction; if false, a method. + """ name: str src_loc: SrcLoc is_transaction: bool @@ -17,6 +29,20 @@ class ProfileInfo: @dataclass class RunStat: + """Collected statistics about a transaction or method. + + Attributes + ---------- + name : str + The name. + src_loc : SrcLoc + Source location. + locked : int + For methods: the number of cycles this method was locked because of + a disabled call (a call under a false condition). For transactions: + the number of cycles this transaction was ready to run, but did not + run because a conflicting transaction has run instead. + """ name: str src_loc: str locked: int = 0 @@ -29,6 +55,15 @@ def make(info: ProfileInfo): @dataclass class RunStatNode: + """A statistics tree. Summarizes call graph information. + + Attributes + ---------- + stat : RunStat + Statistics. + callers : dict[int, RunStatNode] + Statistics for the method callers. For transactions, this is empty. + """ stat: RunStat callers: dict[int, "RunStatNode"] = field(default_factory=dict) @@ -40,6 +75,21 @@ def make(info: ProfileInfo): @dataclass_json @dataclass class CycleProfile: + """Profile information for a single clock cycle. + + Transactions and methods are referred to by unique IDs. + + Attributes + ---------- + locked : dict[int, int] + For each transaction which didn't run because of a conflict, the + transaction which has run instead. For each method which was used + but didn't run because of a disabled call, the caller which + used it. + running : dict[int, Optional[int]] + For each running method, its caller. Running transactions don't + have a caller (the value is `None`). + """ locked: dict[int, int] = field(default_factory=dict) running: dict[int, Optional[int]] = field(default_factory=dict) @@ -47,6 +97,19 @@ class CycleProfile: @dataclass_json @dataclass class Profile: + """Transactron execution profile. + + Can be saved by the simulator, and then restored by an analysis tool. + In the profile data structure, methods and transactions are referred to + by their unique ID numbers. + + Attributes + ---------- + transactions_and_methods : dict[int, ProfileInfo] + Information about transactions and methods. + cycles : list[CycleProfile] + Profile information for each cycle of the simulation. + """ transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) cycles: list[CycleProfile] = field(default_factory=list) @@ -59,7 +122,7 @@ def decode(file_name: str): with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore - def analyze_methods(self, recursive=True) -> list[RunStat]: + def analyze_methods(self, recursive=False) -> list[RunStat]: stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} def rec(c: CycleProfile, node: RunStatNode, i: int): From cc68f4dbc5f992a3b3811aa108349a2a25d1dc1e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 2 Jan 2024 12:40:05 +0100 Subject: [PATCH 19/35] Lint --- test/common/profiler.py | 12 +++++++++--- transactron/profiler.py | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index 9e5f8a1c9..62356e004 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -1,6 +1,6 @@ import os.path from amaranth.sim import * -from transactron.core import MethodMap, TransactionManager, Transaction, Method +from transactron.core import MethodMap, TransactionManager from transactron.profiler import CycleProfile, Profile, ProfileInfo from transactron.utils import SrcLoc from .functions import TestGen @@ -33,7 +33,9 @@ def local_src_loc(src_loc: SrcLoc): ) for method in method_map.methods: - profile.transactions_and_methods[get_id(method)] = ProfileInfo(method.owned_name, local_src_loc(method.src_loc), False) + profile.transactions_and_methods[get_id(method)] = ProfileInfo( + method.owned_name, local_src_loc(method.src_loc), False + ) cycle = 0 @@ -75,7 +77,11 @@ def local_src_loc(src_loc: SrcLoc): if get_id(t_or_m) in running: cprof.running[get_id(method)] = get_id(t_or_m) elif get_id(method) in locked_methods: - caller = next(get_id(t_or_m) for t_or_m in method_map.method_parents[method] if get_id(t_or_m) in running or get_id(t_or_m) in locked_methods) + caller = next( + get_id(t_or_m) + for t_or_m in method_map.method_parents[method] + if get_id(t_or_m) in running or get_id(t_or_m) in locked_methods + ) cprof.locked[get_id(method)] = caller yield diff --git a/transactron/profiler.py b/transactron/profiler.py index 785dcd063..b74c1786c 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -22,6 +22,7 @@ class ProfileInfo: is_transaction : bool If true, this object describes a transaction; if false, a method. """ + name: str src_loc: SrcLoc is_transaction: bool @@ -43,6 +44,7 @@ class RunStat: the number of cycles this transaction was ready to run, but did not run because a conflicting transaction has run instead. """ + name: str src_loc: str locked: int = 0 @@ -64,6 +66,7 @@ class RunStatNode: callers : dict[int, RunStatNode] Statistics for the method callers. For transactions, this is empty. """ + stat: RunStat callers: dict[int, "RunStatNode"] = field(default_factory=dict) @@ -90,6 +93,7 @@ class CycleProfile: For each running method, its caller. Running transactions don't have a caller (the value is `None`). """ + locked: dict[int, int] = field(default_factory=dict) running: dict[int, Optional[int]] = field(default_factory=dict) @@ -110,6 +114,7 @@ class Profile: cycles : list[CycleProfile] Profile information for each cycle of the simulation. """ + transactions_and_methods: dict[int, ProfileInfo] = field(default_factory=dict) cycles: list[CycleProfile] = field(default_factory=list) From b0f36887c6473452421920d2fda924bbccdb9490 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 4 Jan 2024 17:21:22 +0100 Subject: [PATCH 20/35] Recursive statistics --- scripts/tprof.py | 30 +++++++++++++++++++++++++++--- transactron/profiler.py | 6 +++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index 8b3dc9375..1955471bf 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -3,13 +3,31 @@ import argparse import sys from pathlib import Path +from typing import Optional +from collections.abc import Iterable from tabulate import tabulate +from dataclasses import astuple topdir = Path(__file__).parent.parent sys.path.insert(0, str(topdir)) -from transactron.profiler import Profile # noqa: E402 +from transactron.profiler import Profile, RunStatNode # noqa: E402 + + +def process_stat_tree( + xs: Iterable[RunStatNode], recursive: bool, ret: Optional[list[tuple]] = None, depth=0 +) -> list[tuple]: + if ret is None: + ret = list[tuple]() + for x in xs: + row = astuple(x.stat) + if recursive: + row = (depth * "-",) + row + ret.append(row) + if recursive and x.callers: + process_stat_tree(x.callers.values(), recursive, ret, depth + 1) + return ret def main(): @@ -20,9 +38,15 @@ def main(): profile = Profile.decode(args.file_name[0]) - methods = profile.analyze_methods(recursive=False) + recursive = True + + methods = profile.analyze_methods(recursive=recursive) + + headers = ["name", "source location", "locked", "run"] + if recursive: + headers = [""] + headers - print(tabulate(methods, headers=["name", "source location", "locked", "run"])) + print(tabulate(process_stat_tree(methods, recursive), headers=headers)) if __name__ == "__main__": diff --git a/transactron/profiler.py b/transactron/profiler.py index b74c1786c..2c8190c74 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -127,14 +127,14 @@ def decode(file_name: str): with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore - def analyze_methods(self, recursive=False) -> list[RunStat]: + def analyze_methods(self, recursive=False) -> list[RunStatNode]: stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} def rec(c: CycleProfile, node: RunStatNode, i: int): node.stat.run += 1 caller = c.running[i] if recursive and caller is not None: - if caller not in stats[i].callers: + if caller not in node.callers: node.callers[caller] = RunStatNode.make(self.transactions_and_methods[caller]) rec(c, node.callers[caller], caller) @@ -145,4 +145,4 @@ def rec(c: CycleProfile, node: RunStatNode, i: int): for i in c.locked: stats[i].stat.locked += 1 - return list(map(lambda node: node.stat, stats.values())) + return list(stats.values()) From 9ae895ace74e98dfd379233d8953ae056d0f9a64 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 4 Jan 2024 17:41:30 +0100 Subject: [PATCH 21/35] Recursive locking calculation --- transactron/profiler.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/transactron/profiler.py b/transactron/profiler.py index 2c8190c74..2baca4fe6 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -130,19 +130,26 @@ def decode(file_name: str): def analyze_methods(self, recursive=False) -> list[RunStatNode]: stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} - def rec(c: CycleProfile, node: RunStatNode, i: int): - node.stat.run += 1 - caller = c.running[i] + def rec(c: CycleProfile, node: RunStatNode, i: int, locking_call = False): + if i in c.running: + if not locking_call: + node.stat.run += 1 + else: + node.stat.locked += 1 + caller = c.running[i] + else: + node.stat.locked += 1 + caller = c.locked[i] if recursive and caller is not None: if caller not in node.callers: node.callers[caller] = RunStatNode.make(self.transactions_and_methods[caller]) - rec(c, node.callers[caller], caller) + rec(c, node.callers[caller], caller, locking_call) for c in self.cycles: for i in c.running: rec(c, stats[i], i) for i in c.locked: - stats[i].stat.locked += 1 + rec(c, stats[i], i, locking_call=True) return list(stats.values()) From 90458a447225a0a4622328d0bfa1930cc258ae71 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 4 Jan 2024 21:56:39 +0100 Subject: [PATCH 22/35] Lint --- transactron/profiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transactron/profiler.py b/transactron/profiler.py index 2baca4fe6..8002733cb 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -130,7 +130,7 @@ def decode(file_name: str): def analyze_methods(self, recursive=False) -> list[RunStatNode]: stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} - def rec(c: CycleProfile, node: RunStatNode, i: int, locking_call = False): + def rec(c: CycleProfile, node: RunStatNode, i: int, locking_call=False): if i in c.running: if not locking_call: node.stat.run += 1 From 41be0e287289579f7c2bf600dde36984e1c6438e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:13:15 +0100 Subject: [PATCH 23/35] Make dir, replace Settle with Delay --- test/common/infrastructure.py | 1 + test/common/profiler.py | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/common/infrastructure.py b/test/common/infrastructure.py index c0eb87bc3..ba86eb749 100644 --- a/test/common/infrastructure.py +++ b/test/common/infrastructure.py @@ -203,6 +203,7 @@ def run_simulation(self, module: HasElaborate, max_cycles: float = 10e4, add_tra if profile is not None: profile_dir = "test/__profiles__" profile_file = unittest.TestCase.id(self) + os.makedirs(profile_dir, exist_ok=True) profile.encode(f"{profile_dir}/{profile_file}.json") self.assertTrue(res, "Simulation time limit exceeded") diff --git a/test/common/profiler.py b/test/common/profiler.py index 62356e004..b86126ac4 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -41,9 +41,7 @@ def local_src_loc(src_loc: SrcLoc): yield Passive() while True: - # TODO: wait for the end of cycle - for _ in range(3): - yield Settle() + yield Delay(5e-7) # shorter than one clock cycle cprof = CycleProfile() profile.cycles.append(cprof) From a52c43b80d7dc1d2e931532ce8e148e74b90aa72 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:35:06 +0100 Subject: [PATCH 24/35] Filtering and sorting --- scripts/tprof.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index 1955471bf..780ee3000 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -2,11 +2,12 @@ import argparse import sys +import re from pathlib import Path from typing import Optional from collections.abc import Iterable from tabulate import tabulate -from dataclasses import astuple +from dataclasses import astuple, asdict topdir = Path(__file__).parent.parent sys.path.insert(0, str(topdir)) @@ -32,13 +33,16 @@ def process_stat_tree( def main(): parser = argparse.ArgumentParser() + parser.add_argument("-g", "--call-graph", action="store_true", help="Show call graph") + parser.add_argument("-s", "--sort", choices=["name", "locked", "run"], default="name", help="Sort by column") + parser.add_argument("-f", "--filter", help="Filter by name, regular expressions can be used") parser.add_argument("file_name", nargs=1) args = parser.parse_args() - profile = Profile.decode(args.file_name[0]) + profile: Profile = Profile.decode(args.file_name[0]) - recursive = True + recursive = args.call_graph methods = profile.analyze_methods(recursive=recursive) @@ -46,6 +50,12 @@ def main(): if recursive: headers = [""] + headers + methods.sort(key=lambda node: asdict(node.stat)[args.sort]) + + if args.filter: + pattern = re.compile(args.filter) + methods = [node for node in methods if pattern.search(node.stat.name)] + print(tabulate(process_stat_tree(methods, recursive), headers=headers)) From 56ab29f14474cf8382f3c0c22ffaaffc3dc132a1 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:39:32 +0100 Subject: [PATCH 25/35] Indented names in call graph --- scripts/tprof.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index 780ee3000..ff1372758 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -22,10 +22,10 @@ def process_stat_tree( if ret is None: ret = list[tuple]() for x in xs: - row = astuple(x.stat) - if recursive: - row = (depth * "-",) + row - ret.append(row) + row = asdict(x.stat) + if recursive and depth: + row["name"] = (2*depth-1) * "-" + " " + row["name"] + ret.append(tuple(row.values())) if recursive and x.callers: process_stat_tree(x.callers.values(), recursive, ret, depth + 1) return ret @@ -47,8 +47,6 @@ def main(): methods = profile.analyze_methods(recursive=recursive) headers = ["name", "source location", "locked", "run"] - if recursive: - headers = [""] + headers methods.sort(key=lambda node: asdict(node.stat)[args.sort]) From 1052b52e540794eafc072f6ab8ecec7f11d27c12 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:48:54 +0100 Subject: [PATCH 26/35] Filtering by source location --- scripts/tprof.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index ff1372758..43eefe1b8 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -5,15 +5,15 @@ import re from pathlib import Path from typing import Optional -from collections.abc import Iterable +from collections.abc import Callable, Iterable from tabulate import tabulate -from dataclasses import astuple, asdict +from dataclasses import asdict topdir = Path(__file__).parent.parent sys.path.insert(0, str(topdir)) -from transactron.profiler import Profile, RunStatNode # noqa: E402 +from transactron.profiler import Profile, RunStat, RunStatNode # noqa: E402 def process_stat_tree( @@ -31,11 +31,17 @@ def process_stat_tree( return ret +def filter_nodes(nodes: list[RunStatNode], key: Callable[[RunStat], str], regex: str): + pattern = re.compile(regex) + return [node for node in nodes if pattern.search(key(node.stat))] + + def main(): parser = argparse.ArgumentParser() parser.add_argument("-g", "--call-graph", action="store_true", help="Show call graph") parser.add_argument("-s", "--sort", choices=["name", "locked", "run"], default="name", help="Sort by column") - parser.add_argument("-f", "--filter", help="Filter by name, regular expressions can be used") + parser.add_argument("-f", "--filter-name", help="Filter by name, regular expressions can be used") + parser.add_argument("-l", "--filter-loc", help="Filter by source location, regular expressions can be used") parser.add_argument("file_name", nargs=1) args = parser.parse_args() @@ -44,17 +50,18 @@ def main(): recursive = args.call_graph - methods = profile.analyze_methods(recursive=recursive) + nodes = profile.analyze_methods(recursive=recursive) headers = ["name", "source location", "locked", "run"] - methods.sort(key=lambda node: asdict(node.stat)[args.sort]) + nodes.sort(key=lambda node: asdict(node.stat)[args.sort]) - if args.filter: - pattern = re.compile(args.filter) - methods = [node for node in methods if pattern.search(node.stat.name)] + if args.filter_name: + nodes = filter_nodes(nodes, lambda stat: stat.name, args.filter_name) + if args.filter_loc: + nodes = filter_nodes(nodes, lambda stat: stat.src_loc, args.filter_loc) - print(tabulate(process_stat_tree(methods, recursive), headers=headers)) + print(tabulate(process_stat_tree(nodes, recursive), headers=headers)) if __name__ == "__main__": From b154c4d0e3e6d149fda3fb6aa759209cd7293bac Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:49:47 +0100 Subject: [PATCH 27/35] Lint --- scripts/tprof.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index 43eefe1b8..ed0717059 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -24,7 +24,7 @@ def process_stat_tree( for x in xs: row = asdict(x.stat) if recursive and depth: - row["name"] = (2*depth-1) * "-" + " " + row["name"] + row["name"] = (2 * depth - 1) * "-" + " " + row["name"] ret.append(tuple(row.values())) if recursive and x.callers: process_stat_tree(x.callers.values(), recursive, ret, depth + 1) From db4cad904f72d195f893eda94ca81f9f241c3095 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 11:51:19 +0100 Subject: [PATCH 28/35] Run traces in CI --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4227aae64..f450a431b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -246,8 +246,8 @@ jobs: - name: Run tests run: ./scripts/run_tests.py --verbose - - name: Check traces - run: ./scripts/run_tests.py -t -c 1 TestCore + - name: Check traces and profiles + run: ./scripts/run_tests.py -t -p -c 1 TestCore lint: name: Check code formatting and typing From 4ed7b2eb9a8b5b3307611294b3bf5b51dfc0eb2a Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Mon, 8 Jan 2024 12:01:21 +0100 Subject: [PATCH 29/35] Review suggestion --- scripts/tprof.py | 2 +- transactron/profiler.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index ed0717059..93b510a56 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -46,7 +46,7 @@ def main(): args = parser.parse_args() - profile: Profile = Profile.decode(args.file_name[0]) + profile = Profile.decode(args.file_name[0]) recursive = args.call_graph diff --git a/transactron/profiler.py b/transactron/profiler.py index 8002733cb..011ec93d2 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -123,7 +123,7 @@ def encode(self, file_name: str): fp.write(self.to_json()) # type: ignore @staticmethod - def decode(file_name: str): + def decode(file_name: str) -> "Profile": with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore From e0e4a38b2b52ad668cf24932112587808ef013bb Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 9 Jan 2024 20:24:36 +0100 Subject: [PATCH 30/35] Transaction profile --- scripts/tprof.py | 10 ++++++++- transactron/profiler.py | 45 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/scripts/tprof.py b/scripts/tprof.py index 93b510a56..9575e86e5 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -40,6 +40,9 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument("-g", "--call-graph", action="store_true", help="Show call graph") parser.add_argument("-s", "--sort", choices=["name", "locked", "run"], default="name", help="Sort by column") + parser.add_argument( + "-m", "--mode", choices=["transactions", "methods"], default="transactions", help="Profile display mode" + ) parser.add_argument("-f", "--filter-name", help="Filter by name, regular expressions can be used") parser.add_argument("-l", "--filter-loc", help="Filter by source location, regular expressions can be used") parser.add_argument("file_name", nargs=1) @@ -50,7 +53,12 @@ def main(): recursive = args.call_graph - nodes = profile.analyze_methods(recursive=recursive) + if args.mode == "transactions": + nodes = profile.analyze_transactions(recursive=recursive) + elif args.mode == "methods": + nodes = profile.analyze_methods(recursive=recursive) + else: + assert False headers = ["name", "source location", "locked", "run"] diff --git a/transactron/profiler.py b/transactron/profiler.py index 011ec93d2..89086ceba 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -1,3 +1,4 @@ +from collections import defaultdict from typing import Optional from dataclasses import dataclass, field from transactron.utils import SrcLoc @@ -127,8 +128,44 @@ def decode(file_name: str) -> "Profile": with open(file_name, "r") as fp: return Profile.from_json(fp.read()) # type: ignore + def analyze_transactions(self, recursive=False) -> list[RunStatNode]: + stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items() if info.is_transaction} + + def rec(c: CycleProfile, node: RunStatNode, i: int): + if i in c.running: + node.stat.run += 1 + elif i in c.locked: + node.stat.locked += 1 + if recursive: + for j in called[i]: + if j not in node.callers: + node.callers[j] = RunStatNode.make(self.transactions_and_methods[j]) + rec(c, node.callers[j], j) + + for c in self.cycles: + called = defaultdict[int, set[int]](set) + + for i, j in c.running.items(): + if j is not None: + called[j].add(i) + + for i, j in c.locked.items(): + called[j].add(i) + + for i in c.running: + if i in stats: + rec(c, stats[i], i) + + for i in c.locked: + if i in stats: + stats[i].stat.locked += 1 + + return list(stats.values()) + def analyze_methods(self, recursive=False) -> list[RunStatNode]: - stats = {i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items()} + stats = { + i: RunStatNode.make(info) for i, info in self.transactions_and_methods.items() if not info.is_transaction + } def rec(c: CycleProfile, node: RunStatNode, i: int, locking_call=False): if i in c.running: @@ -147,9 +184,11 @@ def rec(c: CycleProfile, node: RunStatNode, i: int, locking_call=False): for c in self.cycles: for i in c.running: - rec(c, stats[i], i) + if i in stats: + rec(c, stats[i], i) for i in c.locked: - rec(c, stats[i], i, locking_call=True) + if i in stats: + rec(c, stats[i], i, locking_call=True) return list(stats.values()) From d914b66fea8769fbee1fae600e0d3b52eba24f7e Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Tue, 9 Jan 2024 20:30:40 +0100 Subject: [PATCH 31/35] Recursive sorting of trees --- scripts/tprof.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/tprof.py b/scripts/tprof.py index 9575e86e5..e5209299d 100755 --- a/scripts/tprof.py +++ b/scripts/tprof.py @@ -36,6 +36,12 @@ def filter_nodes(nodes: list[RunStatNode], key: Callable[[RunStat], str], regex: return [node for node in nodes if pattern.search(key(node.stat))] +def sort_node(node: RunStatNode, sort_order: str): + node.callers = dict(sorted(node.callers.items(), key=lambda node: asdict(node[1].stat)[sort_order])) + for node2 in node.callers.values(): + sort_node(node2, sort_order) + + def main(): parser = argparse.ArgumentParser() parser.add_argument("-g", "--call-graph", action="store_true", help="Show call graph") @@ -63,6 +69,8 @@ def main(): headers = ["name", "source location", "locked", "run"] nodes.sort(key=lambda node: asdict(node.stat)[args.sort]) + for node in nodes: + sort_node(node, args.sort) if args.filter_name: nodes = filter_nodes(nodes, lambda stat: stat.name, args.filter_name) From 377b2b7e9e3dca3d3ae554422a4fd46c7700f0d7 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Wed, 10 Jan 2024 15:50:24 +0100 Subject: [PATCH 32/35] Extend dev env documentation --- docs/development-environment.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/docs/development-environment.md b/docs/development-environment.md index cfdb58cad..610fe0610 100644 --- a/docs/development-environment.md +++ b/docs/development-environment.md @@ -40,6 +40,7 @@ The `run_tests.py` script has the following options: * `-l`, `--list` -- lists available tests. This option is helpful, e.g., to find a name of a test generated using the `parameterized` package. * `-t`, `--trace` -- generates waveforms in the `vcd` format and `gtkw` files for the `gtkwave` tool. The files are saved in the `test/__traces__/` directory. Useful for debugging and test-driven development. +* `-p`, `--profile` -- generates Transactron execution profile information, which can then be read by the script `tprof.py`. The files are saved in the `test/__profile__/` directory. Useful for analyzing performance. * `-v`, `--verbose` -- makes the test runner more verbose. It will, for example, print the names of all the tests being run. ### lint.sh @@ -76,3 +77,35 @@ The `core_graph.py` script has the following options: ### build\_docs.sh Generates local documentation using [Sphinx](https://www.sphinx-doc.org/). The generated HTML files are located in `build/html`. + +### tprof.py + +Processes Transactron profile files and presents them in a readable way. +To generate a profile file, the `run_tests.py` script should be used with the `--profile` option. +The `tprof.py` can then be run as follows: + +``` +scripts/tprof.py test/__profile__/profile_file.json +``` + +This displays the profile information about transactions by default. +For method profiles, one should use the `--mode=methods` option. + +The columns have the following meaning: + +* `name` -- the name of the transaction or method in question. The method names are displayed together with the containing module name to differentiate between identically named methods in different modules. +* `source location` -- the file and line where the transaction or method was declared. Used to further disambiguate transaction/methods. +* `locked` -- for methods, shows the number of cycles the method was locked by the caller (called with a false condition). For transactions, shows the number of cycles the transaction could run, but was forced to wait by another, conflicting, transaction. +* `run` -- shows the number of cycles the given method/transaction was running. + +To display information about method calls, one can use the `--call-graph` option. +When displaying transaction profiles, this option produces a call graph. For each transaction, there is a tree of methods which are called by this transaction. +Counters presented in the tree shows information about the calls from the transaction in the root of the tree: if a method is also called by a different transaction, these calls are not counted. +When displaying method profiles, an inverted call graph is produced: the transactions are in the leaves, and the children nodes are the callers of the method in question. +In this mode, the `locked` field in the tree shows how many cycles a given method or transaction was responsible for locking the method in the root. + +Other options of `tprof.py` are: + +* `--sort` -- selects which column is used for sorting rows. +* `--filter-name` -- filters rows by name. Regular expressions can be used. +* `--filter-loc` -- filters rows by source locations. Regular expressions can be used. From 753ec789b700bfd25fefb16a25e81d64f9cc6008 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Fri, 12 Jan 2024 17:29:12 +0100 Subject: [PATCH 33/35] Clock period dependency in profiler --- test/common/infrastructure.py | 19 +++++++++++++++---- test/common/profiler.py | 4 ++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/test/common/infrastructure.py b/test/common/infrastructure.py index 4358e27f6..bf9956c7b 100644 --- a/test/common/infrastructure.py +++ b/test/common/infrastructure.py @@ -137,12 +137,18 @@ def _wrapping_function(self): class PysimSimulator(Simulator): - def __init__(self, module: HasElaborate, max_cycles: float = 10e4, add_transaction_module=True, traces_file=None): + def __init__( + self, + module: HasElaborate, + max_cycles: float = 10e4, + add_transaction_module=True, + traces_file=None, + clk_period=1e-6, + ): test_module = _TestModule(module, add_transaction_module) self.tested_module = tested_module = test_module.tested_module super().__init__(test_module) - clk_period = 1e-6 self.add_clock(clk_period) if isinstance(tested_module, HasDebugSignals): @@ -188,15 +194,20 @@ def run_simulation(self, module: HasElaborate, max_cycles: float = 10e4, add_tra if "__COREBLOCKS_DUMP_TRACES" in os.environ: traces_file = unittest.TestCase.id(self) + clk_period = 1e-6 sim = PysimSimulator( - module, max_cycles=max_cycles, add_transaction_module=add_transaction_module, traces_file=traces_file + module, + max_cycles=max_cycles, + add_transaction_module=add_transaction_module, + traces_file=traces_file, + clk_period=clk_period, ) yield sim profile = None if "__TRANSACTRON_PROFILE" in os.environ and isinstance(sim.tested_module, TransactionModule): profile = Profile() - sim.add_sync_process(profiler_process(sim.tested_module.transactionManager, profile)) + sim.add_sync_process(profiler_process(sim.tested_module.transactionManager, profile, clk_period)) res = sim.run() diff --git a/test/common/profiler.py b/test/common/profiler.py index b86126ac4..721dc08fc 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -8,7 +8,7 @@ __all__ = ["profiler_process"] -def profiler_process(transaction_manager: TransactionManager, profile: Profile): +def profiler_process(transaction_manager: TransactionManager, profile: Profile, clk_period: float): def process() -> TestGen: method_map = MethodMap(transaction_manager.transactions) cgr, _, _ = TransactionManager._conflict_graph(method_map) @@ -41,7 +41,7 @@ def local_src_loc(src_loc: SrcLoc): yield Passive() while True: - yield Delay(5e-7) # shorter than one clock cycle + yield Delay((1 - 1e-4) * clk_period) # shorter than one clock cycle cprof = CycleProfile() profile.cycles.append(cprof) From 52f9a724d58ff033a0b6842ce02527b00c45812f Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Fri, 12 Jan 2024 17:30:22 +0100 Subject: [PATCH 34/35] Extend doc --- transactron/profiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transactron/profiler.py b/transactron/profiler.py index 89086ceba..410538ac4 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -111,7 +111,7 @@ class Profile: Attributes ---------- transactions_and_methods : dict[int, ProfileInfo] - Information about transactions and methods. + Information about transactions and methods indexed by ID numbers. cycles : list[CycleProfile] Profile information for each cycle of the simulation. """ From 88aaf32c6c3693d5b13e88853f52debc364141aa Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 25 Jan 2024 12:16:02 +0100 Subject: [PATCH 35/35] Remove cycles variable --- test/common/profiler.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/common/profiler.py b/test/common/profiler.py index 721dc08fc..58c236153 100644 --- a/test/common/profiler.py +++ b/test/common/profiler.py @@ -37,8 +37,6 @@ def local_src_loc(src_loc: SrcLoc): method.owned_name, local_src_loc(method.src_loc), False ) - cycle = 0 - yield Passive() while True: yield Delay((1 - 1e-4) * clk_period) # shorter than one clock cycle @@ -83,6 +81,5 @@ def local_src_loc(src_loc: SrcLoc): cprof.locked[get_id(method)] = caller yield - cycle = cycle + 1 return process