amaranth-lang / amaranth-soc

System on Chip toolkit for Amaranth HDL
BSD 2-Clause "Simplified" License
84 stars 31 forks source link

Resources on dense memory windows must be sized multiples of "parent" data_width? #39

Open chenz opened 1 year ago

chenz commented 1 year ago

Apparently, resources in a dense memory window cannot be smaller than the data width of the parent memory map. This can be demonstrated in test_memory.py by making the resource in the dense window smaller than the ratio of the window (2 < 4):

# from this, which works
self.win3.add_resource(self.res6, name="name6", size=16)
# to this, which raises AssertionError in MemoryMap._translate(), via MemoryMap.all_resources() later on
self.win3.add_resource(self.res6, name="name6", size=2)

Here is another test case I created:

import pytest

from amaranth_soc.memory import MemoryMap

def test_dense():
    regs = ("reg0", "reg1", "reg2", "reg3")
    window = MemoryMap(addr_width=2, data_width=8)
    for reg in regs:
        window.add_resource(reg, name=reg, size=1)
    memory_map = MemoryMap(addr_width=1, data_width=16)
    (start, end, ratio) = memory_map.add_window(window, sparse=False)
    assert start == 0
    assert end == 2
    assert ratio == 2
    assert memory_map.decode_address(0) == regs[0] # unexpected! (would expect regs[0], regs[1])
    assert memory_map.decode_address(1) == regs[0] # completely unexpected!
    with pytest.raises(AssertionError): # unexpected!
        list(memory_map.all_resources())
    for reg in regs:
        with pytest.raises(AssertionError): # unexpected!
            res = memory_map.find_resource(reg)

I would expect that it is possible to address multiple units with a single address in this case. Am I making a logical mistake this this assumption?

If not, is this just a problem with the implementation (e.g. ResourceInfo not being able to represent "fractional" addresses)?

In either case, I think this should be caught in add_window() already (or at least with an explanatory exception text).