hdl.mem: lower Memory directly to $mem_v2 RTLIL cell.

The design decision of using split memory ports in the internal
representation (copied from Yosys) was misguided and caused no end
of misery. Remove any uses of `$memrd`/`$memwr` and lower memories
directly to a combined memory cell, currently the RTLIL one.

Author: wanda-phi

amaranth/back/rtlil.py

@@ -863,49 +863,21 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
             if sub_name is None:
                 sub_name = module.anonymous()
 
-            sub_params = OrderedDict()
-            if hasattr(subfragment, "parameters"):
-                for param_name, param_value in subfragment.parameters.items():
-                    if isinstance(param_value, mem.Memory):
-                        memory = param_value
-                        if memory not in memories:
-                            memories[memory] = module.memory(width=memory.width, size=memory.depth,
-                                                             name=memory.name, attrs=memory.attrs)
-                            addr_bits = bits_for(memory.depth)
-                            data_parts = []
-                            data_mask = (1 << memory.width) - 1
-                            for addr in range(memory.depth):
-                                if addr < len(memory.init):
-                                    data = memory.init[addr] & data_mask
-                                else:
-                                    data = 0
-                                data_parts.append("{:0{}b}".format(data, memory.width))
-                            module.cell("$meminit", ports={
-                                "\\ADDR": rhs_compiler(ast.Const(0, addr_bits)),
-                                "\\DATA": "{}'".format(memory.width * memory.depth) +
-                                          "".join(reversed(data_parts)),
-                            }, params={
-                                "MEMID": memories[memory],
-                                "ABITS": addr_bits,
-                                "WIDTH": memory.width,
-                                "WORDS": memory.depth,
-                                "PRIORITY": 0,
-                            })
-
-                        param_value = memories[memory]
-
-                    sub_params[param_name] = param_value
+            sub_params = OrderedDict(getattr(subfragment, "parameters", {}))
 
             sub_type, sub_port_map = \
                 _convert_fragment(builder, subfragment, name_map,
                                   hierarchy=hierarchy + (sub_name,))
 
+            if sub_type == "$mem_v2" and "MEMID" not in sub_params:
+                sub_params["MEMID"] = "$" + sub_name
+            
             sub_ports = OrderedDict()
             for port, value in sub_port_map.items():
                 if not isinstance(subfragment, ir.Instance):
                     for signal in value._rhs_signals():
                         compiler_state.resolve_curr(signal, prefix=sub_name)
-                if len(value) > 0:
+                if len(value) > 0 or sub_type == "$mem_v2":
                     sub_ports[port] = rhs_compiler(value)
 
             module.cell(sub_type, name=sub_name, ports=sub_ports, params=sub_params,

amaranth/compat/fhdl/specials.py

@@ -83,12 +83,6 @@ def __init__(self, adr, dat_r, we=None, dat_w=None, async_read=False, re=None,
         self.clock = ClockSignal(clock_domain)
 
 
-@extend(NativeMemory)
-@deprecated("it is not necessary or permitted to add Memory as a special or submodule")
-def elaborate(self, platform):
-    return Fragment()
-
-
 class CompatMemory(NativeMemory, Elaboratable):
     def __init__(self, width, depth, init=None, name=None):
         super().__init__(width=width, depth=depth, init=init, name=name)

amaranth/hdl/ir.py

@@ -181,7 +181,6 @@ def _resolve_hierarchy_conflicts(self, hierarchy=("top",), mode="warn"):
         assert mode in ("silent", "warn", "error")
 
         driver_subfrags = SignalDict()
-        memory_subfrags = OrderedDict()
         def add_subfrag(registry, entity, entry):
             # Because of missing domain insertion, at the point when this code runs, we have
             # a mixture of bound and unbound {Clock,Reset}Signals. Map the bound ones to
@@ -212,24 +211,16 @@ def add_subfrag(registry, entity, entry):
                 flatten_subfrags.add((subfrag, subfrag_hierarchy))
 
             if isinstance(subfrag, Instance):
-                # For memories (which are subfragments, but semantically a part of superfragment),
-                # record that this fragment is driving it.
-                if subfrag.type in ("$memrd", "$memwr"):
-                    memory = subfrag.parameters["MEMID"]
-                    add_subfrag(memory_subfrags, memory, (None, hierarchy))
-
                 # Never flatten instances.
                 continue
 
             # First, recurse into subfragments and let them detect driver conflicts as well.
-            subfrag_drivers, subfrag_memories = \
+            subfrag_drivers = \
                 subfrag._resolve_hierarchy_conflicts(subfrag_hierarchy, mode)
 
-            # Second, classify subfragments by signals they drive and memories they use.
+            # Second, classify subfragments by signals they drive.
             for signal in subfrag_drivers:
                 add_subfrag(driver_subfrags, signal, (subfrag, subfrag_hierarchy))
-            for memory in subfrag_memories:
-                add_subfrag(memory_subfrags, memory, (subfrag, subfrag_hierarchy))
 
         # Find out the set of subfragments that needs to be flattened into this fragment
         # to resolve driver-driver conflicts.
@@ -253,20 +244,6 @@ def flatten_subfrags_if_needed(subfrags):
                 message += "; hierarchy will be flattened"
                 warnings.warn_explicit(message, DriverConflict, *signal.src_loc)
 
-        for memory, subfrags in memory_subfrags.items():
-            subfrag_names = flatten_subfrags_if_needed(subfrags)
-            if not subfrag_names:
-                continue
-
-            # While we're at it, show a message.
-            message = ("Memory '{}' is accessed from multiple fragments: {}"
-                       .format(memory.name, ", ".join(subfrag_names)))
-            if mode == "error":
-                raise DriverConflict(message)
-            elif mode == "warn":
-                message += "; hierarchy will be flattened"
-                warnings.warn_explicit(message, DriverConflict, *memory.src_loc)
-
         # Flatten hierarchy.
         for subfrag, subfrag_hierarchy in sorted(flatten_subfrags, key=lambda x: x[1]):
             self._merge_subfragment(subfrag)
@@ -282,8 +259,7 @@ def flatten_subfrags_if_needed(subfrags):
             return self._resolve_hierarchy_conflicts(hierarchy, mode)
 
         # Nothing was flattened, we're done!
-        return (SignalSet(driver_subfrags.keys()),
-                set(memory_subfrags.keys()))
+        return SignalSet(driver_subfrags.keys())
 
     def _propagate_domains_up(self, hierarchy=("top",)):
         from .xfrm import DomainRenamer

amaranth/hdl/mem.py

@@ -3,13 +3,13 @@
 
 from .. import tracer
 from .ast import *
-from .ir import Elaboratable, Instance
+from .ir import Elaboratable, Instance, Fragment
 
 
 __all__ = ["Memory", "ReadPort", "WritePort", "DummyPort"]
 
 
-class Memory:
+class Memory(Elaboratable):
     """A word addressable storage.
 
     Parameters
@@ -58,6 +58,8 @@ def __init__(self, *, width, depth, init=None, name=None, attrs=None, simulate=T
                                           .format(name or "memory", addr)))
 
         self.init = init
+        self._read_ports = []
+        self._write_ports = []
 
     @property
     def init(self):
@@ -116,6 +118,96 @@ def __getitem__(self, index):
         """Simulation only."""
         return self._array[index]
 
+    def elaborate(self, platform):
+        init = "".join(format(Const(elem, unsigned(self.width)).value, f"0{self.width}b") for elem in reversed(self.init))
+        init = Const(int(init or "0", 2), len(self.init) * self.width)
+        rd_clk = []
+        rd_clk_enable = 0
+        rd_transparency_mask = 0
+        for index, port in enumerate(self._read_ports):
+            if port.domain != "comb":
+                rd_clk.append(ClockSignal(port.domain))
+                rd_clk_enable |= 1 << index
+                if port.transparent:
+                    for write_index, write_port in enumerate(self._write_ports):
+                        if port.domain == write_port.domain:
+                            rd_transparency_mask |= 1 << (index * len(self._write_ports) + write_index)
+            else:
+                rd_clk.append(Const(0, 1))
+        f = Instance("$mem_v2",
+            *(("a", attr, value) for attr, value in self.attrs.items()),
+            p_SIZE=self.depth,
+            p_OFFSET=0,
+            p_ABITS=Shape.cast(range(self.depth)).width,
+            p_WIDTH=self.width,
+            p_INIT=init,
+            p_RD_PORTS=len(self._read_ports),
+            p_RD_CLK_ENABLE=Const(rd_clk_enable, len(self._read_ports)) if self._read_ports else Const(0, 1),
+            p_RD_CLK_POLARITY=Const(-1, unsigned(len(self._read_ports))) if self._read_ports else Const(0, 1),
+            p_RD_TRANSPARENCY_MASK=Const(rd_transparency_mask, max(1, len(self._read_ports) * len(self._write_ports))),
+            p_RD_COLLISION_X_MASK=Const(0, max(1, len(self._read_ports) * len(self._write_ports))),
+            p_RD_WIDE_CONTINUATION=Const(0, len(self._read_ports)) if self._read_ports else Const(0, 1),
+            p_RD_CE_OVER_SRST=Const(0, len(self._read_ports)) if self._read_ports else Const(0, 1),
+            p_RD_ARST_VALUE=Const(0, len(self._read_ports) * self.width),
+            p_RD_SRST_VALUE=Const(0, len(self._read_ports) * self.width),
+            p_RD_INIT_VALUE=Const(0, len(self._read_ports) * self.width),
+            p_WR_PORTS=len(self._write_ports),
+            p_WR_CLK_ENABLE=Const(-1, unsigned(len(self._write_ports))) if self._write_ports else Const(0, 1),
+            p_WR_CLK_POLARITY=Const(-1, unsigned(len(self._write_ports))) if self._write_ports else Const(0, 1),
+            p_WR_PRIORITY_MASK=Const(0, len(self._write_ports) * len(self._write_ports)) if self._write_ports else Const(0, 1),
+            p_WR_WIDE_CONTINUATION=Const(0, len(self._write_ports)) if self._write_ports else Const(0, 1),
+            i_RD_CLK=Cat(rd_clk),
+            i_RD_EN=Cat(port.en for port in self._read_ports),
+            i_RD_ARST=Const(0, len(self._read_ports)),
+            i_RD_SRST=Const(0, len(self._read_ports)),
+            i_RD_ADDR=Cat(port.addr for port in self._read_ports),
+            o_RD_DATA=Cat(port.data for port in self._read_ports),
+            i_WR_CLK=Cat(ClockSignal(port.domain) for port in self._write_ports),
+            i_WR_EN=Cat(Cat(en_bit.replicate(port.granularity) for en_bit in port.en) for port in self._write_ports),
+            i_WR_ADDR=Cat(port.addr for port in self._write_ports),
+            i_WR_DATA=Cat(port.data for port in self._write_ports),
+        )
+        for port in self._read_ports:
+            port._MustUse__used = True
+            if port.domain == "comb":
+                # Asynchronous port
+                f.add_statements(port.data.eq(self._array[port.addr]))
+                f.add_driver(port.data)
+            else:
+                # Synchronous port
+                data = self._array[port.addr]
+                for write_port in self._write_ports:
+                    if port.domain == write_port.domain and port.transparent:
+                        if len(write_port.en) > 1:
+                            parts = []
+                            for index, en_bit in enumerate(write_port.en):
+                                offset = index * write_port.granularity
+                                bits   = slice(offset, offset + write_port.granularity)
+                                cond = en_bit & (port.addr == write_port.addr)
+                                parts.append(Mux(cond, write_port.data[bits], data[bits]))
+                            data = Cat(parts)
+                        else:
+                            data = Mux(write_port.en, write_port.data, data)
+                f.add_statements(
+                    Switch(port.en, {
+                        1: port.data.eq(data)
+                    })
+                )
+                f.add_driver(port.data, port.domain)
+        for port in self._write_ports:
+            port._MustUse__used = True
+            if len(port.en) > 1:
+                for index, en_bit in enumerate(port.en):
+                    offset = index * port.granularity
+                    bits   = slice(offset, offset + port.granularity)
+                    write_data = self._array[port.addr][bits].eq(port.data[bits])
+                    f.add_statements(Switch(en_bit, { 1: write_data }))
+            else:
+                write_data = self._array[port.addr].eq(port.data)
+                f.add_statements(Switch(port.en, { 1: write_data }))
+            for signal in self._array:
+                f.add_driver(signal, port.domain)
+        return f
 
 class ReadPort(Elaboratable):
     """A memory read port.
@@ -142,9 +234,7 @@ class ReadPort(Elaboratable):
     data : Signal(memory.width), out
         Read data.
     en : Signal or Const, in
-        Read enable. If asserted, ``data`` is updated with the word stored at ``addr``. Note that
-        transparent ports cannot assign ``en`` (which is hardwired to 1 instead), as doing so is
-        currently not supported by Yosys.
+        Read enable. If asserted, ``data`` is updated with the word stored at ``addr``.
 
     Exceptions
     ----------
@@ -162,59 +252,19 @@ def __init__(self, memory, *, domain="sync", transparent=True, src_loc_at=0):
                            name="{}_r_addr".format(memory.name), src_loc_at=1 + src_loc_at)
         self.data = Signal(memory.width,
                            name="{}_r_data".format(memory.name), src_loc_at=1 + src_loc_at)
-        if self.domain != "comb" and not transparent:
+        if self.domain != "comb":
             self.en = Signal(name="{}_r_en".format(memory.name), reset=1,
                              src_loc_at=1 + src_loc_at)
         else:
             self.en = Const(1)
 
+        memory._read_ports.append(self)
+
     def elaborate(self, platform):
-        f = Instance("$memrd",
-            p_MEMID=self.memory,
-            p_ABITS=self.addr.width,
-            p_WIDTH=self.data.width,
-            p_CLK_ENABLE=self.domain != "comb",
-            p_CLK_POLARITY=1,
-            p_TRANSPARENT=self.transparent,
-            i_CLK=ClockSignal(self.domain) if self.domain != "comb" else Const(0),
-            i_EN=self.en,
-            i_ADDR=self.addr,
-            o_DATA=self.data,
-        )
-        if self.domain == "comb":
-            # Asynchronous port
-            f.add_statements(self.data.eq(self.memory._array[self.addr]))
-            f.add_driver(self.data)
-        elif not self.transparent:
-            # Synchronous, read-before-write port
-            f.add_statements(
-                Switch(self.en, {
-                    1: self.data.eq(self.memory._array[self.addr])
-                })
-            )
-            f.add_driver(self.data, self.domain)
+        if self is self.memory._read_ports[0]:
+            return self.memory
         else:
-            # Synchronous, write-through port
-            # This model is a bit unconventional. We model transparent ports as asynchronous ports
-            # that are latched when the clock is high. This isn't exactly correct, but it is very
-            # close to the correct behavior of a transparent port, and the difference should only
-            # be observable in pathological cases of clock gating. A register is injected to
-            # the address input to achieve the correct address-to-data latency. Also, the reset
-            # value of the data output is forcibly set to the 0th initial value, if any--note that
-            # many FPGAs do not guarantee this behavior!
-            if len(self.memory.init) > 0:
-                self.data.reset = operator.index(self.memory.init[0])
-            latch_addr = Signal.like(self.addr)
-            f.add_statements(
-                latch_addr.eq(self.addr),
-                Switch(ClockSignal(self.domain), {
-                    0: self.data.eq(self.data),
-                    1: self.data.eq(self.memory._array[latch_addr]),
-                }),
-            )
-            f.add_driver(latch_addr, self.domain)
-            f.add_driver(self.data)
-        return f
+            return Fragment()
 
 
 class WritePort(Elaboratable):
@@ -272,31 +322,13 @@ def __init__(self, memory, *, domain="sync", granularity=None, src_loc_at=0):
         self.en   = Signal(memory.width // granularity,
                            name="{}_w_en".format(memory.name), src_loc_at=1 + src_loc_at)
 
+        memory._write_ports.append(self)
+
     def elaborate(self, platform):
-        f = Instance("$memwr",
-            p_MEMID=self.memory,
-            p_ABITS=self.addr.width,
-            p_WIDTH=self.data.width,
-            p_CLK_ENABLE=1,
-            p_CLK_POLARITY=1,
-            p_PRIORITY=0,
-            i_CLK=ClockSignal(self.domain),
-            i_EN=Cat(en_bit.replicate(self.granularity) for en_bit in self.en),
-            i_ADDR=self.addr,
-            i_DATA=self.data,
-        )
-        if len(self.en) > 1:
-            for index, en_bit in enumerate(self.en):
-                offset = index * self.granularity
-                bits   = slice(offset, offset + self.granularity)
-                write_data = self.memory._array[self.addr][bits].eq(self.data[bits])
-                f.add_statements(Switch(en_bit, { 1: write_data }))
+        if not self.memory._read_ports and self is self.memory._write_ports[0]:
+            return self.memory
         else:
-            write_data = self.memory._array[self.addr].eq(self.data)
-            f.add_statements(Switch(self.en, { 1: write_data }))
-        for signal in self.memory._array:
-            f.add_driver(signal, self.domain)
-        return f
+            return Fragment()
 
 
 class DummyPort:

amaranth/hdl/xfrm.py

@@ -720,10 +720,14 @@ def _insert_control(self, fragment, domain, signals):
 
     def on_fragment(self, fragment):
         new_fragment = super().on_fragment(fragment)
-        if isinstance(new_fragment, Instance) and new_fragment.type in ("$memrd", "$memwr"):
-            clk_port, clk_dir = new_fragment.named_ports["CLK"]
-            if isinstance(clk_port, ClockSignal) and clk_port.domain in self.controls:
-                en_port, en_dir = new_fragment.named_ports["EN"]
-                en_port = Mux(self.controls[clk_port.domain], en_port, Const(0, len(en_port)))
-                new_fragment.named_ports["EN"] = en_port, en_dir
+        if isinstance(new_fragment, Instance) and new_fragment.type == "$mem_v2":
+            for kind in ["RD", "WR"]:
+                clk_parts = new_fragment.named_ports[kind + "_CLK"][0].parts
+                en_parts = new_fragment.named_ports[kind + "_EN"][0].parts
+                new_en = []
+                for clk, en in zip(clk_parts, en_parts):
+                    if isinstance(clk, ClockSignal) and clk.domain in self.controls:
+                        en = Mux(self.controls[clk.domain], en, Const(0, len(en)))
+                    new_en.append(en)
+                new_fragment.named_ports[kind + "_EN"] = Cat(new_en), "i"
         return new_fragment