From 0ae1ed8902d5b0749651c28b3e274a79b16a8096 Mon Sep 17 00:00:00 2001 From: Marek Materzok Date: Thu, 7 Mar 2024 14:47:40 +0100 Subject: [PATCH] Profiles for regression tests (#596) --- scripts/run_benchmarks.py | 4 + test/regression/benchmark.py | 5 + test/regression/cocotb.py | 48 +++++++- test/regression/common.py | 3 + test/regression/conftest.py | 1 + test/regression/pysim.py | 13 ++- test/regression/test_regression.py | 6 +- transactron/profiler.py | 169 ++++++++++++++++++++++++++++- transactron/testing/profiler.py | 73 ++----------- transactron/utils/__init__.py | 1 + transactron/utils/gen.py | 84 +++++++++++++- transactron/utils/idgen.py | 15 +++ 12 files changed, 346 insertions(+), 76 deletions(-) create mode 100644 transactron/utils/idgen.py diff --git a/scripts/run_benchmarks.py b/scripts/run_benchmarks.py index 843b64f53..0dc25098a 100755 --- a/scripts/run_benchmarks.py +++ b/scripts/run_benchmarks.py @@ -144,6 +144,7 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument("-l", "--list", action="store_true", help="List all benchmarks") 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("-b", "--backend", default="cocotb", choices=["cocotb", "pysim"], help="Simulation backend") parser.add_argument( @@ -171,6 +172,9 @@ def main(): print(f"Could not find benchmark '{args.benchmark_name}'") sys.exit(1) + if args.profile: + os.environ["__TRANSACTRON_PROFILE"] = "1" + success = run_benchmarks(benchmarks, args.backend, args.trace, args.verbose) if not success: print("Benchmark execution failed") diff --git a/test/regression/benchmark.py b/test/regression/benchmark.py index 8f9cb56c2..5b465d650 100644 --- a/test/regression/benchmark.py +++ b/test/regression/benchmark.py @@ -9,6 +9,7 @@ test_dir = Path(__file__).parent.parent embench_dir = test_dir.joinpath("external/embench/build/src") results_dir = test_dir.joinpath("regression/benchmark_results") +profile_dir = test_dir.joinpath("__profiles__") @dataclass_json @@ -77,6 +78,10 @@ async def run_benchmark(sim_backend: SimulationBackend, benchmark_name: str): result = await sim_backend.run(mem_model, timeout_cycles=2000000) + if result.profile is not None: + os.makedirs(profile_dir, exist_ok=True) + result.profile.encode(f"{profile_dir}/benchmark.{benchmark_name}.json") + if not result.success: raise RuntimeError("Simulation timed out") diff --git a/test/regression/cocotb.py b/test/regression/cocotb.py index faae07b9e..87c043688 100644 --- a/test/regression/cocotb.py +++ b/test/regression/cocotb.py @@ -8,13 +8,14 @@ import cocotb from cocotb.clock import Clock, Timer from cocotb.handle import ModifiableObject -from cocotb.triggers import FallingEdge, Event, with_timeout +from cocotb.triggers import FallingEdge, Event, RisingEdge, with_timeout from cocotb_bus.bus import Bus from cocotb.result import SimTimeoutError from .memory import * from .common import SimulationBackend, SimulationExecutionResult +from transactron.profiler import CycleProfile, MethodSamples, Profile, ProfileSamples, TransactionSamples from transactron.utils.gen import GenerationInfo @@ -151,12 +152,43 @@ def get_cocotb_handle(self, path_components: list[str]) -> ModifiableObject: obj = self.dut # Skip the first component, as it is already referenced in "self.dut" for component in path_components[1:]: - # As the component may start with '_' character, we need to use '_id' - # function instead of 'getattr' - this is required by cocotb. - obj = obj._id(component, extended=False) + try: + # As the component may start with '_' character, we need to use '_id' + # function instead of 'getattr' - this is required by cocotb. + obj = obj._id(component, extended=False) + except AttributeError: + if component[0] == "\\" and component[-1] == " ": + # workaround for cocotb/verilator weirdness + # for some escaped names lookup fails, but works when unescaped + obj = obj._id(component[1:-1], extended=False) + else: + raise return obj + async def profile_handler(self, clock, profile: Profile): + clock_edge_event = RisingEdge(clock) + + while True: + samples = ProfileSamples() + + for transaction_id, location in self.gen_info.transaction_signals_location.items(): + request_val = self.get_cocotb_handle(location.request) + runnable_val = self.get_cocotb_handle(location.runnable) + grant_val = self.get_cocotb_handle(location.grant) + samples.transactions[transaction_id] = TransactionSamples( + bool(request_val.value), bool(runnable_val.value), bool(grant_val.value) + ) + + for method_id, location in self.gen_info.method_signals_location.items(): + run_val = self.get_cocotb_handle(location.run) + samples.methods[method_id] = MethodSamples(bool(run_val.value)) + + cprof = CycleProfile.make(samples, self.gen_info.profile_data) + profile.cycles.append(cprof) + + await clock_edge_event # type: ignore + async def assert_handler(self, clock): clock_edge_event = FallingEdge(clock) @@ -182,6 +214,12 @@ async def run(self, mem_model: CoreMemoryModel, timeout_cycles: int = 5000) -> S data_wb = WishboneSlave(self.dut, "wb_data", self.dut.clk, mem_model, is_instr_bus=False) cocotb.start_soon(data_wb.start()) + profile = None + if "__TRANSACTRON_PROFILE" in os.environ: + profile = Profile() + profile.transactions_and_methods = self.gen_info.profile_data.transactions_and_methods + cocotb.start_soon(self.profile_handler(self.dut.clk, profile)) + cocotb.start_soon(self.assert_handler(self.dut.clk)) success = True @@ -192,6 +230,8 @@ async def run(self, mem_model: CoreMemoryModel, timeout_cycles: int = 5000) -> S result = SimulationExecutionResult(success) + result.profile = profile + for metric_name, metric_loc in self.gen_info.metrics_location.items(): result.metric_values[metric_name] = {} for reg_name, reg_loc in metric_loc.regs.items(): diff --git a/test/regression/common.py b/test/regression/common.py index 9124f5711..62481c17e 100644 --- a/test/regression/common.py +++ b/test/regression/common.py @@ -1,6 +1,8 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field +from typing import Optional from .memory import CoreMemoryModel +from transactron.profiler import Profile @dataclass @@ -18,6 +20,7 @@ class SimulationExecutionResult: success: bool metric_values: dict[str, dict[str, int]] = field(default_factory=dict) + profile: Optional[Profile] = None class SimulationBackend(ABC): diff --git a/test/regression/conftest.py b/test/regression/conftest.py index 54648bbb5..144fe5c93 100644 --- a/test/regression/conftest.py +++ b/test/regression/conftest.py @@ -6,6 +6,7 @@ test_dir = Path(__file__).parent.parent riscv_tests_dir = test_dir.joinpath("external/riscv-tests") +profile_dir = test_dir.joinpath("__profiles__") def get_all_test_names(): diff --git a/test/regression/pysim.py b/test/regression/pysim.py index 525b13234..424d83d8e 100644 --- a/test/regression/pysim.py +++ b/test/regression/pysim.py @@ -1,13 +1,16 @@ import re +import os from amaranth.sim import Passive, Settle from amaranth.utils import exact_log2 from amaranth import * +from transactron.core import TransactionManagerKey + from .memory import * from .common import SimulationBackend, SimulationExecutionResult -from transactron.testing import PysimSimulator, TestGen +from transactron.testing import PysimSimulator, TestGen, profiler_process, Profile from transactron.utils.dependencies import DependencyContext, DependencyManager from transactron.lib.metrics import HardwareMetricsManager from ..peripherals.test_wishbone import WishboneInterfaceWrapper @@ -142,6 +145,12 @@ async def run(self, mem_model: CoreMemoryModel, timeout_cycles: int = 5000) -> S sim.add_sync_process(self._wishbone_slave(mem_model, wb_instr_ctrl, is_instr_bus=True)) sim.add_sync_process(self._wishbone_slave(mem_model, wb_data_ctrl, is_instr_bus=False)) + profile = None + if "__TRANSACTRON_PROFILE" in os.environ: + transaction_manager = DependencyContext.get().get_dependency(TransactionManagerKey()) + profile = Profile() + sim.add_sync_process(profiler_process(transaction_manager, profile)) + metric_values: dict[str, dict[str, int]] = {} def on_sim_finish(): @@ -160,7 +169,7 @@ def on_sim_finish(): if self.verbose: self.pretty_dump_metrics(metric_values) - return SimulationExecutionResult(success, metric_values) + return SimulationExecutionResult(success, metric_values, profile) def stop(self): self.running = False diff --git a/test/regression/test_regression.py b/test/regression/test_regression.py index ec5d33a9e..a85c9c7f4 100644 --- a/test/regression/test_regression.py +++ b/test/regression/test_regression.py @@ -1,6 +1,6 @@ from .memory import * from .common import SimulationBackend -from .conftest import riscv_tests_dir +from .conftest import riscv_tests_dir, profile_dir from test.regression.pysim import PySimulation import xml.etree.ElementTree as eT import asyncio @@ -48,6 +48,10 @@ async def run_test(sim_backend: SimulationBackend, test_name: str): result = await sim_backend.run(mem_model, timeout_cycles=5000) + if result.profile is not None: + os.makedirs(profile_dir, exist_ok=True) + result.profile.encode(f"{profile_dir}/test.regression.{test_name}.json") + if not result.success: raise RuntimeError("Simulation timed out") diff --git a/transactron/profiler.py b/transactron/profiler.py index 410538ac4..0132b2ef7 100644 --- a/transactron/profiler.py +++ b/transactron/profiler.py @@ -1,18 +1,31 @@ +import os from collections import defaultdict from typing import Optional from dataclasses import dataclass, field -from transactron.utils import SrcLoc from dataclasses_json import dataclass_json +from transactron.utils import SrcLoc, IdGenerator +from transactron.core import MethodMap, TransactionManager -__all__ = ["ProfileInfo", "Profile"] +__all__ = [ + "ProfileInfo", + "ProfileData", + "RunStat", + "RunStatNode", + "Profile", + "TransactionSamples", + "MethodSamples", + "ProfileSamples", +] @dataclass_json @dataclass class ProfileInfo: - """Information about transactions and methods. In `Profile`, transactions - and methods are referred to by their unique ID numbers. + """Information about transactions and methods. + + In `Profile`, transactions and methods are referred to by their unique ID + numbers. Attributes ---------- @@ -29,6 +42,68 @@ class ProfileInfo: is_transaction: bool +@dataclass +class ProfileData: + """Information about transactions and methods from the transaction manager. + + This data is required for transaction profile generation in simulators. + Transactions and methods are referred to by their unique ID numbers. + + Attributes + ---------- + transactions_and_methods: dict[int, ProfileInfo] + Information about individual transactions and methods. + method_parents: dict[int, list[int]] + Lists the callers (transactions and methods) for each method. Key is + method ID. + transactions_by_method: dict[int, list[int]] + Lists which transactions are calling each method. Key is method ID. + transaction_conflicts: dict[int, list[int]] + List which other transactions conflict with each transaction. + """ + + transactions_and_methods: dict[int, ProfileInfo] + method_parents: dict[int, list[int]] + transactions_by_method: dict[int, list[int]] + transaction_conflicts: dict[int, list[int]] + + @staticmethod + def make(transaction_manager: TransactionManager): + transactions_and_methods = dict[int, ProfileInfo]() + method_parents = dict[int, list[int]]() + transactions_by_method = dict[int, list[int]]() + transaction_conflicts = dict[int, list[int]]() + + method_map = MethodMap(transaction_manager.transactions) + cgr, _, _ = TransactionManager._conflict_graph(method_map) + get_id = IdGenerator() + + def local_src_loc(src_loc: SrcLoc): + return (os.path.relpath(src_loc[0]), src_loc[1]) + + for transaction in method_map.transactions: + transactions_and_methods[get_id(transaction)] = ProfileInfo( + transaction.owned_name, local_src_loc(transaction.src_loc), True + ) + + for method in method_map.methods: + transactions_and_methods[get_id(method)] = ProfileInfo( + method.owned_name, local_src_loc(method.src_loc), False + ) + method_parents[get_id(method)] = [get_id(t_or_m) for t_or_m in method_map.method_parents[method]] + transactions_by_method[get_id(method)] = [ + get_id(t_or_m) for t_or_m in method_map.transactions_by_method[method] + ] + + for transaction, transactions in cgr.items(): + transaction_conflicts[get_id(transaction)] = [get_id(transaction2) for transaction2 in transactions] + + return ( + ProfileData(transactions_and_methods, method_parents, transactions_by_method, transaction_conflicts), + get_id, + ) + + @dataclass class RunStat: """Collected statistics about a transaction or method. @@ -76,6 +151,54 @@ def make(info: ProfileInfo): return RunStatNode(RunStat.make(info)) +@dataclass +class TransactionSamples: + """Runtime value of transaction control signals in a given clock cycle. + + Attributes + ---------- + request: bool + The value of the transaction's ``request`` signal. + runnable: bool + The value of the transaction's ``runnable`` signal. + grant: bool + The value of the transaction's ``grant`` signal. + """ + + request: bool + runnable: bool + grant: bool + + +@dataclass +class MethodSamples: + """Runtime value of method control signals in a given clock cycle. + + Attributes + ---------- + run: bool + The value of the method's ``run`` signal. + """ + + run: bool + + +@dataclass +class ProfileSamples: + """Runtime values of all transaction and method control signals. + + Attributes + ---------- + transactions: dict[int, TransactionSamples] + Runtime values of transaction control signals for each transaction. + methods: dict[int, MethodSamples] + Runtime values of method control signals for each method. + """ + + transactions: dict[int, TransactionSamples] = field(default_factory=dict) + methods: dict[int, MethodSamples] = field(default_factory=dict) + + @dataclass_json @dataclass class CycleProfile: @@ -98,6 +221,44 @@ class CycleProfile: locked: dict[int, int] = field(default_factory=dict) running: dict[int, Optional[int]] = field(default_factory=dict) + @staticmethod + def make(samples: ProfileSamples, data: ProfileData): + cprof = CycleProfile() + + for transaction_id, transaction_samples in samples.transactions.items(): + if transaction_samples.grant: + cprof.running[transaction_id] = None + elif transaction_samples.request and transaction_samples.runnable: + for transaction2_id in data.transaction_conflicts[transaction_id]: + if samples.transactions[transaction2_id].grant: + cprof.locked[transaction_id] = transaction2_id + + running = set(cprof.running) + for method_id, method_samples in samples.methods.items(): + if method_samples.run: + running.add(method_id) + + locked_methods = set[int]() + for method_id in samples.methods.keys(): + if method_id not in running: + if any(transaction_id in running for transaction_id in data.transactions_by_method[method_id]): + locked_methods.add(method_id) + + for method_id in samples.methods.keys(): + if method_id in running: + for t_or_m_id in data.method_parents[method_id]: + if t_or_m_id in running: + cprof.running[method_id] = t_or_m_id + elif method_id in locked_methods: + caller = next( + t_or_m_id + for t_or_m_id in data.method_parents[method_id] + if t_or_m_id in running or t_or_m_id in locked_methods + ) + cprof.locked[method_id] = caller + + return cprof + @dataclass_json @dataclass diff --git a/transactron/testing/profiler.py b/transactron/testing/profiler.py index f857d4586..18451112c 100644 --- a/transactron/testing/profiler.py +++ b/transactron/testing/profiler.py @@ -1,8 +1,6 @@ -import os.path from amaranth.sim import * from transactron.core import MethodMap, TransactionManager -from transactron.profiler import CycleProfile, Profile, ProfileInfo -from transactron.utils import SrcLoc +from transactron.profiler import CycleProfile, MethodSamples, Profile, ProfileData, ProfileSamples, TransactionSamples from .functions import TestGen __all__ = ["profiler_process"] @@ -10,75 +8,28 @@ def profiler_process(transaction_manager: TransactionManager, profile: Profile): def process() -> TestGen: + profile_data, get_id = ProfileData.make(transaction_manager) 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 - - 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, local_src_loc(transaction.src_loc), True - ) - - 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 = profile_data.transactions_and_methods yield Passive() while True: yield Tick("sync_neg") - cprof = CycleProfile() - profile.cycles.append(cprof) + samples = ProfileSamples() for transaction in method_map.transactions: - request = yield transaction.request - runnable = yield transaction.runnable - grant = yield transaction.grant + samples.transactions[get_id(transaction)] = TransactionSamples( + bool((yield transaction.request)), + bool((yield transaction.runnable)), + bool((yield transaction.grant)), + ) - if grant: - cprof.running[get_id(transaction)] = None - elif request and runnable: - for transaction2 in cgr[transaction]: - if (yield transaction2.grant): - 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)) + samples.methods[get_id(method)] = MethodSamples(bool((yield method.run))) - for method in method_map.methods: - if get_id(method) in running: - for t_or_m in method_map.method_parents[method]: - 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 - ) - cprof.locked[get_id(method)] = caller + cprof = CycleProfile.make(samples, profile_data) + profile.cycles.append(cprof) yield diff --git a/transactron/utils/__init__.py b/transactron/utils/__init__.py index 5af46d3a1..703b2aad9 100644 --- a/transactron/utils/__init__.py +++ b/transactron/utils/__init__.py @@ -7,3 +7,4 @@ from .assertion import * # noqa: F401 from .dependencies import * # noqa: F401 from .depcache import * # noqa: F401 +from .idgen import * # noqa: F401 diff --git a/transactron/utils/gen.py b/transactron/utils/gen.py index b2cbf6da9..2ff40dec2 100644 --- a/transactron/utils/gen.py +++ b/transactron/utils/gen.py @@ -5,7 +5,11 @@ from amaranth.back import verilog from amaranth.hdl import Fragment +from transactron.core import TransactionManager, MethodMap, TransactionManagerKey from transactron.lib.metrics import HardwareMetricsManager +from transactron.utils.dependencies import DependencyContext +from transactron.utils.idgen import IdGenerator +from transactron.profiler import ProfileData from transactron.utils._typing import SrcLoc from transactron.utils.assertion import assert_bits @@ -39,6 +43,40 @@ class MetricLocation: regs: dict[str, list[str]] = field(default_factory=dict) +@dataclass_json +@dataclass +class TransactionSignalsLocation: + """Information about transaction control signals in the generated Verilog code. + + Attributes + ---------- + request: list[str] + The location of the ``request`` signal. + runnable: list[str] + The location of the ``runnable`` signal. + grant: list[str] + The location of the ``grant`` signal. + """ + + request: list[str] + runnable: list[str] + grant: list[str] + + +@dataclass_json +@dataclass +class MethodSignalsLocation: + """Information about method control signals in the generated Verilog code. + + Attributes + ---------- + run: list[str] + The location of the ``run`` signal. + """ + + run: list[str] + + @dataclass_json @dataclass class AssertLocation: @@ -72,8 +110,12 @@ class GenerationInfo: Locations and metadata for assertion signals. """ - metrics_location: dict[str, MetricLocation] = field(default_factory=dict) - asserts: list[AssertLocation] = field(default_factory=list) + metrics_location: dict[str, MetricLocation] + transaction_signals_location: dict[int, TransactionSignalsLocation] + method_signals_location: dict[int, MethodSignalsLocation] + profile_data: ProfileData + metrics_location: dict[str, MetricLocation] + asserts: list[AssertLocation] def encode(self, file_name: str): """ @@ -111,7 +153,6 @@ def escape_verilog_identifier(identifier: str) -> str: for char in characters_to_escape: if char in identifier: - # Note the intentional space at the end. return f"\\{identifier} " return identifier @@ -142,6 +183,30 @@ def collect_metric_locations(name_map: "SignalDict") -> dict[str, MetricLocation return metrics_location +def collect_transaction_method_signals( + transaction_manager: TransactionManager, name_map: "SignalDict" +) -> tuple[dict[int, TransactionSignalsLocation], dict[int, MethodSignalsLocation]]: + transaction_signals_location: dict[int, TransactionSignalsLocation] = {} + method_signals_location: dict[int, MethodSignalsLocation] = {} + + method_map = MethodMap(transaction_manager.transactions) + get_id = IdGenerator() + + for transaction in method_map.transactions: + request_loc = get_signal_location(transaction.request, name_map) + runnable_loc = get_signal_location(transaction.runnable, name_map) + grant_loc = get_signal_location(transaction.grant, name_map) + transaction_signals_location[get_id(transaction)] = TransactionSignalsLocation( + request_loc, runnable_loc, grant_loc + ) + + for method in method_map.methods: + run_loc = get_signal_location(method.run, name_map) + method_signals_location[get_id(method)] = MethodSignalsLocation(run_loc) + + return (transaction_signals_location, method_signals_location) + + def collect_asserts(name_map: "SignalDict") -> list[AssertLocation]: asserts: list[AssertLocation] = [] @@ -157,6 +222,17 @@ def generate_verilog( fragment = Fragment.get(top_module, platform=None).prepare(ports=ports) verilog_text, name_map = verilog.convert_fragment(fragment, name=top_name, emit_src=True, strip_internal_attrs=True) - gen_info = GenerationInfo(metrics_location=collect_metric_locations(name_map), asserts=collect_asserts(name_map)) + transaction_manager = DependencyContext.get().get_dependency(TransactionManagerKey()) + transaction_signals, method_signals = collect_transaction_method_signals( + transaction_manager, name_map # type: ignore + ) + profile_data, _ = ProfileData.make(transaction_manager) + gen_info = GenerationInfo( + metrics_location=collect_metric_locations(name_map), # type: ignore + transaction_signals_location=transaction_signals, + method_signals_location=method_signals, + profile_data=profile_data, + asserts=collect_asserts(name_map), + ) return verilog_text, gen_info diff --git a/transactron/utils/idgen.py b/transactron/utils/idgen.py new file mode 100644 index 000000000..459f3160e --- /dev/null +++ b/transactron/utils/idgen.py @@ -0,0 +1,15 @@ +__all__ = ["IdGenerator"] + + +class IdGenerator: + def __init__(self): + self.id_map = dict[int, int]() + self.id_seq = 0 + + def __call__(self, obj): + try: + return self.id_map[id(obj)] + except KeyError: + self.id_seq += 1 + self.id_map[id(obj)] = self.id_seq + return self.id_seq