Repository: amaranth-lang/amaranth
Pull request #1177 (2024-02-29 19:55:11)

A batch of platform-related fixes
- Change SDC hierarchy separator to `/` explicitly on Lattice platforms from the `.` Synopsys default (Intel uses `|` and Xilinx already sets it to `/`);
- Fix IO buffer factories on MachXO platforms;
- Remove `tcl_escape` which cannot fulfill its contract (it cannot correctly escape the string `\` with `{}` quotes) and use `tcl_quote` instead.


Patch:
diff --git a/amaranth/build/plat.py b/amaranth/build/plat.py
index 4880f67..6844b8d 100644
--- a/amaranth/build/plat.py
+++ b/amaranth/build/plat.py
@@ -423,9 +423,6 @@ class TemplatedPlatform(Platform):
                     return f"_{ord(match.group(1)[0]):02x}_"
             return "".join(escape_one(m) for m in re.finditer(r"([^A-Za-z0-9_])|(.)", string))
 
-        def tcl_escape(string):
-            return "{" + re.sub(r"([{}\\])", r"\\\1", string) + "}"
-
         def tcl_quote(string):
             return '"' + re.sub(r"([$[\\])", r"\\\1", string) + '"'
 
@@ -449,7 +446,6 @@ class TemplatedPlatform(Platform):
                 compiled.environment.filters["options"] = options
                 compiled.environment.filters["hierarchy"] = hierarchy
                 compiled.environment.filters["ascii_escape"] = ascii_escape
-                compiled.environment.filters["tcl_escape"] = tcl_escape
                 compiled.environment.filters["tcl_quote"] = tcl_quote
             except jinja2.TemplateSyntaxError as e:
                 e.args = (f"{e.message} (at {origin}:{e.lineno})",)
diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py
index 5f63f56..2eed406 100644
--- a/amaranth/hdl/_ir.py
+++ b/amaranth/hdl/_ir.py
@@ -581,13 +581,14 @@ class Design:
         """
         self.fragment_names[fragment] = hierarchy
 
-        signal_names = set(self.signal_names[fragment].values())
+        taken_names = set(self.signal_names[fragment].values())
         for subfragment_index, (subfragment, subfragment_name, subfragment_src_loc) in enumerate(fragment.subfragments):
             if subfragment_name is None:
                 subfragment_name = f"U${subfragment_index}"
-            elif subfragment_name in signal_names:
+            elif subfragment_name in taken_names:
                 subfragment_name = f"{subfragment_name}$U${subfragment_index}"
-            assert subfragment_name not in signal_names
+            assert subfragment_name not in taken_names
+            taken_names.add(subfragment_name)
             self._assign_names_to_fragments(subfragment, hierarchy=(*hierarchy, subfragment_name))
 
 
diff --git a/amaranth/vendor/_gowin.py b/amaranth/vendor/_gowin.py
index 9409cf2..0ad3f13 100644
--- a/amaranth/vendor/_gowin.py
+++ b/amaranth/vendor/_gowin.py
@@ -288,7 +288,7 @@ class GowinPlatform(TemplatedPlatform):
         "{{name}}.sdc": r"""
         // {{autogenerated}}
         {% for net_signal,port_signal,frequency in platform.iter_clock_constraints() -%}
-            create_clock -name {{port_signal.name|tcl_escape}} -period {{1000000000/frequency}}  [get_ports {{port_signal.name|tcl_escape}}]
+            create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
         {% endfor %}
         {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
         """,
diff --git a/amaranth/vendor/_lattice_ecp5.py b/amaranth/vendor/_lattice_ecp5.py
index 10c8e58..bb63537 100644
--- a/amaranth/vendor/_lattice_ecp5.py
+++ b/amaranth/vendor/_lattice_ecp5.py
@@ -206,7 +206,7 @@ class LatticeECP5Platform(TemplatedPlatform):
                 -lpf {{name}}.lpf \
                 -synthesis synplify
             {% for file in platform.iter_files(".v", ".sv", ".vhd", ".vhdl") -%}
-                prj_src add {{file|tcl_escape}}
+                prj_src add {{file|tcl_quote}}
             {% endfor %}
             prj_src add {{name}}.v
             prj_impl option top {{name}}
@@ -234,11 +234,12 @@ class LatticeECP5Platform(TemplatedPlatform):
             {{get_override("add_preferences")|default("# (add_preferences placeholder)")}}
         """,
         "{{name}}.sdc": r"""
+            set_hierarchy_separator {/}
             {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
                 {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_escape}}]
+                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_escape}}]
+                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
diff --git a/amaranth/vendor/_lattice_ice40.py b/amaranth/vendor/_lattice_ice40.py
index 0894025..2d48777 100644
--- a/amaranth/vendor/_lattice_ice40.py
+++ b/amaranth/vendor/_lattice_ice40.py
@@ -216,7 +216,7 @@ class LatticeICE40Platform(TemplatedPlatform):
         "{{name}}_syn.prj": r"""
             # {{autogenerated}}
             {% for file in platform.iter_files(".v", ".sv", ".vhd", ".vhdl") -%}
-                add_file -verilog {{file|tcl_escape}}
+                add_file -verilog {{file|tcl_quote}}
             {% endfor %}
             add_file -verilog {{name}}.v
             add_file -constraint {{name}}.sdc
@@ -236,11 +236,12 @@ class LatticeICE40Platform(TemplatedPlatform):
         """,
         "{{name}}.sdc": r"""
             # {{autogenerated}}
+            set_hierarchy_separator {/}
             {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
                 {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_escape}}]
+                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_escape}}]
+                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
diff --git a/amaranth/vendor/_lattice_machxo_2_3l.py b/amaranth/vendor/_lattice_machxo_2_3l.py
index ef219bd..ae94940 100644
--- a/amaranth/vendor/_lattice_machxo_2_3l.py
+++ b/amaranth/vendor/_lattice_machxo_2_3l.py
@@ -72,7 +72,7 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
                 -lpf {{name}}.lpf \
                 -synthesis synplify
             {% for file in platform.iter_files(".v", ".sv", ".vhd", ".vhdl") -%}
-                prj_src add {{file|tcl_escape}}
+                prj_src add {{file|tcl_quote}}
             {% endfor %}
             prj_src add {{name}}.v
             prj_impl option top {{name}}
@@ -101,11 +101,12 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
             {{get_override("add_preferences")|default("# (add_preferences placeholder)")}}
         """,
         "{{name}}.sdc": r"""
+            set_hierarchy_separator {/}
             {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
                 {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_escape}}]
+                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_escape}}]
+                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
@@ -339,7 +340,7 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
                             valid_xdrs=(0, 1, 2), valid_attrs=True)
         m = Module()
         i, o, t = self._get_xdr_buffer(m, pin, i_invert=invert)
-        for bit in range(len(port)):
+        for bit in range(pin.width):
             m.submodules[f"{pin.name}_{bit}"] = Instance("IB",
                 i_I=port.io[bit],
                 o_O=i[bit]
@@ -351,7 +352,7 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
                             valid_xdrs=(0, 1, 2), valid_attrs=True)
         m = Module()
         i, o, t = self._get_xdr_buffer(m, pin, o_invert=invert)
-        for bit in range(len(port)):
+        for bit in range(pin.width):
             m.submodules[f"{pin.name}_{bit}"] = Instance("OB",
                 i_I=o[bit],
                 o_O=port.io[bit]
@@ -363,7 +364,7 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
                             valid_xdrs=(0, 1, 2), valid_attrs=True)
         m = Module()
         i, o, t = self._get_xdr_buffer(m, pin, o_invert=invert)
-        for bit in range(len(port)):
+        for bit in range(pin.width):
             m.submodules[f"{pin.name}_{bit}"] = Instance("OBZ",
                 i_T=t,
                 i_I=o[bit],
@@ -376,7 +377,7 @@ class LatticeMachXO2Or3LPlatform(TemplatedPlatform):
                             valid_xdrs=(0, 1, 2), valid_attrs=True)
         m = Module()
         i, o, t = self._get_xdr_buffer(m, pin, i_invert=invert, o_invert=invert)
-        for bit in range(len(port)):
+        for bit in range(pin.width):
             m.submodules[f"{pin.name}_{bit}"] = Instance("BB",
                 i_T=t,
                 i_I=o[bit],
diff --git a/amaranth/vendor/_quicklogic.py b/amaranth/vendor/_quicklogic.py
index 697ec4c..33a490e 100644
--- a/amaranth/vendor/_quicklogic.py
+++ b/amaranth/vendor/_quicklogic.py
@@ -64,7 +64,7 @@ class QuicklogicPlatform(TemplatedPlatform):
             # {{autogenerated}}
             {% for port_name, pin_name, attrs in platform.iter_port_constraints_bits() -%}
                 {% for attr_name, attr_value in attrs.items() -%}
-                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_escape}} }]
+                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_quote}} }]
                 {% endfor %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
diff --git a/amaranth/vendor/_xilinx.py b/amaranth/vendor/_xilinx.py
index 9d6fa7f..1475649 100644
--- a/amaranth/vendor/_xilinx.py
+++ b/amaranth/vendor/_xilinx.py
@@ -158,12 +158,12 @@ class XilinxPlatform(TemplatedPlatform):
             # {{autogenerated}}
             create_project -force -name {{name}} -part {{platform._part}}
             {% for file in platform.iter_files(".v", ".sv", ".vhd", ".vhdl") -%}
-                add_files {{file|tcl_escape}}
+                add_files {{file|tcl_quote}}
             {% endfor %}
             add_files {{name}}.v
             read_xdc {{name}}.xdc
             {% for file in platform.iter_files(".xdc") -%}
-                read_xdc {{file|tcl_escape}}
+                read_xdc {{file|tcl_quote}}
             {% endfor %}
             {{get_override("script_after_read")|default("# (script_after_read placeholder)")}}
             synth_design -top {{name}} {{get_override("synth_design_opts")}}
@@ -218,16 +218,16 @@ class XilinxPlatform(TemplatedPlatform):
         "{{name}}.xdc": r"""
             # {{autogenerated}}
             {% for port_name, pin_name, attrs in platform.iter_port_constraints_bits() -%}
-                set_property LOC {{pin_name}} [get_ports {{port_name|tcl_escape}}]
+                set_property LOC {{pin_name}} [get_ports {{port_name|tcl_quote}}]
                 {% for attr_name, attr_value in attrs.items() -%}
-                    set_property {{attr_name}} {{attr_value|tcl_escape}} [get_ports {{port_name|tcl_escape}}]
+                    set_property {{attr_name}} {{attr_value|tcl_quote}} [get_ports {{port_name|tcl_quote}}]
                 {% endfor %}
             {% endfor %}
             {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
                 {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|ascii_escape}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_escape}}]
+                    create_clock -name {{port_signal.name|ascii_escape}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|ascii_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_escape}}]
+                    create_clock -name {{net_signal.name|ascii_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
@@ -417,7 +417,7 @@ class XilinxPlatform(TemplatedPlatform):
             # {{autogenerated}}
             {% for port_name, pin_name, attrs in platform.iter_port_constraints_bits() -%}
                 {% for attr_name, attr_value in attrs.items() -%}
-                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_escape}} }]
+                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_quote}} }]
                 {% endfor %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
@@ -529,8 +529,8 @@ class XilinxPlatform(TemplatedPlatform):
             # {{autogenerated}}
             {% for port_name, pin_name, attrs in platform.iter_port_constraints_bits() -%}
                 {% for attr_name, attr_value in attrs.items() -%}
-                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_escape}}]
-                    set_property LOC {{pin_name}} [get_ports {{port_name|tcl_escape}}]
+                    set_property {{attr_name}} {{attr_value}} [get_ports {{port_name|tcl_quote}}]
+                    set_property LOC {{pin_name}} [get_ports {{port_name|tcl_quote}}]
                 {% endfor %}
             {% endfor %}
         """,


Test patch:
diff --git a/tests/test_hdl_ir.py b/tests/test_hdl_ir.py
index bfd63f5..299344a 100644
--- a/tests/test_hdl_ir.py
+++ b/tests/test_hdl_ir.py
@@ -1030,6 +1030,18 @@ class NamesTestCase(FHDLTestCase):
             a_f: ("top", "a$U$0")
         })
 
+    def test_assign_names_to_fragments_duplicate(self):
+        f = Fragment()
+        f.add_subfragment(a1_f := Fragment(), name="a")
+        f.add_subfragment(a2_f := Fragment(), name="a")
+
+        design = Design(f, ports=[], hierarchy=("top",))
+        self.assertEqual(design.fragment_names, {
+            f: ("top",),
+            a1_f: ("top", "a"),
+            a2_f: ("top", "a$U$1"),
+        })
+
 
 class ElaboratesTo(Elaboratable):
     def __init__(self, lower):