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

Fix fasm2bels issues when running with nextpnr #164

Open
acomodi opened this issue Jun 17, 2020 · 10 comments
Open

Fix fasm2bels issues when running with nextpnr #164

acomodi opened this issue Jun 17, 2020 · 10 comments
Labels
bug Something isn't working

Comments

@acomodi
Copy link
Contributor

acomodi commented Jun 17, 2020

There are some errors produced in the fasm2bels flow described here: #159 (comment).

These errors did not show up for the smaller test designs, but rather when running the more complex ones such as the litex-linux one.

@acomodi acomodi added this to In progress in fpga-tool-perf Jun 17, 2020
@acomodi
Copy link
Contributor Author

acomodi commented Jun 25, 2020

#161 enabled fasm2bels for nextpnr.

However, there are errors that show up when running the litex-linux test:

  1. channels.db seems to be incomplete as it miss some wires. In fact, during fasm2bels, this code fails. By using the channels.db from symbiflow arch defs, this issue seems to be solved.
    The problem seems to be related to the BRAM CLKARDCLK wire, need to investigate the problem in more details, as it is possible that there are some missing steps in the creation of the reduced channels.db file.
  2. clb models fail with the following assertion:
Traceback (most recent call last):
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/site-packages/fasm2bels/__main__.py", line 4, in <module>
    main()
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/site-packages/fasm2bels/fasm2bels.py", line 394, in main
    process_tile(top, tile, tile_features)
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/site-packages/fasm2bels/fasm2bels.py", line 101, in process_tile
    PROCESS_TILE[tile_type](top.conn, top, tile, tile_features)
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/site-packages/fasm2bels/models/clb_models.py", line 1224, in process_clb
    process_slice(top, slices[s])
  File "/data/fpga-tool-perf/env/conda/envs/fpga-tool-perf-env/lib/python3.7/site-packages/fasm2bels/models/clb_models.py", line 1196, in process_slice
    assert can_have_carry4
AssertionError

  1. Some sinks result in being orphan sinks with no source: this could be solved by allowing orphan sinks in fasm2bels
  2. By temporarily allowing orphan nets to get past the error, the following error shows up during tcl sourcing in Vivado:
ERROR: [Vivado 12-2285] Cannot set LOC property of instance 'CLBLM_L_X8Y107_SLICE_X10Y107_RAM64X1S_C',  for bel B6LUT Conflicting nets for SLICE_X10Y107.B6LUT.WA4
Two net names are: CLBLM_R_X7Y104_SLICE_X8Y104_DQ and CLBLM_L_X8Y107_SLICE_X11Y107_D5Q,  for bel C6LUT Conflicting nets for SLICE_X10Y107.C6LUT.WA4
Two net names are: CLBLM_R_X7Y104_SLICE_X8Y104_DQ and CLBLM_L_X8Y107_SLICE_X11Y107_D5Q
Resolution: When using BEL constraints, ensure the BEL constraints are defined before the LOC constraints to avoid conflicts at a given site.

As the resolution says, it may be necessary to get first all the BEL constraints defined and later all the LOC constraints. This would likely double the size of the tcl file and its source run-time.

@litghost
Copy link
Contributor

litghost commented Jun 25, 2020

As the resolution says, it may be necessary to get first all the BEL constraints defined and later all the LOC constraints. This would likely double the size of the tcl file and its source run-time.

This will not help. Using an XDC file instead of a TCL file may help, unclear.

@mithro mithro added the bug Something isn't working label Jun 25, 2020
@acomodi
Copy link
Contributor Author

acomodi commented Jul 21, 2020

New issue found when running with the updated fasm2bels library the blinky test:

WARNING: [Vivado 12-1023] No nets matched for command 'get_nets -of_object [get_pins {*CLBLM_R_X39Y73_SLICE_X60Y73_CARRY4/CO[3]}]'.
## set net [get_nets -of_object $pin]
## set route "[list  [get_nodes -of_object [get_wires CLBLM_R_X39Y73/CLBLM_M_COUT]] [get_nodes -of_object [get_wires CLBLM_R_X39Y73/CLBLM_M_COUT_N]] ] "
## set_property FIXED_ROUTE $route $net
ERROR: [Common 17-55] 'set_property' expects at least one object.
Resolution: If [get_<value>] was used to populate the object, check to make sure this command returns at least one valid object.

There seems to be a net that cannot be found.

@litghost
Copy link
Contributor

New issue found when running with the updated fasm2bels library the blinky test:

WARNING: [Vivado 12-1023] No nets matched for command 'get_nets -of_object [get_pins {*CLBLM_R_X39Y73_SLICE_X60Y73_CARRY4/CO[3]}]'.
## set net [get_nets -of_object $pin]
## set route "[list  [get_nodes -of_object [get_wires CLBLM_R_X39Y73/CLBLM_M_COUT]] [get_nodes -of_object [get_wires CLBLM_R_X39Y73/CLBLM_M_COUT_N]] ] "
## set_property FIXED_ROUTE $route $net
ERROR: [Common 17-55] 'set_property' expects at least one object.
Resolution: If [get_<value>] was used to populate the object, check to make sure this command returns at least one valid object.

There seems to be a net that cannot be found.

That might be a bug in fasm2bels, or it could be an antenna CARRY4 from nextpnr. There is FASM feature (e.g. CLBLL_L.SLICEL_X0.PRECYINIT.CIN !00_12 30_13 !30_14) that indicates that a COUT -> CIN connection. If that CARRY4 has not outputs, I believe fasm2bels might not emit it. I would argue that nextpnr shouldn't have emitted the CARRY4 in that case, but it is also a bug for fasm2bels to have an antenna route.

If you confirm that the PRECYINIT.CIN for the tile above the antenna route is present, then we should fix fasm2bels. It is also worth exploring whether the CARRY4 is an antenna object (no outputs used) and if so, open an issue on nextpnr-xilinx about emission of antenna CARRY4's.

If the PRECYINIT.CIN for the tile above the antenna is not present, then this is only a fasm2bels bug.

@acomodi
Copy link
Contributor Author

acomodi commented Jul 21, 2020

It appears that the COUT->CIN connection is there indeed by looking into the fasm output:

CLBLM_R_X39Y73.SLICEL_X1.DFF.ZINI
CLBLM_R_X39Y73.SLICEL_X1.DFF.ZRST
CLBLM_R_X39Y73.SLICEL_X1.DFFMUX.DX
CLBLM_R_X39Y73.SLICEL_X1.FFSYNC
CLBLM_R_X39Y73.SLICEL_X1.NOCLKINV
CLBLM_R_X39Y73.SLICEL_X1.PRECYINIT.C0
CLBLM_R_X39Y73.SLICEM_X0.AFF.ZINI
CLBLM_R_X39Y73.SLICEM_X0.AFF.ZRST
CLBLM_R_X39Y73.SLICEM_X0.AFFMUX.AX
CLBLM_R_X39Y73.SLICEM_X0.ALUT.DI1MUX.BDI1_BMC31
CLBLM_R_X39Y73.SLICEM_X0.ALUT.INIT[63:0] = 64'b1010101010101010101010101010101011001100110011001100110011001100
CLBLM_R_X39Y73.SLICEM_X0.AOUTMUX.XOR
CLBLM_R_X39Y73.SLICEM_X0.BFF.ZINI
CLBLM_R_X39Y73.SLICEM_X0.BFF.ZRST
CLBLM_R_X39Y73.SLICEM_X0.BFFMUX.XOR
CLBLM_R_X39Y73.SLICEM_X0.BLUT.DI1MUX.DI_CMC31
CLBLM_R_X39Y73.SLICEM_X0.BLUT.INIT[63:0] = 64'b1100110011001100110011001100110011110000111100001111000011110000
CLBLM_R_X39Y73.SLICEM_X0.CARRY4.ACY0
CLBLM_R_X39Y73.SLICEM_X0.CARRY4.BCY0
CLBLM_R_X39Y73.SLICEM_X0.CARRY4.CCY0
CLBLM_R_X39Y73.SLICEM_X0.CARRY4.DCY0
CLBLM_R_X39Y73.SLICEM_X0.CLUT.DI1MUX.DI_DMC31
CLBLM_R_X39Y73.SLICEM_X0.CLUT.INIT[63:0] = 64'b1111000011110000111100001111000010101010101010101010101010101010
CLBLM_R_X39Y73.SLICEM_X0.COUTMUX.XOR
CLBLM_R_X39Y73.SLICEM_X0.DFF.ZINI
CLBLM_R_X39Y73.SLICEM_X0.DFF.ZRST
CLBLM_R_X39Y73.SLICEM_X0.DFFMUX.DX
CLBLM_R_X39Y73.SLICEM_X0.DLUT.INIT[63:0] = 64'b1100110011001100110011001100110010101010101010101010101010101010
CLBLM_R_X39Y73.SLICEM_X0.DOUTMUX.XOR
CLBLM_R_X39Y73.SLICEM_X0.FFSYNC
CLBLM_R_X39Y73.SLICEM_X0.NOCLKINV
CLBLM_R_X39Y73.SLICEM_X0.PRECYINIT.CIN

CLBLM_R_X39Y74.SLICEL_X1.AFF.ZINI
CLBLM_R_X39Y74.SLICEL_X1.AFF.ZRST
CLBLM_R_X39Y74.SLICEL_X1.AFFMUX.AX
CLBLM_R_X39Y74.SLICEL_X1.BFF.ZINI
CLBLM_R_X39Y74.SLICEL_X1.BFF.ZRST
CLBLM_R_X39Y74.SLICEL_X1.BFFMUX.BX
CLBLM_R_X39Y74.SLICEL_X1.D5FF.ZINI
CLBLM_R_X39Y74.SLICEL_X1.D5FF.ZRST
CLBLM_R_X39Y74.SLICEL_X1.D5FFMUX.IN_B
CLBLM_R_X39Y74.SLICEL_X1.DOUTMUX.D5Q
CLBLM_R_X39Y74.SLICEL_X1.FFSYNC
CLBLM_R_X39Y74.SLICEL_X1.NOCLKINV
CLBLM_R_X39Y74.SLICEL_X1.PRECYINIT.C0
CLBLM_R_X39Y74.SLICEM_X0.AFF.ZINI
CLBLM_R_X39Y74.SLICEM_X0.AFF.ZRST
CLBLM_R_X39Y74.SLICEM_X0.AFFMUX.XOR
CLBLM_R_X39Y74.SLICEM_X0.ALUT.DI1MUX.BDI1_BMC31
CLBLM_R_X39Y74.SLICEM_X0.ALUT.INIT[63:0] = 64'b1111000011110000111100001111000011001100110011001100110011001100
CLBLM_R_X39Y74.SLICEM_X0.BLUT.DI1MUX.DI_CMC31
CLBLM_R_X39Y74.SLICEM_X0.BLUT.INIT[63:0] = 64'b1111111100000000111111110000000011001100110011001100110011001100
CLBLM_R_X39Y74.SLICEM_X0.BOUTMUX.XOR
CLBLM_R_X39Y74.SLICEM_X0.CARRY4.ACY0
CLBLM_R_X39Y74.SLICEM_X0.CARRY4.BCY0
CLBLM_R_X39Y74.SLICEM_X0.CARRY4.CCY0
CLBLM_R_X39Y74.SLICEM_X0.CARRY4.DCY0
CLBLM_R_X39Y74.SLICEM_X0.CLUT.DI1MUX.DI_DMC31
CLBLM_R_X39Y74.SLICEM_X0.CLUT.INIT[63:0] = 64'b1010101010101010101010101010101011001100110011001100110011001100
CLBLM_R_X39Y74.SLICEM_X0.COUTMUX.XOR
CLBLM_R_X39Y74.SLICEM_X0.DLUT.INIT[63:0] = 64'b1010101010101010101010101010101010101010101010101010101010101010
CLBLM_R_X39Y74.SLICEM_X0.DOUTMUX.XOR
CLBLM_R_X39Y74.SLICEM_X0.FFSYNC
CLBLM_R_X39Y74.SLICEM_X0.NOCLKINV
CLBLM_R_X39Y74.SLICEM_X0.PRECYINIT.CIN

This is the output verilog from fasm2bels related to the problematic CARRY:

(* KEEP, DONT_TOUCH *)                                                                                                                                                                                                                                                            
CARRY4 #(                                                                                                                                                                                                                                                                         
) CLBLM_R_X39Y73_SLICE_X60Y73_CARRY4 (                                                                                                                                                                                                                                            
.CI(CLBLM_R_X39Y72_SLICE_X60Y72_COUT),                                                                                                                                                                                                                                              
.CO({CLBLM_R_X39Y73_SLICE_X60Y73_D_CY, CLBLM_R_X39Y73_SLICE_X60Y73_C_CY, CLBLM_R_X39Y73_SLICE_X60Y73_B_CY, CLBLM_R_X39Y73_SLICE_X60Y73_A_CY}),                                                                                                                                      
.CYINIT(1'b0),                                                                                                                                                                                                                                                                      
.DI({CLBLM_R_X39Y73_SLICE_X60Y73_DO5, CLBLM_R_X39Y73_SLICE_X60Y73_CO5, CLBLM_R_X39Y73_SLICE_X60Y73_BO5, CLBLM_R_X39Y73_SLICE_X60Y73_AO5}),                                                                                                                                          
.O({CLBLM_R_X39Y73_SLICE_X60Y73_D_XOR, CLBLM_R_X39Y73_SLICE_X60Y73_C_XOR, CLBLM_R_X39Y73_SLICE_X60Y73_B_XOR, CLBLM_R_X39Y73_SLICE_X60Y73_A_XOR}),                                                                                                                                   
.S({CLBLM_R_X39Y73_SLICE_X60Y73_DO6, CLBLM_R_X39Y73_SLICE_X60Y73_CO6, CLBLM_R_X39Y73_SLICE_X60Y73_BO6, CLBLM_R_X39Y73_SLICE_X60Y73_AO6})                                                                                                                                            
);    

The CO outputs are not used anywhere, while the O ones are connected to valid nets.

@litghost
Copy link
Contributor

The error above looks like the CARRY4 at CLBLM_R_X39Y73.SLICEM_X0 is missing. The Verilog you pasted is missing a "LOC=" annotation.

@acomodi
Copy link
Contributor Author

acomodi commented Jul 21, 2020

Actually the LOC should be in the XDC.

For vpr-fasm2bels, carry chains slices are LOCed as follows:

set_property LOC SLICE_X45Y65 [get_cells *CLBLL_L_X28Y65_SLICE_X45Y65_CARRY4]
set_property LOC SLICE_X45Y66 [get_cells *CLBLL_L_X28Y66_SLICE_X45Y66_CARRY4]
set_property LOC SLICE_X45Y67 [get_cells *CLBLL_L_X28Y67_SLICE_X45Y67_CARRY4]
set_property LOC SLICE_X45Y68 [get_cells *CLBLL_L_X28Y68_SLICE_X45Y68_CARRY4]
set_property LOC SLICE_X45Y69 [get_cells *CLBLL_L_X28Y69_SLICE_X45Y69_CARRY4]
set_property LOC SLICE_X45Y70 [get_cells *CLBLL_L_X28Y70_SLICE_X45Y70_CARRY4]
set_property LOC SLICE_X65Y48 [get_cells *CLBLM_R_X41Y48_SLICE_X65Y48_CARRY4]
set_property LOC SLICE_X65Y49 [get_cells *CLBLM_R_X41Y49_SLICE_X65Y49_CARRY4]

These similar lines above are not present in the nextpnr fasm2bels XDC output. I'll investigate that

@litghost
Copy link
Contributor

Actually the LOC should be in the XDC.

Good point.

@acomodi
Copy link
Contributor Author

acomodi commented Jul 22, 2020

After updating fpga-tool-perf to use the latest version of yosys I can now see two issues that block us from having nextpnr bitstreams run through fasm2bels:

  1. BRAM sinks cannot be connected to their sources. The output below is obtained by allowing orphan sinks:
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB18_X0Y44_REGCLKB not found
// ERROR, failed to find source for node = 34309 (BRAM_L_X6Y110/BRAM_CLK0_4)
// ERROR, failed to find source for node = 132499 (BRAM_L_X6Y110/BRAM_FIFO36_REGCLKARDRCLKL)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB36_X0Y22_REGCLKARDRCLKL not found
// ERROR, failed to find source for node = 34310 (BRAM_L_X6Y110/BRAM_CLK1_4)
// ERROR, failed to find source for node = 132500 (BRAM_L_X6Y110/BRAM_FIFO36_REGCLKARDRCLKU)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB36_X0Y22_REGCLKARDRCLKU not found
// ERROR, failed to find source for node = 33109 (BRAM_L_X6Y110/BRAM_CLK0_0)
// ERROR, failed to find source for node = 132501 (BRAM_L_X6Y110/BRAM_FIFO36_REGCLKBL)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB36_X0Y22_REGCLKBL not found
// ERROR, failed to find source for node = 33110 (BRAM_L_X6Y110/BRAM_CLK1_0)
// ERROR, failed to find source for node = 132502 (BRAM_L_X6Y110/BRAM_FIFO36_REGCLKBU)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB36_X0Y22_REGCLKBU not found
// ERROR, failed to find source for node = 34310 (BRAM_L_X6Y110/BRAM_CLK1_4)
// ERROR, failed to find source for node = 132800 (BRAM_L_X6Y110/BRAM_RAMB18_REGCLKARDRCLK)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB18_X0Y45_REGCLKARDRCLK not found
// ERROR, failed to find source for node = 33110 (BRAM_L_X6Y110/BRAM_CLK1_0)
// ERROR, failed to find source for node = 132801 (BRAM_L_X6Y110/BRAM_RAMB18_REGCLKB)
// ERROR, source for sink wire BRAM_L_X6Y110_RAMB18_X0Y45_REGCLKB not found
  1. Error described one the comment above. After fixing edalize to have a correct xdc parsing step, the error goes silent and becomes a CRITICAL WARNING. This needs to be solved and, if cells/nets/pins cannot be found, Vivado should through an error.

@litghost
Copy link
Contributor

  1. BRAM sinks cannot be connected to their sources. The output below is obtained by allowing orphan sinks:

These may be nextpnr bugs, if nextpnr is not routing the clocks to the additional ports. Should be straight forward to fix.

@kgugala kgugala removed this from In progress in fpga-tool-perf Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants