Merge pull request #1091 from enjoy-digital/memory-dev

fhdl/memory: Improve generation code and avoid specific generation for Efinix FPGAs.

litex/build/efinix/efinity.py

@@ -18,12 +18,12 @@ import datetime
 from xml.dom import expatbuilder
 import xml.etree.ElementTree as et
 
-from litex.build.generic_platform import *
-
 from migen.fhdl.structure import _Fragment
 from migen.fhdl.tools import *
 from migen.fhdl.namer import build_namespace
+from migen.fhdl.simplify import FullMemoryWE
 
+from litex.build.generic_platform import *
 from litex.build.generic_platform import Pins, IOStandard, Misc
 from litex.build import tools
 
@@ -268,6 +268,9 @@ class EfinityToolchain:
         os.makedirs(build_dir, exist_ok=True)
         os.chdir(build_dir)
 
+        # Apply FullMemoryWE on design (Efiniy does not infer memories correctly otherwise).
+        FullMemoryWE()(fragment)
+
         # Finalize design
         if not isinstance(fragment, _Fragment):
             fragment = fragment.get_fragment()

litex/gen/fhdl/memory.py

@@ -1,3 +1,10 @@
+#
+# This file is part of LiteX (Adapted from Migen for LiteX usage).
+#
+# This file is Copyright (c) 2013-2014 Sebastien Bourdeauducq <sb@m-labs.hk>
+# This file is Copyright (c) 2021 Florent Kermarrec <florent@enjoy-digital.fr>
+# SPDX-License-Identifier: BSD-2-Clause
+
 from migen.fhdl.structure import *
 from migen.fhdl.module import *
 from migen.fhdl.bitcontainer import bits_for
@@ -6,91 +13,143 @@ from migen.fhdl.verilog import _printexpr as verilog_printexpr
 from migen.fhdl.specials import *
 
 def memory_emit_verilog(memory, ns, add_data_file):
-    r = ""
+    # Helpers.
+    # --------
     def gn(e):
         if isinstance(e, Memory):
             return ns.get_name(e)
         else:
             return verilog_printexpr(ns, e)[0]
-    adrbits = bits_for(memory.depth-1)
-
-    r += "reg [" + str(memory.width-1) + ":0] " \
-        + gn(memory) \
-        + "[0:" + str(memory.depth-1) + "];\n"
 
+    # Parameters.
+    # -----------
+    r         = ""
     adr_regs  = {}
     data_regs = {}
 
-    # https://github.com/enjoy-digital/litex/issues/1003
+    # Ports Transformations.
-    # FIXME: Verify behaviour with the different FPGA toolchains.
+    # ----------------------
+
+    # Set Port Mode to Read-First when several Ports with different Clocks.
+    # FIXME: Verify behaviour with the different FPGA toolchains, try to avoid it.
     clocks = [port.clock for port in memory.ports]
     if clocks.count(clocks[0]) != len(clocks):
         for port in memory.ports:
             port.mode = READ_FIRST
 
+    # Set Port Granularity when 0.
     for port in memory.ports:
-        if not port.async_read:
+        if port.we_granularity == 0:
-            if port.mode == WRITE_FIRST:
+            port.we_granularity = memory.width
-                adr_reg = Signal(name_override="memadr")
-                r += "reg [" + str(adrbits-1) + ":0] " \
-                    + gn(adr_reg) + ";\n"
-                adr_regs[id(port)] = adr_reg
-            else:
-                data_reg = Signal(name_override="memdat")
-                r += "reg [" + str(memory.width-1) + ":0] " \
-                    + gn(data_reg) + ";\n"
-                data_regs[id(port)] = data_reg
 
-    for port in memory.ports:
+    # Memory Description.
-        r += "always @(posedge " + gn(port.clock) + ") begin\n"
+    # -------------------
-        if port.we is not None:
+    r += "//" + "-"*80 + "\n"
-            if port.we_granularity:
+    r += f"// Memory {gn(memory)}: {memory.depth}-words x {memory.width}-bit\n"
-                n = memory.width//port.we_granularity
+    r += "//" + "-"*80 + "\n"
-                for i in range(n):
+    for n, port in enumerate(memory.ports):
-                    m = i*port.we_granularity
+        r += f"// Port {n} | "
-                    M = (i+1)*port.we_granularity-1
+        if port.async_read:
-                    sl = "[" + str(M) + ":" + str(m) + "]"
+            r += "Read: Async | "
-                    r += "\tif (" + gn(port.we) + "[" + str(i) + "])\n"
-                    r += "\t\t" + gn(memory) + "[" + gn(port.adr) + "]" + sl + " <= " + gn(port.dat_w) + sl + ";\n"
         else:
-                r += "\tif (" + gn(port.we) + ")\n"
+            r += "Read: Sync  | "
-                r += "\t\t" + gn(memory) + "[" + gn(port.adr) + "] <= " + gn(port.dat_w) + ";\n"
+        if port.we is None:
-        if not port.async_read:
+            r += "Write: ---- | "
+        else:
+            r += "Write: Sync | "
+            r += "Mode: "
             if port.mode == WRITE_FIRST:
-                rd = "\t" + gn(adr_regs[id(port)]) + " <= " + gn(port.adr) + ";\n"
+                r += "Write-First | "
-            else:
+            elif port.mode == READ_FIRST:
-                bassign = gn(data_regs[id(port)]) + " <= " + gn(memory) + "[" + gn(port.adr) + "];\n"
+                r += "Read-First  | "
-                if port.mode == READ_FIRST:
-                    rd = "\t" + bassign
             elif port.mode == NO_CHANGE:
-                    rd = "\tif (!" + gn(port.we) + ")\n" \
+                r += "No-Change | "
-                      + "\t\t" + bassign
+            r += f"Write-Granularity: {port.we_granularity} "
+        r += "\n"
+
+    # Memory Logic Declaration/Initialization.
+    # ----------------------------------------
+    r += f"reg [{memory.width-1}:0] {gn(memory)}[0:{memory.depth-1}];\n"
+    if memory.init is not None:
+        content = ""
+        formatter = f"{{:0{int(memory.width/4)}x}}\n"
+        for d in memory.init:
+            content += formatter.format(d)
+        memory_filename = add_data_file(f"{gn(memory)}.init", content)
+
+        r += "initial begin\n"
+        r += f"\t$readmemh(\"{memory_filename}\", {gn(memory)});\n"
+        r += "end\n"
+
+    # Port Intermediate Signals.
+    # --------------------------
+    for n, port in enumerate(memory.ports):
+        # No Intermediate Signal for Async Read.
+        if port.async_read:
+            continue
+
+        # Create Address Register in Write-First mode.
+        if port.mode in [WRITE_FIRST]:
+            adr_regs[n] = Signal(name_override=f"{gn(memory)}_adr{n}")
+            r += f"reg [{bits_for(memory.depth-1)-1}:0] {gn(adr_regs[n])};\n"
+
+        # Create Data Register in Read-First/No Change mode.
+        if port.mode in [READ_FIRST, NO_CHANGE]:
+            data_regs[n] = Signal(name_override=f"{gn(memory)}_dat{n}")
+            r += f"reg [{memory.width-1}:0] {gn(data_regs[n])};\n"
+
+    # Ports Write/Read Logic.
+    # -----------------------
+    for n, port in enumerate(memory.ports):
+        r += f"always @(posedge {gn(port.clock)}) begin\n"
+        # Write Logic.
+        if port.we is not None:
+            # Split Write Logic.
+            for i in range(memory.width//port.we_granularity):
+                wbit = f"[{i}]" if memory.width != port.we_granularity else ""
+                r += f"\tif ({gn(port.we)}{wbit})\n"
+                lbit =     i*port.we_granularity
+                hbit = (i+1)*port.we_granularity-1
+                r += f"\t\t{gn(memory)}[{gn(port.adr)}][{hbit}:{lbit}] <= {gn(port.dat_w)}[{hbit}:{lbit}];\n"
+
+        # Read Logic.
+        if not port.async_read:
+            # In Write-First mode, Read from Address Register.
+            if port.mode in [WRITE_FIRST]:
+                rd = f"\t{gn(adr_regs[n])} <= {gn(port.adr)};\n"
+
+            # In Read-First/No Change mode:
+            if port.mode in [READ_FIRST, NO_CHANGE]:
+                rd = ""
+                # Only Read in No-Change mode when no Write.
+                if port.mode == NO_CHANGE:
+                    rd += f"\tif (!{gn(port.we)})\n\t"
+                # Read-First/No-Change Read logic.
+                rd += f"\t{gn(data_regs[n])} <= {gn(memory)}[{gn(port.adr)}];\n"
+
+            # Add Read-Enable Logic.
             if port.re is None:
                 r += rd
             else:
-                r += "\tif (" + gn(port.re) + ")\n"
+                r += f"\tif ({gn(port.re)})\n"
                 r += "\t" + rd.replace("\n\t", "\n\t\t")
-        r += "end\n\n"
+        r += "end\n"
 
-    for port in memory.ports:
+    # Ports Read Mapping.
+    # -------------------
+    for n, port in enumerate(memory.ports):
+        # Direct (Asynchronous) Read on Async-Read mode.
         if port.async_read:
-            r += "assign " + gn(port.dat_r) + " = " + gn(memory) + "[" + gn(port.adr) + "];\n"
+            r += f"assign {gn(port.dat_r)} = {gn(memory)}[{gn(port.adr)}];\n"
-        else:
+            continue
-            if port.mode == WRITE_FIRST:
-                r += "assign " + gn(port.dat_r) + " = " + gn(memory) + "[" + gn(adr_regs[id(port)]) + "];\n"
-            else:
-                r += "assign " + gn(port.dat_r) + " = " + gn(data_regs[id(port)]) + ";\n"
-    r += "\n"
 
-    if memory.init is not None:
+        # Write-First mode: Do Read through Address Register.
-        content = ""
+        if port.mode in [WRITE_FIRST]:
-        formatter = "{:0" + str(int(memory.width / 4)) + "X}\n"
+            r += f"assign {gn(port.dat_r)} = {gn(memory)}[{gn(adr_regs[n])}];\n"
-        for d in memory.init:
-            content += formatter.format(d)
-        memory_filename = add_data_file(gn(memory) + ".init", content)
 
-        r += "initial begin\n"
+        # Read-First/No-Change mode: Data already Read on Data Register.
-        r += "\t$readmemh(\"" + memory_filename + "\", " + gn(memory) + ");\n"
+        if port.mode in [READ_FIRST, NO_CHANGE]:
-        r += "end\n\n"
+             r += f"assign {gn(port.dat_r)} = {gn(data_regs[n])};\n"
+    r +=  "//" + "-"*80 + "\n\n"
 
     return r

litex/gen/fhdl/memory_efinix.py

@@ -1,132 +0,0 @@
-from migen.fhdl.structure import *
-from migen.fhdl.module import *
-from migen.fhdl.bitcontainer import bits_for
-from migen.fhdl.tools import *
-from migen.fhdl.verilog import _printexpr as verilog_printexpr
-from migen.fhdl.specials import *
-
-def memory_emit_verilog(memory, ns, add_data_file):
-    r = ""
-    def gn(e):
-        if isinstance(e, Memory):
-            return ns.get_name(e)
-        else:
-            return verilog_printexpr(ns, e)[0]
-    adrbits = bits_for(memory.depth-1)
-
-    for i in range(memory.width // 8):
-        r += "reg [" + str((memory.width//4)-1) + ":0] " \
-        + gn(memory) + '_efx_' + str(i) \
-        + "[0:" + str(memory.depth-1) + "];\n"
-
-    adr_regs = {}
-    data_regs = {}
-    for port in memory.ports:
-        if not port.async_read:
-            if port.mode == WRITE_FIRST:
-                adr_reg = Signal(name_override="memadr")
-                r += "reg [" + str(adrbits-1) + ":0] " \
-                    + gn(adr_reg) + ";\n"
-                adr_regs[id(port)] = adr_reg
-            else:
-                data_reg = Signal(name_override="memdat")
-                r += "reg [" + str(memory.width-1) + ":0] " \
-                    + gn(data_reg) + ";\n"
-                data_regs[id(port)] = data_reg
-
-    for port in memory.ports:
-        r += "always @(posedge " + gn(port.clock) + ") begin\n"
-        if port.we is not None:
-            if port.we_granularity:
-                n = memory.width//port.we_granularity
-                for i in range(n):
-                    if (i > 0):
-                        r += "always @(posedge " + gn(port.clock) + ") begin\n"
-                    m = i*port.we_granularity
-                    M = (i+1)*port.we_granularity-1
-                    sl = "[" + str(M) + ":" + str(m) + "]"
-                    r += "\tif (" + gn(port.we) + "[" + str(i) + "])\n"
-                    r += "\t\t" + gn(memory) + '_efx_' + str(i) + "[" + gn(port.adr) + "]" + " <= " + gn(port.dat_w) + sl + ";\n"
-                    r += "end\n"
-            else:
-                r += "\tif (" + gn(port.we) + ")\n"
-                r += "\t\t" + gn(memory) + "[" + gn(port.adr) + "] <= " + gn(port.dat_w) + ";\n"
-        if not port.async_read:
-            if port.mode == WRITE_FIRST:
-                r += "always @(posedge " + gn(port.clock) + ") begin\n"
-                rd = "\t" + gn(adr_regs[id(port)]) + " <= " + gn(port.adr) + ";\n"
-            else:
-                bassign = ""
-                for i in range(memory.width // 8):
-                    m = i*port.we_granularity
-                    M = (i+1)*port.we_granularity-1
-                    sl = "[" + str(M) + ":" + str(m) + "]"
-                    bassign += gn(data_regs[id(port)]) + sl + " <= " + gn(memory) + "_efx_" + str(i) + "[" + gn(port.adr) + "];\n"
-                if port.mode == READ_FIRST:
-                    rd = "\t" + bassign
-                elif port.mode == NO_CHANGE:
-                    rd = "\tif (!" + gn(port.we) + ")\n" \
-                      + "\t\t" + bassign
-            if port.re is None:
-                r += rd
-            else:
-                r += "\tif (" + gn(port.re) + ")\n"
-                r += "\t" + rd.replace("\n\t", "\n\t\t")
-        r += "end\n\n"
-
-    for port in memory.ports:
-        if port.async_read:
-                r += "assign " + gn(port.dat_r) + " = " + gn(memory) + "[" + gn(port.adr) + "];\n"
-        else:
-            if port.mode == WRITE_FIRST:
-                for i in range(memory.width // 8):
-                    m = i*port.we_granularity
-                    M = (i+1)*port.we_granularity-1
-                    sl = "[" + str(M) + ":" + str(m) + "]"
-                    r += "assign " + gn(port.dat_r) + sl + " = " + gn(memory) + "_efx_" + str(i) + "[" + gn(adr_regs[id(port)]) + "];\n"
-            else:
-                r += "assign " + gn(port.dat_r) + " = " + gn(data_regs[id(port)]) + ";\n"
-    r += "\n"
-
-    if memory.init is not None:
-        content_7_0 = ""
-        content_15_8 = ""
-        content_23_16 = ""
-        content_31_24 = ""
-        formatter = "{:0" + str(int(memory.width / 4)) + "X}\n"
-
-        init_7_0 = []
-        init_15_8 = []
-        init_23_16 = []
-        init_31_24 = []
-
-        for w in memory.init:
-            init_7_0.append(w & 0xff)
-            init_15_8.append((w >> 8) & 0xff)
-            init_23_16.append((w >> 16) & 0xff)
-            init_31_24.append((w >> 24) & 0xff)
-
-        for d in init_7_0:
-            content_7_0 += formatter.format(d)
-
-        for d in init_15_8:
-            content_15_8 += formatter.format(d)
-
-        for d in init_23_16:
-            content_23_16 += formatter.format(d)
-
-        for d in init_31_24:
-            content_31_24 += formatter.format(d)
-
-        memory_filename1 = add_data_file(gn(memory) + "_efx_1.init", content_7_0)
-        memory_filename2 = add_data_file(gn(memory) + "_efx_2.init", content_15_8)
-        memory_filename3 = add_data_file(gn(memory) + "_efx_3.init", content_23_16)
-        memory_filename4 = add_data_file(gn(memory) + "_efx_4.init", content_31_24)
-        r += "initial begin\n"
-        r += "\t$readmemh(\"" + memory_filename1 + "\", " + gn(memory)+ "_efx_0" + ");\n"
-        r += "\t$readmemh(\"" + memory_filename2 + "\", " + gn(memory)+ "_efx_1" + ");\n"
-        r += "\t$readmemh(\"" + memory_filename3 + "\", " + gn(memory)+ "_efx_2" + ");\n"
-        r += "\t$readmemh(\"" + memory_filename4 + "\", " + gn(memory)+ "_efx_3" + ");\n"
-        r += "end\n\n"
-
-    return r

litex/gen/fhdl/verilog.py

@@ -439,13 +439,8 @@ def _print_specials(overrides, specials, ns, add_data_file, attr_translate):
             attr = _print_attribute(special.attr, attr_translate)
             if attr:
                 r += attr + " "
-        # Replace Migen Memory's emit_verilog with our implementation.
+        # Replace Migen Memory's emit_verilog with LiteX's implementation.
         if isinstance(special, Memory):
-            from litex.build.efinix.platform import EfinixPlatform
-            if isinstance(special.platform, EfinixPlatform) and (special.width == 32): # FIXME: Improve.
-                from litex.gen.fhdl.memory_efinix import memory_emit_verilog
-                pr = memory_emit_verilog(special, ns, add_data_file)
-            else:
             from litex.gen.fhdl.memory import memory_emit_verilog
             pr = memory_emit_verilog(special, ns, add_data_file)
         else: