unexpected behavior against multiple conflicting drivers in odin BLIFReader with yosys elaborator

poname commented 2 years ago

When using yosys+odin with the 'paj_boundtop_hierarchy_no_mem' verilog input file, with or without arch file, everything looks fine and the verilog file is being elaborated and partial mapped successfuly, however if we take a deep look into intermediate blif file generated by yosys (coarsen_netlist.yosys.blif) and yosys log (elaboration.yosys.log) we see warning for multiple conflicting drivers in the design in

elaboration.yosys.log
...
-- Running command check' --

Executing CHECK pass (checking for obvious problems). Checking module boundcontroller... Checking module listhandler... Checking module onlyonecycle... Checking module paj_boundtop_hierarchy_no_mem... Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [13]: port debugcount[13] of cell boundcont01 (boundcontroller) port debugcount[13] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [12]: port debugcount[12] of cell boundcont01 (boundcontroller) port debugcount[12] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [11]: port debugcount[11] of cell boundcont01 (boundcontroller) port debugcount[11] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [10]: port debugcount[10] of cell boundcont01 (boundcontroller) port debugcount[10] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [9]: port debugcount[9] of cell boundcont01 (boundcontroller) port debugcount[9] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [8]: port debugcount[8] of cell boundcont01 (boundcontroller) port debugcount[8] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [7]: port debugcount[7] of cell boundcont01 (boundcontroller) port debugcount[7] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [6]: port debugcount[6] of cell boundcont01 (boundcontroller) port debugcount[6] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [5]: port debugcount[5] of cell boundcont01 (boundcontroller) port debugcount[5] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [4]: port debugcount[4] of cell boundcont01 (boundcontroller) port debugcount[4] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [3]: port debugcount[3] of cell boundcont01 (boundcontroller) port debugcount[3] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [2]: port debugcount[2] of cell boundcont01 (boundcontroller) port debugcount[2] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [1]: port debugcount[1] of cell boundcont01 (boundcontroller) port debugcount[1] of cell boundcont10 (boundcontroller) Warning: multiple conflicting drivers for paj_boundtop_hierarchy_no_mem.\debugcount01 [0]: port debugcount[0] of cell boundcont01 (boundcontroller) port debugcount[0] of cell boundcont10 (boundcontroller) Checking module rayinterface... Checking module resultcounter... Checking module resultinterface... Checking module resulttransmit... Checking module sortedstack... Checking module spram... Checking module spramblock... Checking module sramcontroller... Checking module vblockramcontroller... Found and reported 14 problems. ...
```
Now have a look into the generated blif file by yosys in 
> **coarsen_netlist.yosys.blif:1404**
```
.subckt $sub A[0]=boundcont01.count_$adffQ.Q[0] A[1]=boundcont01.count$adffQ.Q[1] A[2]=boundcont01.count$adffQ.Q[2] A[3]=boundcont01.count$adffQ.Q[3] A[4]=boundcont01.count$adffQ.Q[4] A[5]=boundcont01.count$adffQ.Q[5] A[6]=boundcont01.count$adffQ.Q[6] A[7]=boundcont01.count$adffQ.Q[7] A[8]=boundcont01.count$adffQ.Q[8] A[9]=boundcont01.count$adffQ.Q[9] A[10]=boundcont01.count$adffQ.Q[10] A[11]=boundcont01.count$adffQ.Q[11] A[12]=boundcont01.count$adffQ.Q[12] A[13]=boundcont01.count$adff_Q.Q[13] B=$true Y[0]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[0] Y[1]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[1] Y[2]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[2] Y[3]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[3] Y[4]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[4] Y[5]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[5] Y[6]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[6] Y[7]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[7] Y[8]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[8] Y[9]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[9] Y[10]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[10] Y[11]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[11] Y[12]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[12] Y[13]=boundcont01.temptriIDvalid$pmux_Y_B1$mux_YS$mux_S_Y[13]
```
inputs **A[13:0]** are connected to wires **boundcont01.count_$adff_Q.Q[13:0]**, while we can see in
> **coarsen_netlist.yosys.blif:77**
```
.subckt $sdff CLK=tm3_clkv0 D[0]=boundcont10.count$adffQ.D[0] D[1]=boundcont10.count$adffQ.D[1] D[2]=boundcont10.count$adffQ.D[2] D[3]=boundcont10.count$adffQ.D[3] D[4]=boundcont10.count$adffQ.D[4] D[5]=boundcont10.count$adffQ.D[5] D[6]=boundcont10.count$adffQ.D[6] D[7]=boundcont10.count$adffQ.D[7] D[8]=boundcont10.count$adffQ.D[8] D[9]=boundcont10.count$adffQ.D[9] D[10]=boundcont10.count$adffQ.D[10] D[11]=boundcont10.count$adffQ.D[11] D[12]=boundcont10.count$adffQ.D[12] D[13]=boundcont10.count$adffQ.D[13] Q[0]=boundcont01.count$adffQ.Q[0] Q[1]=boundcont01.count$adffQ.Q[1] Q[2]=boundcont01.count$adffQ.Q[2] Q[3]=boundcont01.count$adffQ.Q[3] Q[4]=boundcont01.count$adffQ.Q[4] Q[5]=boundcont01.count$adffQ.Q[5] Q[6]=boundcont01.count$adffQ.Q[6] Q[7]=boundcont01.count$adffQ.Q[7] Q[8]=boundcont01.count$adffQ.Q[8] Q[9]=boundcont01.count$adffQ.Q[9] Q[10]=boundcont01.count$adffQ.Q[10] Q[11]=boundcont01.count$adffQ.Q[11] Q[12]=boundcont01.count$adffQ.Q[12] Q[13]=boundcont01.count$adff_Q.Q[13] SRST=pglobalreset .param CLK_POLARITY 00000000000000000000000000000001 .param SRST_POLARITY 00000000000000000000000000000001 .param SRST_VALUE 00000000000000 .param WIDTH 00000000000000000000000000001110
```
and then in
> **coarsen_netlist.yosys.blif:82**
```
.subckt $sdff CLK=tm3_clkv0 D[0]=boundcont01.count$adffQ.D[0] D[1]=boundcont01.count$adffQ.D[1] D[2]=boundcont01.count$adffQ.D[2] D[3]=boundcont01.count$adffQ.D[3] D[4]=boundcont01.count$adffQ.D[4] D[5]=boundcont01.count$adffQ.D[5] D[6]=boundcont01.count$adffQ.D[6] D[7]=boundcont01.count$adffQ.D[7] D[8]=boundcont01.count$adffQ.D[8] D[9]=boundcont01.count$adffQ.D[9] D[10]=boundcont01.count$adffQ.D[10] D[11]=boundcont01.count$adffQ.D[11] D[12]=boundcont01.count$adffQ.D[12] D[13]=boundcont01.count$adffQ.D[13] Q[0]=boundcont01.count$adffQ.Q[0] Q[1]=boundcont01.count$adffQ.Q[1] Q[2]=boundcont01.count$adffQ.Q[2] Q[3]=boundcont01.count$adffQ.Q[3] Q[4]=boundcont01.count$adffQ.Q[4] Q[5]=boundcont01.count$adffQ.Q[5] Q[6]=boundcont01.count$adffQ.Q[6] Q[7]=boundcont01.count$adffQ.Q[7] Q[8]=boundcont01.count$adffQ.Q[8] Q[9]=boundcont01.count$adffQ.Q[9] Q[10]=boundcont01.count$adffQ.Q[10] Q[11]=boundcont01.count$adffQ.Q[11] Q[12]=boundcont01.count$adffQ.Q[12] Q[13]=boundcont01.count$adff_Q.Q[13] SRST=pglobalreset .param CLK_POLARITY 00000000000000000000000000000001 .param SRST_POLARITY 00000000000000000000000000000001 .param SRST_VALUE 00000000000000 .param WIDTH 00000000000000000000000000001110
```
that wires **boundcont01.count_$adff_Q.Q[13:0]** are being driven by outputs of the two diffferent sets of SDFFs and also these SDFFs are connected to two different sets of inputs **boundcont10.count_$adff_Q.D[13:0]** and **boundcont01.count_$adff_Q.D[13:0]** respectively. But BLIFReader only considers the first driver (outputs of the first $sdff in line 77) for $sub subcircuit and ignores the second driver (outputs of the second $sdff in line 82). In downstream functions like in `adders.cpp:1204` and `` the number of driver pins are being checked explicitely to be less than or equal to 1, `oassert(temp_pin->net->num_driver_pins <= 1);`. Now it is not clear why BLIFReader is ignoring second driving set for the $sub subcircuit, it makes more sense to whether detect and report **multiple conflicting drivers** or add both drivers to the netlist and let others decide how to deal with it.
```

Expected Behaviour

to detect and report the multiple conflicting drivers in BLIFReadrear or create netlist with multiple conflicting drivers and let downstream tasks (like BLIFElaborator or PartialMapper) decide what to do

Current Behaviour

multiple drivers are being ignored and the netlist is created with first defined driver and thereafter blif elaboration and partial mapping are being performed successfully while ignoring second conflicting driver.

Possible Solution

Steps to Reproduce

./odin_II --elaborator yosys --fflegalize -V regression_test/benchmark/verilog/large/paj_boundtop_hierarchy_no_mem.v

Context

Your Environment

VTR revision used: 8.1.0-dev+6531416ce
Operating System and version: Ubuntu 22.04
Compiler version: GNU 11.2.0 on Linux-5.15.0-46-generic x86_64

sdamghan commented 2 years ago

Thanks @poname - As we discussed, the Odin-II codebase currently does not support multiple drivers for a net. This is a really good finding, and interestingly a similar issue with the boundtop benchmark, which is using a similar module to paj_boundtop_hierarchy_no_mem, has been discussed by @aman26kbm and me. So, please dive into this issue to determine the problem source by investigating the Yosys synthesis behaviour and checking the generated coarse-grained BLIF file. I suspect the issue could come from the Verilog file, so I will take a look into the Verilog file.

poname commented 2 years ago

Thanks @sdamghan for the reply, in the verilog file there is an issue,

paj_boundtop_hierarchy_no_mem.v:225

boundcontroller boundcont10(raygroupout10, raygroupwe10, raygroupid10, enablenear10, raygroup10, raygroupvalid10, busy10, triIDvalid10, triID10, wanttriID, reset10, baseaddress10, newresult, BoundNodeID10, resultid, hitmask10, dataready10, empty10, level10, boundnodeIDout10, ack10, lhreset10, addrind10, addrindvalid10, ostdata, ostdatavalid, tladdr10, tladdrvalid10, tldata, tldatavalid, t1i, t2i, t3i, u1i, u2i, u3i, v1i, v2i, v3i, id1i, id2i, id3i, hit1i, hit2i, hit3i, t1_10, t2_10, t3_10, u1_10, u2_10, u3_10, v1_10, v2_10, v3_10, id1_10, id2_10, id3_10, hit1_10, hit2_10, hit3_10, bcvalid10, done, cntreset10, passCTS10, passCTS01, pglobalreset, tm3_clk_v0, state10, debugsubcount10, debugcount01);

the last parameter should be debugcount10 instead of debugcount01. The problem for this specific input is resolved.

sdamghan commented 2 years ago

Thanks @poname - this issue was already fixed in the main benchmark, i.e., boundtop, in the _vtrflow/benchmarks/verilog directory. As we already discussed, some benchmarks under the _ODINII/regression directory might not be fully updated or synchronized with the main vtr benchmarks. Please keep it in your mind to update them in a separate PR to avoid further conflicts. There is also another issue with the boundtop benchmark; Yosys+Odin-II and Odin-II come up with different results for this benchmark, especially for a number of I/O. Similar behaviour is also seen for stereovision2 with the flagship and agilex architectures. Please dive into this problem and search for the issue source along with providing frequent updates on this GitHub issue.

poname commented 2 years ago

Thanks, as we discussed, for example for the boundtop benchmark the number of input nodes are different in the stats for ODIN-II vs Yosys+ODIN-II, but the number of inputs in the final generated blif files are the same. Perhaps this could be a good point to dig deeper.

sdamghan commented 2 years ago

@poname - not sure about the statement, as I just ran the boundtop benchmark with all three frontends and achieved the number of I/Os as follows at the end of the VTR flow (vpr.out):

Odin-II

Pb types usage...
  io               : 335
   inpad           : 142
   outpad          : 193

Yosys+Odin-II

Pb types usage...
  io                   : 389
   inpad               : 196
   outpad              : 193

Yosys

Pb types usage...
  io                   : 389
   inpad               : 196
   outpad              : 193

The difference in the CLB usage for the boundtop benchmark is mainly due to the memory inference, as I see Odin-II infers arrays of registers while Yosys+Odin-II infers a hard memory block. This is solely the behaviour we see in this benchmark; however, the scenario for stereovison2 is not the same. I recommend you go through the stereovison2 benchmark to investigate more about the discrepancy between VTR frontends.

verilog-to-routing / vtr-verilog-to-routing