Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

platform.add_clock_constraint does not work for instances with vivado #373

Closed
rroohhh opened this issue Apr 30, 2020 · 7 comments
Closed

platform.add_clock_constraint does not work for instances with vivado #373

rroohhh opened this issue Apr 30, 2020 · 7 comments
Labels
bug toolchain:vivado
Milestone
0.3

Comments

@rroohhh
Copy link
Contributor

rroohhh commented Apr 30, 2020

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

This comment has been minimized.

Sign in to view
@whitequark
Copy link
Member

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
Copy link
Contributor Author

rroohhh commented Apr 30, 2020

Alright, sorry for not keeping them seperate.

@whitequark
Copy link
Member

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
Copy link
Member

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

@whitequark
Copy link
Member

whitequark commented May 20, 2020 •
edited

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 whitequark added this to the 0.3 milestone May 20, 2020
@whitequark
Copy link
Member

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

whitequark added a commit that referenced this issue May 21, 2020
@whitequark
vendor.xilinx_{7series,ultrascale}: don't use write_verilog -decimal.
702e41b 
In commit 892cff0, `-decimal` was used when writing Verilog for
Vivado targets because it treats (* keep=32'd1 *) and (* keep=1 *)
differently in violation of Verilog LRM. However, it is possible
to avoid that workaround by using (* keep="TRUE" *). Do that,
and remove `-decimal` to avoid special-casing 32-bit constants.

Refs #373.
@anuejn anuejn mentioned this issue Nov 19, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug toolchain:vivado
Milestone
0.3
Development

No branches or pull requests
2 participants
@whitequark @rroohhh