Implement RFC 40: Arbitrary Memory shapes.
#1052
Conversation
| @@ -831,7 +831,11 @@ def _convert_fragment(builder, fragment, name_map, hierarchy): | |||
|
|
|||
| if isinstance(fragment, mem.MemoryInstance): | |||
| memory = fragment.memory | |||
| init = "".join(format(ast.Const(elem, ast.unsigned(memory.width)).value, f"0{memory.width}b") for elem in reversed(memory.init)) | |||
| if isinstance(memory.shape, ast.ShapeCastable): | |||
There was a problem hiding this comment.
please move the const-casting code to Memory.__init__, so that 1) it's deduplicated between RTLIL and sim, 2) any errors are reported earlier, during Memory construction
There was a problem hiding this comment.
er, to Memory.init setter I guess
There was a problem hiding this comment.
I considered doing it that way, but I figure either way we should keep the uncasted values around so the getter is consistent with the setter. The getter also returns a mutable list, so the following code is valid:
mem = Memory(width = 8, depth = 16)
for i in range(mem.depth):
mem.init.append(…)If we want to do the cast in the setter, we should also make the list immutable without going through the setter.
There was a problem hiding this comment.
We can make MemoryInitializer a new MutableSequence collection that ensures all of the values inside are const-castable.
| @@ -1929,6 +1929,8 @@ class ValueSet(_MappedKeySet): | |||
|
|
|||
| class SignalKey: | |||
| def __init__(self, signal): | |||
| if isinstance(signal, ValueCastable): | |||
| signal = Value.cast(signal) | |||
There was a problem hiding this comment.
I'd say this is a consequence of RFC 15; a Signal constructed from a ShapeCastable becomes a ValueCastable around the signal.
When constructing a SignalKey from such a signal, in this case port.data, we need to unwrap the underlying signal.
There was a problem hiding this comment.
So this comes back to the additions to hdl/xfrm.py.
| @@ -831,7 +831,11 @@ def _convert_fragment(builder, fragment, name_map, hierarchy): | |||
|
|
|||
| if isinstance(fragment, mem.MemoryInstance): | |||
| memory = fragment.memory | |||
| init = "".join(format(ast.Const(elem, ast.unsigned(memory.width)).value, f"0{memory.width}b") for elem in reversed(memory.init)) | |||
| if isinstance(memory.shape, ast.ShapeCastable): | |||
There was a problem hiding this comment.
We can make MemoryInitializer a new MutableSequence collection that ensures all of the values inside are const-castable.
| cast = lambda elem: ast.Value.cast(memory.shape.const(elem)).value | ||
| else: | ||
| cast = lambda elem: elem | ||
| init = "".join(format(ast.Const(cast(elem), ast.unsigned(memory.width)).value, f"0{memory.width}b") for elem in reversed(memory.init)) |
There was a problem hiding this comment.
This seems like a bunch of repeated work.
| depth : int | ||
| Word count. This memory contains ``depth`` storage elements. | ||
| init : list of int | ||
| init : list |
| @@ -75,7 +107,10 @@ def init(self, new_init): | |||
| try: | |||
| for addr in range(len(self._array)): | |||
| if addr < len(self._init): | |||
| self._array[addr].reset = operator.index(self._init[addr]) | |||
| if isinstance(self.shape, ShapeCastable): | |||
There was a problem hiding this comment.
Having a MemoryInitializer collection would make this code nicer, too.
| @@ -254,6 +289,8 @@ class WritePort(Elaboratable): | |||
| divide memory width evenly. | |||
| """ | |||
| def __init__(self, memory, *, domain="sync", granularity=None, src_loc_at=0): | |||
| if granularity is not None and isinstance(memory.shape, ShapeCastable) or memory.shape.signed: | |||
There was a problem hiding this comment.
I think a check for not (isinstance(memory.shape, Shape) and memory.shape.unsigned) is clearer.
| @@ -276,11 +276,11 @@ def map_memory_ports(self, fragment, new_fragment): | |||
| for port in new_fragment.read_ports: | |||
| port.en = self.on_value(port.en) | |||
| port.addr = self.on_value(port.addr) | |||
| port.data = self.on_value(port.data) | |||
| port.data = self.on_value(Value.cast(port.data)) | |||
There was a problem hiding this comment.
Doesn't this code break the logic of the transformer? @wanda-phi
|
Superseded by #1142. |
No description provided.