build: add clock constraints on IOPort instead of buffered Signal.

Author: wanda-phi

amaranth/build/plat.py

@@ -199,13 +199,13 @@ class TemplatedPlatform(Platform):
     }
 
     def iter_clock_constraints(self):
-        for net_signal, port_signal, frequency in super().iter_clock_constraints():
+        for net, frequency in super().iter_clock_constraints():
             # Skip any clock constraints placed on signals that are never used in the design.
             # Otherwise, it will cause a crash in the vendor platform if it supports clock
             # constraints on non-port nets.
-            if net_signal not in self._name_map:
+            if isinstance(net, Signal) and net not in self._name_map:
                 continue
-            yield net_signal, port_signal, frequency
+            yield net, frequency
 
     def toolchain_prepare(self, fragment, name, *, emit_src=True, **kwargs):
         # Restrict the name of the design to a strict alphanumeric character set. Platforms will
@@ -327,8 +327,11 @@ def options(opts):
             else:
                 return " ".join(opts)
 
-        def hierarchy(signal, separator):
-            return separator.join(self._name_map[signal][1:])
+        def hierarchy(net, separator):
+            if isinstance(net, IOPort):
+                return net.name
+            else:
+                return separator.join(self._name_map[net][1:])
 
         def ascii_escape(string):
             def escape_one(match):

amaranth/build/res.py

@@ -109,8 +109,9 @@ def __init__(self, resources, connectors):
 
         # List of (pin, port, buffer) pairs for non-dir="-" requests.
         self._pins      = []
-        # Constraint list
+        # Constraint lists
         self._clocks    = SignalDict()
+        self._io_clocks = {}
 
         self.add_resources(resources)
         self.add_connectors(connectors)
@@ -219,6 +220,8 @@ def resolve(resource, dir, xdr, path, attrs):
                         for name in phys_names
                     ])
                     port = io.SingleEndedPort(iop, invert=phys.invert, direction=direction)
+                    if resource.clock is not None:
+                        self.add_clock_constraint(iop, resource.clock.frequency)
                 if isinstance(phys, DiffPairs):
                     phys_names_p = phys.p.map_names(self._conn_pins, resource)
                     phys_names_n = phys.n.map_names(self._conn_pins, resource)
@@ -232,7 +235,8 @@ def resolve(resource, dir, xdr, path, attrs):
                         for name in phys_names_n
                     ])
                     port = io.DifferentialPort(p, n, invert=phys.invert, direction=direction)
-
+                    if resource.clock is not None:
+                        self.add_clock_constraint(p, resource.clock.frequency)
                 for phys_name in phys_names:
                     if phys_name in self._phys_reqd:
                         raise ResourceError("Resource component {} uses physical pin {}, but it "
@@ -253,8 +257,6 @@ def resolve(resource, dir, xdr, path, attrs):
                     buffer = PinBuffer(pin, port)
                     self._pins.append((pin, port, buffer))
 
-                    if resource.clock is not None:
-                        self.add_clock_constraint(pin.i, resource.clock.frequency)
                     return pin
 
             else:
@@ -275,38 +277,26 @@ def add_clock_constraint(self, clock, frequency):
             raise TypeError(f"A clock constraint can only be applied to a Signal, but a "
                             f"ClockSignal is provided; assign the ClockSignal to an "
                             f"intermediate signal and constrain the latter instead.")
-        elif not isinstance(clock, Signal):
-            raise TypeError(f"Object {clock!r} is not a Signal")
+        elif not isinstance(clock, (Signal, IOPort)):
+            raise TypeError(f"Object {clock!r} is not a Signal or IOPort")
         if not isinstance(frequency, (int, float)):
             raise TypeError(f"Frequency must be a number, not {frequency!r}")
 
-        if clock in self._clocks:
+        if isinstance(clock, IOPort):
+            clocks = self._io_clocks
+        else:
+            clocks = self._clocks
+
+        frequency = float(frequency)
+        if clock in clocks and clocks[clock] != frequency:
             raise ValueError("Cannot add clock constraint on {!r}, which is already constrained "
                              "to {} Hz"
-                             .format(clock, self._clocks[clock]))
+                             .format(clock, clocks[clock]))
         else:
-            self._clocks[clock] = float(frequency)
+            clocks[clock] = frequency
 
     def iter_clock_constraints(self):
-        # Back-propagate constraints through the input buffer. For clock constraints on pins
-        # (the majority of cases), toolchains work better if the constraint is defined on the pin
-        # and not on the buffered internal net; and if the toolchain is advanced enough that
-        # it considers clock phase and delay of the input buffer, it is *necessary* to define
-        # the constraint on the pin to match the designer's expectation of phase being referenced
-        # to the pin.
-        #
-        # Constraints on nets with no corresponding input pin (e.g. PLL or SERDES outputs) are not
-        # affected.
-        pin_i_to_port = SignalDict()
-        for pin, port, _fragment in self._pins:
-            if hasattr(pin, "i"):
-                if isinstance(port, io.SingleEndedPort):
-                    pin_i_to_port[pin.i] = port.io
-                elif isinstance(port, io.DifferentialPort):
-                    pin_i_to_port[pin.i] = port.p
-                else:
-                    assert False
-
-        for net_signal, frequency in self._clocks.items():
-            port_signal = pin_i_to_port.get(net_signal)
-            yield net_signal, port_signal, frequency
+        for net, frequency in self._clocks.items():
+            yield net, frequency
+        for net, frequency in self._io_clocks.items():
+            yield net, frequency

amaranth/vendor/_altera.py

@@ -310,11 +310,11 @@ class AlteraPlatform(TemplatedPlatform):
             {{get_override("add_settings")|default("# (add_settings placeholder)")}}
         """,
         "{{name}}.sdc": r"""
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{net.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("|")|tcl_quote}}]
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("|")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}

amaranth/vendor/_gowin.py

@@ -439,8 +439,12 @@ def _osc_div(self):
         """,
         "{{name}}.sdc": r"""
         // {{autogenerated}}
-        {% for net_signal,port_signal,frequency in platform.iter_clock_constraints() -%}
-            create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
+        {% for net, frequency in platform.iter_clock_constraints() -%}
+            {% if net.__class__.__name__ == "IOPort" -%}
+                create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{net.name|tcl_quote}}]
+            {% else -%}
+                create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")|tcl_quote}}]
+            {% endif %}
         {% endfor %}
         {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
         """,

amaranth/vendor/_lattice.py

@@ -497,11 +497,11 @@ class LatticePlatform(TemplatedPlatform):
                     {%- for key, value in attrs.items() %} {{key}}={{value}}{% endfor %};
                 {% endif %}
             {% endfor %}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is not none -%}
-                    FREQUENCY PORT "{{port_signal.name}}" {{frequency}} HZ;
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    FREQUENCY PORT "{{net.name}}" {{frequency}} HZ;
                 {% else -%}
-                    FREQUENCY NET "{{net_signal|hierarchy(".")}}" {{frequency}} HZ;
+                    FREQUENCY NET "{{net|hierarchy(".")}}" {{frequency}} HZ;
                 {% endif %}
             {% endfor %}
             {{get_override("add_preferences")|default("# (add_preferences placeholder)")}}
@@ -584,11 +584,11 @@ class LatticePlatform(TemplatedPlatform):
                 ldc_set_port -iobuf {{ '{' }}{%- for key, value in attrs.items() %}{{key}}={{value}} {% endfor %}{{ '}' }} {{'['}}get_ports {{port_name}}{{']'}}
                 {% endif %}
             {% endfor %}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{net.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_preferences")|default("# (add_preferences placeholder)")}}
@@ -684,11 +684,11 @@ class LatticePlatform(TemplatedPlatform):
         """,
         "{{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_quote("Diamond")}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote("Diamond")}}]
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    create_clock -name {{net.name|tcl_quote("Diamond")}} -period {{1000000000/frequency}} [get_ports {{net.name|tcl_quote("Diamond")}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_quote("Diamond")}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote("Diamond")}}]
+                    create_clock -name {{net.name|tcl_quote("Diamond")}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")|tcl_quote("Diamond")}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
@@ -782,11 +782,11 @@ class LatticePlatform(TemplatedPlatform):
         """,
         # Pre-synthesis SDC constraints
         "{{name}}.sdc": r"""
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is not none -%}
-                    create_clock -name {{port_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name}}]
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{net.name}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")}}]
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}

amaranth/vendor/_quicklogic.py

@@ -71,10 +71,8 @@ class QuicklogicPlatform(TemplatedPlatform):
         """,
         "{{name}}.sdc": r"""
             # {{autogenerated}}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is not none -%}
-                    create_clock -period {{100000000/frequency}} {{port_signal.name|ascii_escape}}
-                {% endif %}
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                create_clock -period {{100000000/frequency}} {{net.name|ascii_escape}}
             {% endfor %}
         """
     }

amaranth/vendor/_siliconblue.py

@@ -134,8 +134,8 @@ class SiliconBluePlatform(TemplatedPlatform):
             {% for port_name, pin_name, attrs in platform.iter_port_constraints_bits() -%}
                 set_io {{port_name}} {{pin_name}}
             {% endfor %}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                set_frequency {{net_signal|hierarchy(".")}} {{frequency/1000000}}
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                set_frequency {{net|hierarchy(".")}} {{frequency/1000000}}
             {% endfor%}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
         """,
@@ -242,11 +242,11 @@ class SiliconBluePlatform(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_quote}} -period {{1000000000/frequency}} [get_ports {{port_signal.name|tcl_quote}}]
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                {% if net.__class__.__name__ == "IOPort" -%}
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_ports {{net.name|tcl_quote}}]
                 {% else -%}
-                    create_clock -name {{net_signal.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
+                    create_clock -name {{net.name|tcl_quote}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")|tcl_quote}}]
                 {% endif %}
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}

amaranth/vendor/_xilinx.py

@@ -647,12 +647,8 @@ def vendor_toolchain(self):
                     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_quote}}]
-                {% else -%}
-                    create_clock -name {{net_signal.name|ascii_escape}} -period {{1000000000/frequency}} [get_nets {{net_signal|hierarchy("/")|tcl_quote}}]
-                {% endif %}
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                create_clock -name {{net.name|ascii_escape}} -period {{1000000000/frequency}} [get_nets {{net|hierarchy("/")|tcl_quote}}]
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
         """
@@ -723,9 +719,9 @@ def vendor_toolchain(self):
                     NET "{{port_name}}" {{attr_name}}={{attr_value}};
                 {% endfor %}
             {% endfor %}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                NET "{{net_signal|hierarchy("/")}}" TNM_NET="PRD{{net_signal|hierarchy("/")}}";
-                TIMESPEC "TS{{net_signal|hierarchy("__")}}"=PERIOD "PRD{{net_signal|hierarchy("/")}}" {{1000000000/frequency}} ns HIGH 50%;
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                NET "{{net|hierarchy("/")}}" TNM_NET="PRD{{net|hierarchy("/")}}";
+                TIMESPEC "TS{{net|hierarchy("__")}}"=PERIOD "PRD{{net|hierarchy("/")}}" {{1000000000/frequency}} ns HIGH 50%;
             {% endfor %}
             {{get_override("add_constraints")|default("# (add_constraints placeholder)")}}
         """
@@ -848,10 +844,8 @@ def _symbiflow_device(self):
         """,
         "{{name}}.sdc": r"""
             # {{autogenerated}}
-            {% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
-                {% if port_signal is none -%}
-                    create_clock -period {{1000000000/frequency}} {{net_signal.name|ascii_escape}}
-                {% endif %}
+            {% for net, frequency in platform.iter_clock_constraints() -%}
+                create_clock -period {{1000000000/frequency}} {{net.name|ascii_escape}}
             {% endfor %}
         """
     }
@@ -1173,7 +1167,8 @@ def create_missing_domain(self, name):
 
     def add_clock_constraint(self, clock, frequency):
         super().add_clock_constraint(clock, frequency)
-        clock.attrs["keep"] = "TRUE"
+        if not isinstance(clock, IOPort):
+            clock.attrs["keep"] = "TRUE"
 
     def get_io_buffer(self, buffer):
         if isinstance(buffer, io.Buffer):

tests/test_build_res.py

@@ -227,16 +227,16 @@ def test_request_clock(self):
         clk100_buffer._MustUse__silence = True
         clk50_buffer._MustUse__silence = True
         self.assertEqual(list(self.cm.iter_clock_constraints()), [
-            (clk100.i, clk100_port.p, 100e6),
-            (clk50.i, clk50_port.io, 50e6)
+            (clk100_port.p, 100e6),
+            (clk50_port.io, 50e6)
         ])
 
     def test_add_clock(self):
         with _ignore_deprecated():
             i2c = self.cm.request("i2c")
         self.cm.add_clock_constraint(i2c.scl.o, 100e3)
         self.assertEqual(list(self.cm.iter_clock_constraints()), [
-            (i2c.scl.o, None, 100e3)
+            (i2c.scl.o, 100e3)
         ])
         ((_, _, scl_buffer), (_, _, sda_buffer)) = self.cm.iter_pins()
         scl_buffer._MustUse__silence = True
@@ -269,7 +269,7 @@ def test_wrong_lookup(self):
 
     def test_wrong_clock_signal(self):
         with self.assertRaisesRegex(TypeError,
-                r"^Object None is not a Signal$"):
+                r"^Object None is not a Signal or IOPort$"):
             self.cm.add_clock_constraint(None, 10e6)
 
     def test_wrong_clock_frequency(self):
@@ -335,10 +335,8 @@ def test_wrong_request_with_xdr_dict(self):
 
     def test_wrong_clock_constraint_twice(self):
         with _ignore_deprecated():
-            clk100 = self.cm.request("clk100")
-        (pin, port, buffer), = self.cm.iter_pins()
-        buffer._MustUse__silence = True
+            clk100 = self.cm.request("clk100", dir="-")
         with self.assertRaisesRegex(ValueError,
-                (r"^Cannot add clock constraint on \(sig clk100_0__i\), which is already "
+                (r"^Cannot add clock constraint on \(io-port clk100_0__p\), which is already "
                     r"constrained to 100000000\.0 Hz$")):
-            self.cm.add_clock_constraint(clk100.i, 1e6)
+            self.cm.add_clock_constraint(clk100.p, 1e6)