f4pga / f4pga-arch-defs

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

Carry Chain COUT drives multiple net sinks #1597

Closed acomodi closed 3 years ago

acomodi commented 4 years ago

When updating the baslitex test with the newest version of Litex, I have encountered the following assertion failure in VPR:

vpr/src/place/place_macro.cpp:135 find_all_the_macro: Assertion 'cluster_ctx.clb_nlist.net_sinks(curr_net_id).size() == 1' failed.

The assertion checks whether nets between macros (in this case carry chains) have more than one sink. This because a net connecting to blocks in a macro should not drive other sinks, as this would create inconsistencies with the macro placement.

I have tracked down the possible cause of the issue being in the fact that COUT pins from carry chain get to be connected to external nets rather than just the CIN pin of a next carry in the chain (if no other carry is present in the chain, the COUT pin should not be connected to any other sink).

eblif:

.subckt CARRY4_VPR CO_CHAIN=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.COUT CO_FABRIC0=$techmap451788$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.CO[0] CO_FABRIC1=$techmap451788$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.CO[1] CO_FABRIC2=$techmap451788$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.CO[2] CO_FABRIC3=$techmap451788$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.CO[3] DI0=a7ddrphy_bitslip8_value[0] DI1=$false DI2=$false DI3=$false O0=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.O[0] O1=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.O[1] O2=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.O[2] O3=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.O[3] S0=$abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.S[0] S1=a7ddrphy_bitslip8_value[1] S2=a7ddrphy_bitslip8_value[2] S3=a7ddrphy_bitslip8_value[3]

...

.names $abc$108049$__35631__ $abc$108049$auto$alumacc.cc:485:replace_alu$19819.slice[0].carry4_1st_full.COUT a7ddrphy_bitslip8_r[13] a7ddrphy_bitslip8_r[21] $abc$108049$__39304__

From the eblif, we can see that the COUT pin is present in multiple nets, so this issue most probably relates to the yosys step.

top_synth.v:

  CARRY4_VPR #(
    .CYINIT_AX(1'h0),
    .CYINIT_C0(1'h1),
    .CYINIT_C1(1'h0)
  ) _34817_ (
    .CO_CHAIN(_09496_),
    .CO_FABRIC0(_14535_[0]),
    .CO_FABRIC1(_14535_[1]),
    .CO_FABRIC2(_14535_[2]),
    .CO_FABRIC3(_14535_[3]),
    .DI0(a7ddrphy_bitslip7_value[0]),
    .DI1(1'h0),
    .DI2(1'h0),
    .DI3(1'h0),
    .O0(_09497_[0]),
    .O1(_09497_[1]),
    .O2(_09497_[2]),
    .O3(_09497_[3]),
    .S0(_09498_[0]),
    .S1(a7ddrphy_bitslip7_value[1]),
    .S2(a7ddrphy_bitslip7_value[2]),
    .S3(a7ddrphy_bitslip7_value[3])
  );

...

assign _04983_ = 32'd288577023 >> { _03833_, _09496_, a7ddrphy_bitslip7_r[15], a7ddrphy_bitslip7_r[19], a7ddrphy_bitslip7_r[21] };

Also in the verilog representation, the CO_CHAIN output is connected to an assign statement.

litghost commented 4 years ago

Which version of symbiflow-yosys where you using?

acomodi commented 4 years ago

@litghost This was with the old master+wip (symbiflow-yosys-0.8_3925_g6bccd35a-20200629_180127.tar.bz2)

litghost commented 4 years ago

Can you please make a PR that demostrates this bug? I can try to take a look at it after I make https://github.com/SymbiFlow/symbiflow-arch-defs/pull/1577 green.

acomodi commented 4 years ago

@litghost Sure, here is the PR https://github.com/SymbiFlow/symbiflow-arch-defs/pull/1605

litghost commented 4 years ago

@litghost Sure, here is the PR #1605

Awesome, thanks

litghost commented 4 years ago

I've replicated the issue, debugging now.

litghost commented 4 years ago

I've replicated the issue, debugging now.

I think I know a fix, will test it soon.

litghost commented 4 years ago

I've replicated the issue, debugging now.

I think I know a fix, will test it soon.

My fix brought in other issues, trying a different approach.

litghost commented 4 years ago

I've replicated the issue, debugging now.

I think I know a fix, will test it soon.

My fix brought in other issues, trying a different approach.

I have an approach that appears to P&R, unknown if it works in hardware. I'll test that tommorow.

litghost commented 4 years ago

@acomodi When you updated the base LiteX, I think the SDC and PLL settings got out of sync. Can you double check what is going on there?

Specifically, from Vivado propigation:

--
Clock Name:         crg_clkout0
Waveform(ns):       { 0.000 5.000 }
Period(ns):         10.000
--
Clock Name:         crg_clkout1
Waveform(ns):       { 0.000 1.250 }
Period(ns):         2.500
--
Clock Name:         crg_clkout2
Waveform(ns):       { 0.625 1.875 }
Period(ns):         2.500

From SDC:

# PLL CLKOUT0 60 MHz                                                                                                                                                                                                                                                                                                                                                                                                                     
create_clock -period 16.666 crg_clkout0 -waveform {0.000 8.333}                                                                                                                                                                                                                                                                                                                                                                          

# BUFG CLKOUT0 60 MHz                                                                                                                                                                                                                                                                                                                                                                                                                    
create_clock -period 16.666 crg_clkout_buf0 -waveform {0.000 8.333}                                                                                                                                                                                                                                                                                                                                                                

# PLL CLKOUT1 240 MHz                                                                                                                                                                                                                                                                                                                                                                                                                    
create_clock -period 4.166 crg_clkout1  -waveform {0.000 2.083}                                                                                                                                                                                                                                                                                                                                                                          

# BUFG CLKOUT1 240 MHz                                                                                                                                                                                                                                                                                                                                                                                                                   
create_clock -period 4.166 crg_clkout_buf1 -waveform {0.000 2.083}                                                                                                                                                                                                                                                                                                                                                                       

# PLL CLKOUT2 240 MHz                                                                                                                                                                                                                                                                                                                                                                                                                    
create_clock -period 4.166 crg_clkout2 -waveform {1.041 3.124}                                                                                                                                                                                                                                                                                                                                                                           

# BUFG CLKOUT2 240 MHz                                                                                                                                                                                                                                                                                                                                                                                                                   
create_clock -period 4.166 crg_clkout_buf2 -waveform {1.041 3.124}    
acomodi commented 4 years ago

Can you double check what is going on there?

Right, I checked and I missed to set the clock frequency of the system at 60 MHz when generating the design. I will generate a new one and update the PR