f4pga / f4pga-arch-defs

FOSS architecture definitions of FPGA hardware useful for doing PnR device generation.
https://f4pga.org
ISC License
273 stars 113 forks source link

Litex: drop the --symbiflow flag when generating the minilitex design #1753

Open acomodi opened 4 years ago

acomodi commented 4 years ago

We currently use the --symbiflow flag to generate the minilitex design. This was related to the fact that the PLL generate extra clocks and BUFG which are not used, ending up in clocks generated in the SDC, but they are absent in the cleaned circuit in VPR, therefore the SDC commands for the inexistent generate an error.

Possible solutions:

litghost commented 4 years ago
  • downgrade the VPR error to a warning via a runtime flag

This is not a solution. The issue is that VPR sweeps the netlist and invalidates an SDC that was originally valid. This is a bug in VPR frankly.

mithro commented 3 years ago

There are two potential locations that have bugs, 1) Bug is in Yosys SDC generation -- the yosys plugin is generating a SDC file which refers to a net which does not exists in in the blif. 1) Bug in VPR -- The net appears in the blif and VPR is sweeping the net and not sweeping the SDC too (as @litghost mentions).

acomodi commented 3 years ago

Update with some more data:

Error generated in VPR:

Warning 63: Inferred implicit clock source $iopadmap$top.clk100.O[0] for netlist clock main_crg_clkin (possibly data used as clock)
Warning 64: Timing edge from $iopadmap$top.clk100.I[0] to $iopadmap$top.clk100.O[0] will not be created since $iopadmap$top.clk100.O[0] has been identified as a clock generator
  Timing GraphError 1:
Type: SDC file
File: /data/symbiflow/symbiflow-arch-defs/build/xc/xc7/tests/soc/litex/mini/minilitex_arty/artix7-xc7a50t-virt-xc7a50t-test/top_synth.sdc
Line: 4
Message: Clock name or pattern 'main_crg_clkout1' does not correspond to any nets. To create a virtual clock, use the '-name' option.
 Nodes: 50242
  Timing Graph Edges: 80455
  Timing Graph Levels: 54
# Build Timing Graph took 0.04 seconds (max_rss 198.6 MiB, delta_rss +0.0 MiB)
Netlist contains 8 clocks
  Netlist Clock 'main_crg_clkin' Fanout: 9 pins (0.0%), 9 blocks (0.1%)
  Netlist Clock 'sys_clk' Fanout: 2146 pins (4.3%), 2105 blocks (19.8%)
  Netlist Clock 'main_crg_clkout_buf3' Fanout: 8 pins (0.0%), 8 blocks (0.1%)
  Netlist Clock 'main_crg_clkout_buf4' Fanout: 1 pins (0.0%), 1 blocks (0.0%)
  Netlist Clock 'builder_pll_fb' Fanout: 1 pins (0.0%), 1 blocks (0.0%)
  Netlist Clock 'main_crg_clkout0' Fanout: 1 pins (0.0%), 1 blocks (0.0%)
  Netlist Clock 'main_crg_clkout3' Fanout: 1 pins (0.0%), 1 blocks (0.0%)
  Netlist Clock 'main_crg_clkout4' Fanout: 1 pins (0.0%), 1 blocks (0.0%)
# Load Timing Constraints
# Load Timing Constraints took 0.00 seconds (max_rss 198.6 MiB, delta_rss +0.0 MiB)
The entire flow of VPR took 1.22 seconds (max_rss 198.6 MiB)
ninja: build stopped: subcommand failed.

PLL definition:

PLLE2_ADV #(
        .CLKFBOUT_MULT(4'd12),
        .CLKIN1_PERIOD(10.0),
        .CLKOUT0_DIVIDE(5'd20),
        .CLKOUT0_PHASE(1'd0),
        .CLKOUT1_DIVIDE(3'd5),
        .CLKOUT1_PHASE(1'd0),
        .CLKOUT2_DIVIDE(3'd5),
        .CLKOUT2_PHASE(7'd90),
        .CLKOUT3_DIVIDE(3'd6),
        .CLKOUT3_PHASE(1'd0),
        .CLKOUT4_DIVIDE(6'd48),
        .CLKOUT4_PHASE(1'd0),
        .DIVCLK_DIVIDE(1'd1),
        .REF_JITTER1(0.01),
        .STARTUP_WAIT("FALSE")
) PLLE2_ADV (
        .CLKFBIN(builder_pll_fb),
        .CLKIN1(main_crg_clkin),
        .RST(builder_reset7),
        .CLKFBOUT(builder_pll_fb),
        .CLKOUT0(main_crg_clkout0),
        .CLKOUT1(main_crg_clkout1),
        .CLKOUT2(main_crg_clkout2),
        .CLKOUT3(main_crg_clkout3),
        .CLKOUT4(main_crg_clkout4),
        .LOCKED(main_crg_locked)
);

Generated SDC:

create_clock -period 10 -waveform {0 5} main_crg_clkin
create_clock -period 16.6667 -waveform {0 8.33333} main_crg_clkout0
create_clock -period 16.6667 -waveform {0 8.33333} VexRiscv.IBusCachedPlugin_cache.clk
create_clock -period 4.16667 -waveform {0 2.08333} main_crg_clkout1
create_clock -period 4.16667 -waveform {1.04167 3.125} main_crg_clkout2
create_clock -period 5 -waveform {0 2.5} main_crg_clkout3
create_clock -period 5 -waveform {0 2.5} idelay_clk
create_clock -period 40 -waveform {0 20} main_crg_clkout4
create_clock -period 40 -waveform {0 20} main_crg_clkout_buf4

The PLL instantiates clocks also for the DDR and Ethernet modules, which are absent in the mini test.

The problem is that yosys correctly trims away the BUFGs from clkout1 and clkout2 which are related to DDR clocks, while the other clocks are kept:

Top module ports:

module top(
        output reg serial_tx,
        input wire serial_rx,
        input wire cpu_reset,
        (* dont_touch = "true" *)       input wire clk100,
        output wire eth_ref_clk,
        output reg user_led0,
        output reg user_led1,
        output reg user_led2,
        output reg user_led3
);

After all these observations I believe that yosys correctly trims useless clock nets, and the plugin is not aware of this, but sees the outputs from the PLL and creates the clocks even though they do not actually exist.

@tmichalak @mithro FYI

To reproduce this, the --symbiflow flag needs to be removed: https://github.com/SymbiFlow/symbiflow-arch-defs/blob/8c499dc5f1e13b420ca14abef8e917459ad9e331/xc/xc7/tests/soc/litex/mini/CMakeLists.txt#L22

tmichalak commented 3 years ago

There are two potential locations that have bugs,

  1. Bug is in Yosys SDC generation -- the yosys plugin is generating a SDC file which refers to a net which does not exists in in the blif.
  2. Bug in VPR -- The net appears in the blif and VPR is sweeping the net and not sweeping the SDC too (as @litghost mentions).

So ad.1. The net is present in the eblif. It is just a dangling net not connected to anything. Eblif contains:

.subckt PLLE2_ADV_VPR CLKFBIN=builder_pll_fb CLKFBOUT=builder_pll_fb CLKIN1=main_crg_clkin CLKIN2=$false CLKINSEL=$true CLKOUT0=main_crg_clkout0 CLKOUT1=main_crg_clkout1 CLKOUT2=main_crg_clkout2 CLKOUT3=main_crg_clkout3 CLKOUT4=main_crg_clkout4 CLKOUT5=$techmap176378\PLLE2_ADV.CLKOUT5 DADDR[0]=$false DADDR[1]=$false DADDR[2]=$false DADDR[3]=$false DADDR[4]=$false DADDR[5]=$false DADDR[6]=$false DCLK=$false DEN=$false DI[0]=$false DI[1]=$false DI[2]=$false DI[3]=$false DI[4]=$false DI[5]=$false DI[6]=$false DI[7]=$false DI[8]=$false DI[9]=$false DI[10]=$false DI[11]=$false DI[12]=$false DI[13]=$false DI[14]=$false DI[15]=$false DO[0]=$techmap176378\PLLE2_ADV.DO[0] DO[1]=$techmap176378\PLLE2_ADV.DO[1] DO[2]=$techmap176378\PLLE2_ADV.DO[2] DO[3]=$techmap176378\PLLE2_ADV.DO[3] DO[4]=$techmap176378\PLLE2_ADV.DO[4] DO[5]=$techmap176378\PLLE2_ADV.DO[5] DO[6]=$techmap176378\PLLE2_ADV.DO[6] DO[7]=$techmap176378\PLLE2_ADV.DO[7] DO[8]=$techmap176378\PLLE2_ADV.DO[8] DO[9]=$techmap176378\PLLE2_ADV.DO[9] DO[10]=$techmap176378\PLLE2_ADV.DO[10] DO[11]=$techmap176378\PLLE2_ADV.DO[11] DO[12]=$techmap176378\PLLE2_ADV.DO[12] DO[13]=$techmap176378\PLLE2_ADV.DO[13] DO[14]=$techmap176378\PLLE2_ADV.DO[14] DO[15]=$techmap176378\PLLE2_ADV.DO[15] DRDY=$techmap176378\PLLE2_ADV.DRDY DWE=$false LOCKED=main_crg_locked PWRDWN=$true RST=builder_reset7
.cname PLLE2_ADV

Adding a check during sdc generation if the clock wire is actually driving anything will fix the problem. However, maybe there should be no such net added to eblif in the first place?

mithro commented 3 years ago

Could there be a clean step which sweeps the dangling net before the SDC output?

tmichalak commented 3 years ago

Could there be a clean step which sweeps the dangling net before the SDC output?

I added a check whether the clock divider output wire drives a cell. If it does we don't add a clock on that wire. See https://github.com/SymbiFlow/yosys-symbiflow-plugins/pull/57

mithro commented 3 years ago

@tmichalak - Can you see if using the http://www.clifford.at/yosys/cmd_opt_clean.html pass will remove the wire?

Yosys Open SYnthesis Suite :: Command Reference :: opt_clean
mithro commented 3 years ago

From the docs - http://www.clifford.at/yosys/cmd_opt_clean.html

This pass identifies wires and cells that are unused and removes them. Other passes often remove cells but leave the wires in the design or reconnect the wires but leave the old cells in the design. This pass can be used to clean up after the passes that do the actual work.

Yosys Open SYnthesis Suite :: Command Reference :: opt_clean
tmichalak commented 3 years ago

@mithro I tried the opt_clean and clean commands beforehand but none of them worked. Hence, I went for the fix in the plugin.