From e0e480ef9e58c6c1698f3d714189b7f13232a7d4 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Dec 2025 21:55:09 -0800 Subject: [PATCH] Remove power-of-2 alignment requirement for external components (#30) * Initial plan * Remove alignment check on external components and add tests Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Improve test comment clarity Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> --- src/peakrdl_busdecoder/validate_design.py | 26 +---- tests/unit/test_external_nested.py | 112 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 25 deletions(-) diff --git a/src/peakrdl_busdecoder/validate_design.py b/src/peakrdl_busdecoder/validate_design.py index 8249677..1341781 100644 --- a/src/peakrdl_busdecoder/validate_design.py +++ b/src/peakrdl_busdecoder/validate_design.py @@ -4,7 +4,7 @@ from systemrdl.node import AddressableNode, AddrmapNode, FieldNode, Node, Regfil from systemrdl.rdltypes.references import PropertyReference from systemrdl.walker import RDLListener, RDLWalker, WalkerAction -from .utils import is_pow2, ref_is_internal, roundup_pow2 +from .utils import ref_is_internal if TYPE_CHECKING: from .exporter import BusDecoderExporter @@ -159,27 +159,3 @@ class DesignValidator(RDLListener): else: # Exiting top addrmap. Resolve final answer self.contains_external_block = contains_external_block - - if contains_external_block: - # Check that addressing follows strict alignment rules to allow - # for simplified address bit-pruning - if node.external: - err_suffix = "is external" - else: - err_suffix = "contains an external addrmap/regfile/mem" - - req_align = roundup_pow2(node.size) - if (node.raw_address_offset % req_align) != 0: - self.msg.error( - f"Address offset +0x{node.raw_address_offset:x} of instance '{node.inst_name}' is not a power of 2 multiple of its size 0x{node.size:x}. " - f"This is required by the busdecoder exporter if a component {err_suffix}.", - node.inst.inst_src_ref, - ) - if node.is_array: - assert node.array_stride is not None - if not is_pow2(node.array_stride): - self.msg.error( - f"Address stride of instance array '{node.inst_name}' is not a power of 2" - f"This is required by the busdecoder exporter if a component {err_suffix}.", - node.inst.inst_src_ref, - ) diff --git a/tests/unit/test_external_nested.py b/tests/unit/test_external_nested.py index aca9f28..425200e 100644 --- a/tests/unit/test_external_nested.py +++ b/tests/unit/test_external_nested.py @@ -138,3 +138,115 @@ def test_max_decode_depth_parameter_exists(compile_rdl: Callable[..., AddrmapNod module_file = Path(tmpdir) / "simple.sv" assert module_file.exists() + +def test_unaligned_external_component_supported(compile_rdl: Callable[..., AddrmapNode]) -> None: + """Test that external components can be at unaligned addresses. + + This test verifies that external components don't need to be aligned + to a power-of-2 multiple of their size, as the busdecoder supports + unaligned access. + """ + rdl_source = """ + mem queue_t { + name = "Queue"; + mementries = 1024; + memwidth = 64; + }; + + addrmap buffer_t { + name = "Buffer"; + desc = ""; + + external queue_t multicast @ 0x100; // Not power-of-2 aligned + }; + """ + top = compile_rdl(rdl_source, top="buffer_t") + + with TemporaryDirectory() as tmpdir: + exporter = BusDecoderExporter() + # Should not raise an alignment error + exporter.export(top, tmpdir, cpuif_cls=APB4Cpuif) + + # Verify output was generated + module_file = Path(tmpdir) / "buffer_t.sv" + assert module_file.exists() + + content = module_file.read_text() + # Verify the external component is in the generated code + assert "multicast" in content + + +def test_unaligned_external_component_array_supported(compile_rdl: Callable[..., AddrmapNode]) -> None: + """Test that external component arrays with non-power-of-2 strides are supported. + + This test verifies that external component arrays can have arbitrary strides, + not just power-of-2 strides. + """ + rdl_source = """ + mem queue_t { + name = "Queue"; + mementries = 256; + memwidth = 32; + }; + + addrmap buffer_t { + name = "Buffer"; + desc = ""; + + external queue_t port[4] @ 0x0 += 0x600; // Stride of 0x600 (not power-of-2) to test unaligned support + }; + """ + top = compile_rdl(rdl_source, top="buffer_t") + + with TemporaryDirectory() as tmpdir: + exporter = BusDecoderExporter() + # Should not raise an alignment error + exporter.export(top, tmpdir, cpuif_cls=APB4Cpuif) + + # Verify output was generated + module_file = Path(tmpdir) / "buffer_t.sv" + assert module_file.exists() + + content = module_file.read_text() + # Verify the external component array is in the generated code + assert "port" in content + + +def test_unaligned_external_nested_in_addrmap(compile_rdl: Callable[..., AddrmapNode]) -> None: + """Test that addrmaps containing external components can be at unaligned addresses. + + This verifies that not just external components themselves, but also + non-external addrmaps/regfiles that contain external components can be + at unaligned addresses. + """ + rdl_source = """ + mem queue_t { + name = "Queue"; + mementries = 512; + memwidth = 32; + }; + + addrmap inner_block { + external queue_t ext_queue @ 0x0; + }; + + addrmap outer_block { + inner_block inner @ 0x150; // Not power-of-2 aligned + }; + """ + top = compile_rdl(rdl_source, top="outer_block") + + with TemporaryDirectory() as tmpdir: + exporter = BusDecoderExporter() + # Should not raise an alignment error + exporter.export(top, tmpdir, cpuif_cls=APB4Cpuif) + + # Verify output was generated + module_file = Path(tmpdir) / "outer_block.sv" + assert module_file.exists() + + content = module_file.read_text() + # Verify the nested components are in the generated code + assert "inner" in content + +