platform.add_clock_constraint does not work for instances with vivado

[rroohhh]

For clocks coming from an instance, platform.add_clock_constraint doesn't seem to work, as vivado is unable to find the net that the constraint is assigned to.

Running this example:

from nmigen import *
from nmigen_boards.zturn_lite_z010 import *

class Test(Elaboratable):
    def elaborate(self, plat):
        m = Module()

        fclk = Signal(4)

        m.submodules += Instance("PS7", o_FCLKCLK = fclk)

        clk = Signal()

        m.d.comb += clk.eq(fclk[0])

        m.domains += ClockDomain("sync")
        m.d.comb += ClockSignal().eq(clk)

        plat.add_clock_constraint(clk, 100_000_000)

        return m


plat = ZTurnLiteZ010Platform()

t = Test()

plat.build(t)

causes vivado to issue the following warning:

WARNING: [Vivado 12-507] No nets matched 'clk'. [/data/projects/nmigen_bugs/build/top.xdc:2]

And the subsequent timing report is empty.

Full build directory: build.zip

This is using nmigen commit ba79b0c.

[rroohhh]

Now adding clk.attrs["KEEP"] = "TRUE" makes this example work, however if there are multiple nets with the same name, that still fails for the second net.

Example:

        clk = Signal()
        m.d.comb += clk.eq(fclk[0])

        m.domains += ClockDomain("sync")
        m.d.comb += ClockSignal().eq(clk)

        clk.attrs["KEEP"] = "TRUE"

        plat.add_clock_constraint(clk, 100_000_000)

        clk = Signal()
        m.d.comb += clk.eq(fclk[1])
        clk.attrs["KEEP"] = "TRUE"

        plat.add_clock_constraint(clk, 200_000_000)

This causes the following critical warning in the vivado log:

Parsing XDC File [/data/projects/nmigen_bugs/build/top.xdc]
CRITICAL WARNING: [Common 17-1548] Command failed: can't read "2": no such variable

Full build folder: build.zip

[whitequark]

These are actually two separate bugs, could you please file the 2nd one as another issue? I believe that's a Tcl escaping issue.

[rroohhh]

Alright, sorry for not keeping them seperate.

[whitequark]

Don't blame you--this is immediately obvious to me but it's unlikely to be so to anyone who doesn't maintain the Tcl scripts...

[whitequark]

Ah, #374 is yet another bug you found here :) Nice triple catch.

[whitequark]

There are two issues here. First, the clock net requires an attribute. This can be added with:

--- a/nmigen/vendor/xilinx_7series.py
+++ b/nmigen/vendor/xilinx_7series.py
@@ -177,6 +177,10 @@ class Xilinx7SeriesPlatform(TemplatedPlatform):
                 m.submodules.reset_sync = ResetSynchronizer(rst_i, domain="sync")
             return m
 
+    def add_clock_constraint(self, clock, frequency):
+        super().add_clock_constraint(clock, frequency)
+        clock.attrs["keep"] = 1
+
     def _get_xdr_buffer(self, m, pin, *, i_invert=False, o_invert=False):
         def get_dff(clk, d, q):
             # SDR I/O is performed by packing a flip-flop into the pad IOB.

Unfortunately that's not enough because Vivado doesn't consider (* keep = 32'd1 *) equivalent to (* keep = 1 *) aka (* keep *). This is something it has in common with Quartus, which we have to work around with write_verilog -decimal.

[whitequark]

Let me know if the fix works for you. It's somewhat gross but that's just what we can do with Yosys today.