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

Verilog ports shorted with assign statements in subcells are not handled correctly. #81

Open
d-m-bailey opened this issue Sep 1, 2023 · 7 comments

Comments

@d-m-bailey
Copy link
Contributor

Netgen 1.5.253
Using verilog generated from Cadence Innovus.

  1. From sky130 mpw-4 slot-005, the user_proj_example has many ports shorted.

    assign VWPR = VPWR;
    assign wbs_ack_o = io_oeb[27];
    assign wbs_dat_o[31] = io_oeb[27];
    assign wbs_dat_o[30] = io_oeb[27];
    assign wbs_dat_o[29] = io_oeb[27];
    assign wbs_dat_o[28] = io_oeb[27];
    assign wbs_dat_o[27] = io_oeb[27];
    assign wbs_dat_o[26] = io_oeb[27];
    assign wbs_dat_o[25] = io_oeb[27];
    assign wbs_dat_o[24] = io_oeb[27];
    assign wbs_dat_o[23] = io_oeb[27];
    assign wbs_dat_o[22] = io_oeb[27];
    assign wbs_dat_o[21] = io_oeb[27];
    assign wbs_dat_o[20] = io_oeb[27];
    ...
    

    When used as is, netgen generates results indicating that the internal netlist has been corrupted.

    Net: _noconnect_4_
    sky130_fd_sc_hd__a22o_1/X = 4
    sky130_fd_sc_hd__and2_1/X = 18
    sky130_fd_sc_hd__and3b_1/X = 4
    sky130_fd_sc_hd__conb_1/HI = 1
    sky130_fd_sc_hd__mux2_2/A0 = 16
    sky130_fd_sc_hd__mux2_2/A1 = 16
    sky130_fd_sc_hd__nand4_1/C = 1
    sky130_fd_sc_hd__nand4_1/D = 1
    sky130_fd_sc_hd__or4bb_1/C_N = 1
    sky130_fd_sc_hd__sdfrtn_1/D = 3
    sky130_fd_sc_hd__sdfrtn_1/RESET_B = 3
    sky130_fd_sc_hd__sdfrtn_1/SCD = 3
    sky130_sram_1kbyte_1rw1r_32x256_8/csb1 = 3
    sky130_sram_1kbyte_1rw1r_32x256_8/din0[0:14] = 3
    sky130_sram_1kbyte_1rw1r_32x256_8/din0[25:28] = 3
    sky130_sram_1kbyte_1rw1r_32x256_8/wmask0[0:3] = 3
    

    These terminals are not connected in either the layout or verilog. _noconnect_4_ is a net name generated by netgen.

    Moving the assign statements to the top level user_project_wrapper level yields expected results.

  2. VWPR is a typo that only exists in user_proj_example. Moving this assign statement to the top level causes a segmentation error.
    It appears that assign statements to non-existent nets cause a segfault.

    (Note: user_project_wrapper.v has been modified from the original data to connect power to user_project_example.)

Test case

test_netgen.tgz

tar xzf test_netgen.tgz
cd test_netgen
netgen -batch source lvs.script.error
netgen -batch source lvs.script.fix
netgen -batch source lvs.script.flatten
netgen -batch source lvs.script.segfault
  • lvs.script.error uses the original verilog and will produce lvs.report.error which contains the _noconnect 4_ net explained above.
  • lvs.scrip.fix uses verilog that has been modified by moving the assign statements from user_proj_example to user_project_wrapper. This yields the expected results in lvs.report.fix. The sram power is not connected at the
    user_proj_example level, so there are many errors at that level.
  • lvs.script.flatten uses the original verilog but explicitly flattens the user_proj_example before comparison. It also yields expected results in lvs.report.flatten (no difference with lvs.report.fix at the user_project_wrapper level).
  • lvs.script.segfault has an assign statement to a non-existing net. It segfaults without any output files.

In summary, the work-around is to explicitly flatten any verilog submodules that have assign statements.

@RTimothyEdwards
Copy link
Owner

I have fixed the segfault issue, although generally speaking, netgen is not expected to do syntax checking on a netlist; the netlist should be assumed to be syntactically correct. Especially as it is difficult to code in checks for every type of error, and LVS is something you're supposed to be doing between two netlists that are produced by tools incapable of generating bad syntax. Anyway, I just fix them as you find them. . . Now, on to the more complicated issue. . .

@RTimothyEdwards
Copy link
Owner

@d-m-bailey : If you look at the netgen output, the first (and underlying) error is that the SRAMs are not connected to the power supply (e.g., NJ_user_project_wrapper.gds.spice line 60058. Probably all other errors are related.

@RTimothyEdwards
Copy link
Owner

RTimothyEdwards commented Sep 1, 2023

But okay, I get your point about the _noconnect_4_ net that has a completely bogus connectivity.

@d-m-bailey
Copy link
Contributor Author

d-m-bailey commented Sep 1, 2023

@d-m-bailey : If you look at the netgen output, the first (and underlying) error is that the SRAMs are not connected to the power supply (e.g., NJ_user_project_wrapper.gds.spice line 60058. Probably all other errors are related.

The sram's do not have power connections at the user_project_example level, but there are connections at the user_project_wrapper level (across hierarchies). This design is not open source tool friendly - all levels do not pass LVS independently. Dealing with that is an entirely different issue but can be mitigated by flattening the user_proj_example explicitly in netgen. See lvs.report.flatten.

@RTimothyEdwards
Copy link
Owner

@d-m-bailey : I haven't looked at the layout, only at the layout netlist, but the layout netlist is pretty clear that the SRAM power is unconnected. If the SRAM in the layout was black-boxed, then it could be that the power supplies were connected at a point not marked as a pin, otherwise the extraction is being done wrong somehow.

@RTimothyEdwards
Copy link
Owner

@d-m-bailey : Tentatively, this is fixed (netgen version 1.5.257). There was a block of code that moves shorted pins together. What it ends up doing is changing the pin order of the cell with respect to the pin order of instances of the same cell, and all the pin number gets messed up. However, I'm not sure if there is any code (if there is, it would be in MatchPins()) that depends on shorted pins being adjacent. If so, this code change might prevent pin matching where there are shorted pins. Since this example fails anyway, I can't tell if I have caused an issue with pin matching.

@RTimothyEdwards
Copy link
Owner

Follow-up on this: Issue #87 is exactly what I was concerned about above: An issue caused by not moving shorted pins together. That is now fixed, and I think that is the only other place in the code dependent on the shorted pins being together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants