amaranth-lang / amaranth

A modern hardware definition language and toolchain based on Python
https://amaranth-lang.org/docs/amaranth/
BSD 2-Clause "Simplified" License
1.56k stars 174 forks source link

platform.add_clock_constraint does not work for instances with lattice diamond for machxo2 #546

Open anuejn opened 3 years ago

anuejn commented 3 years ago

Similiar to #373 platform.add_clock_constraint does not work for instances in lattice diamond. However, the same fix does not seem to be applicable. Do you have any ideas what is going wrong?

repro:

from amaranth import *
from amaranth.build import *
from amaranth_boards.tinyfpga_ax2 import *

class Test(Elaboratable):
    def elaborate(self, plat):
        m = Module()
        clk = Signal()
        m.submodules.inst = Instance("JTAGF", o_JTCK=clk)

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

        plat.add_clock_constraint(clk, 2e6)

        plat.add_resources([Resource("gpio", 0, Pins("1", conn=("gpio", 0), dir="o"), Attrs(IOSTANDARD="LVCMOS33"))])
        gpio0 = plat.request("gpio", 0)
        m.d.sync += gpio0.o.eq(~gpio0)

        return m

TinyFPGAAX2Platform().build(Test())
whitequark commented 3 years ago

What is the error you're getting?

anuejn commented 3 years ago

in build/top_impl/top_impl_cck.rpt:

create_clock -name clk -period 500.0 [get_nets {clk$1}]
    @E::"/home/anuejn/tmp/repro/build/top.sdc":1:0:1:0|Source for clock clk not found in netlist.
anuejn commented 3 years ago

build.zip

whitequark commented 3 years ago

It probably doesn't like the $. That doesn't happen for ports because ports are named first. Try Instance("JTAGF", o_JTCK=ClockSignal()).

anuejn commented 3 years ago

How can I constrain it then?

whitequark commented 3 years ago
cd_sync = ClockDomain()
platform.add_clock_constraint(cd_sync.clk, freq)
anuejn commented 3 years ago

Thanks that works. Somehow it doesnt work for my non minimized use case :(. I will try to find another (less minimized) example

anuejn commented 3 years ago

This also doesnt work, probably due to hierarchy:

from nmigen import *
from nmigen.build import *
from nmigen_boards.tinyfpga_ax2 import *

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

        m.submodules.jtag = JTAG()

        return m

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

        cd_sync = ClockDomain("sync")
        m.domains += cd_sync
        plat.add_clock_constraint(cd_sync.clk, 2e6)

        plat.add_resources([Resource("gpio", 0, Pins("1", conn=("gpio", 0), dir="o"), Attrs(IOSTANDARD="LVCMOS33"))])
        gpio0 = plat.request("gpio", 0)
        m.d.sync += gpio0.eq(~gpio0)

        m.submodules.inst = Instance("JTAGF", o_JTCK=cd_sync.clk)

        return m

TinyFPGAAX2Platform().build(Test())

the error is


create_clock -name clk -period 500.0 [get_nets jtag/clk]
    @E::"/home/anuejn/tmp/repro/build/top.sdc":1:0:1:0|Source for clock clk not found in netlist.

build.zip

whitequark commented 3 years ago

The second one is a very serious issue, and one we can already fix (the $ problem is more thorny). You should research how to specify hierarchy for Diamond.

anuejn commented 3 years ago

Sadly I am not at all good with diamond but I will try my best. Maybe @cr1901 can help here too?

whitequark commented 3 years ago

I am not good with it either. I'm just persistent enough that eventually I fix this stuff when it becomes my responsibility.

anuejn commented 3 years ago

Okay according to some microsemi guide to synplify pro you have to use "." as a hierarchy seperator jUsT iN SdC FiLeS. And it works :).

Should i file a PR that fixes that?

whitequark commented 3 years ago

Yep.

whitequark commented 3 years ago

Or maybe use set_hierarchy_separator {/} ?

anuejn commented 3 years ago

The first example really works when we escape $ with a \ in front of it as @cr1901 suggested :)

anuejn commented 3 years ago

Or maybe use set_hierarchy_separator {/} ?

Tested. Works

whitequark commented 4 months ago

Testing with 028d5d80736151dbe6f9d35d35544cbfff6bef87: this is still a problem.

The issue isn't related to hierarchy. It is caused by the $ symbol in this SDC constraint:

 create_clock -name "clk" -period 500.0 [get_nets "clk\$2"]

To see how Tcl performs this escaping, consider:

% puts "clk\$2"
clk$2
% puts {clk\$2}
clk\$2

For some reason, {clk\$2} works. However, "clk$2" doesn't (crash), "clk\$2" doesn't (clock not found), "clk\\$2" doesn't (crash), and "clk\\\$2" doesn't (clock not found), and {clk$2} doesn't (clock not found).

It looks like the only option is to use the weird inconsistent escape scheme that Diamond seems to require. Sadness.

whitequark commented 4 months ago

After further testing, Diamond also requires the clock to be specified using the canonical net, i.e. after bypassing all the aliases (wire alias = canonical;). This seems to happen early in the frontend and no attributes affect this behavior.