Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
chenz opened this issue Mar 30, 2023 · 0 comments

Comments

@chenz
Copy link

chenz commented Mar 30, 2023

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).

@chenz chenz changed the title Resources on dense memory windows must be sized multipes of "parent" data_width? Resources on dense memory windows must be sized multilpes of "parent" data_width? Mar 30, 2023
@chenz chenz changed the title Resources on dense memory windows must be sized multilpes of "parent" data_width? Resources on dense memory windows must be sized multiples of "parent" data_width? Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant