amaranth icon indicating copy to clipboard operation
amaranth copied to clipboard

Clock constraint on custom sync domain clock signal causes error during template rendering

Open qookei opened this issue 9 months ago • 3 comments

Adding a clock constraint on a new clock domain called sync causes an error during Jinja template rendering. Taking away platform.add_clock_constraint(cd_sync.clk, 100e6) or renaming the domain makes the error go away. Fails on Amaranth 0.5.4.

I've tried moving the clock constraint line around, or adding an intermediate signal and assigning cd_sync to it before feeding it into add_clock_constraint to no avail.

More or less minimal test case:

from amaranth import *
from amaranth.build import *
from amaranth.vendor import LatticeECP5Platform


class TopLevel(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        cd_sync = ClockDomain("sync")
        m.domains += cd_sync

        platform.add_clock_constraint(cd_sync.clk, 100e6)
        # For context: imagine a PLL that drives cd_sync.clk is instantiated here

        m.d.sync += Signal().eq(1)

        return m


class MyPlatform(LatticeECP5Platform):
    device      = "LFE5U-25F"
    package     = "BG256"
    speed       = "7"
    default_clk = "clk25"

    resources = [
        Resource("clk25", 0, Pins("P6", dir="i"), Clock(25e6), Attrs(IO_TYPE="LVCMOS33")),
    ]

    connectors = []

if __name__ == '__main__':
    MyPlatform().build(TopLevel())

Error:

Traceback (most recent call last):
  File ".../bug.py", line 33, in <module>
    MyPlatform().build(TopLevel())
  File ".../venv/lib/python3.12/site-packages/amaranth/build/plat.py", line 99, in build
    plan = self.prepare(elaboratable, name, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../venv/lib/python3.12/site-packages/amaranth/build/plat.py", line 159, in prepare
    return self.toolchain_prepare(self._design, name, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../venv/lib/python3.12/site-packages/amaranth/build/plat.py", line 403, in toolchain_prepare
    render(content_tpl, origin=content_tpl))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../venv/lib/python3.12/site-packages/amaranth/build/plat.py", line 384, in render
    return compiled.render({
           ^^^^^^^^^^^^^^^^^
  File ".../venv/lib/python3.12/site-packages/jinja2/environment.py", line 1295, in render
    self.environment.handle_exception()
  File ".../venv/lib/python3.12/site-packages/jinja2/environment.py", line 942, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<template>", line 12, in top-level template code
  File ".../venv/lib/python3.12/site-packages/amaranth/build/plat.py", line 340, in hierarchy
    return separator.join(self._name_map[net][1:])
                          ~~~~~~~~~~~~~~^^^^^
  File ".../venv/lib/python3.12/site-packages/amaranth/hdl/_ast.py", line 3219, in __getitem__
    key = None if key is None else self._map_key(key)
                                   ^^^^^^^^^^^^^^^^^^
  File ".../venv/lib/python3.12/site-packages/amaranth/hdl/_ast.py", line 3300, in __init__
    raise TypeError(f"Object {signal!r} is not an Amaranth signal")
TypeError: Object Undefined is not an Amaranth signal

qookei avatar Feb 12 '25 02:02 qookei

I wouldn't expect your reduced testcase to work since the signal is completely undriven. (We should have a better error message but it will still be a crash.)

Could you make a more complete minimal example with e.g. a PLL please?

whitequark avatar Feb 12 '25 02:02 whitequark

Sure. Also includes an LED to try and see if the design works. Produces the same error as the code above.

If I take away platform.add_clock_constraint(cd_sync.clk, 100e6) it synthesizes and appears to run correctly, and nextpnr derives a 100MHz constraint for the clock (based on the PLL parameters I assume).

from amaranth import *
from amaranth.build import *
from amaranth.vendor import LatticeECP5Platform
from amaranth_boards.resources import *


class TopLevel(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        cd_sync = ClockDomain("sync")
        m.domains += cd_sync
        platform.add_clock_constraint(cd_sync.clk, 100e6)

        m.submodules.pll = Instance(
            "EHXPLLL",

            a_FREQUENCY_PIN_CLKI="25",
            a_FREQUENCY_PIN_CLKOP="100",
            a_ICP_CURRENT="12",
            a_LPF_RESISTOR="8",
            p_CLKI_DIV=1,
            p_CLKOP_DIV=6,
            p_CLKFB_DIV=4,
            p_FEEDBK_PATH="CLKOP",
            p_CLKOP_ENABLE="ENABLED",

            i_CLKI=platform.request("clk25").i,

            o_CLKOP=cd_sync.clk,
            i_CLKFB=cd_sync.clk,
        )

        led = platform.request("led")
        m.d.sync += led.o.eq(~led.o)

        return m


class MyPlatform(LatticeECP5Platform):
    device      = "LFE5U-25F"
    package     = "BG256"
    speed       = "7"
    default_clk = "clk25"

    resources = [
        Resource("clk25", 0, Pins("P6", dir="i"), Clock(25e6), Attrs(IO_TYPE="LVCMOS33")),
        *LEDResources(pins="T6", invert=True,
                      attrs=Attrs(IO_TYPE="LVCMOS33", DRIVE="4")),
    ]

    connectors = []

if __name__ == '__main__':
    MyPlatform().build(TopLevel())

qookei avatar Feb 12 '25 02:02 qookei

Thanks. As a workaround you should be able to rely on nextpnr inferring a constraint from the PLL parameters since this is supposed to be reliable. We will take a look at the issue preventing add_clock_constraint from working.

whitequark avatar Feb 12 '25 02:02 whitequark