RTimothyEdwards / magic

Magic VLSI Layout Tool
Other
448 stars 100 forks source link

extraction routine gives different connectivity depending on cif istyle #85

Open d-m-bailey opened 2 years ago

d-m-bailey commented 2 years ago
  1. For the sky130_sram_macros, connectivity is extracted incorrectly (unconnected nets) if cif istyle sky130(vendor) is specified.
  2. If cif istyle sky130(vendor) is omitted, top level ports are not extracted even though port makeall is specified.

To duplicate the problem, extract the attached tar file into a new directory. Using magic 8.3.189 and netgen 1.5.196, execute the following:

./run_ext.sh  # extracts into magic directory without cif istyle sky130(vendor), but with port makeall
./run_ext_cif.sh. # extract into magic-cif directory with cif istyle sky130(vendor). port makeall is also specified, but irrelevant.
./run_lvs.sh  # lvs in the magic directory. Does not pass lvs due to port mismatch (no ports in extracted netlist).
./run_lvs_cif.sh. # lvs in the magic-cif directory. Does not pass lvs due to unconnected vdd in extracted netlist. 
./run_lvs_cif_fix.sh. # lvs in the magic-cif directory using netlist with modified connectivity. passes lvs.

Recently, cif istyle sky130(vendor) was changed to the default, but not in the sky130A.tech file in this example.

You should be able to verify:

  1. magic/sky130_sram_1kbyte_1rw1r_32x256_8.spice has no top subckt ports.
  2. run_lvs.sh did not pass lvs due to mismatched ports. magic/sky130_sram_1kbyte_1rw1r_32x256_8.lvs.report
  3. run_lvs_cif.sh did not pass lvs due to net count mismatch. magic-cif/sky130_sram_1kbyte_1rw1r_32x256_8.lvs.report The following nets in sky130_sram_1kbyte_1rw1r_32x256_8_bitcell_array are unconnected.
    vdd_uq1854 vdd_uq1982 vdd_uq2046 vdd_uq2110 vdd_uq2238
    vdd_uq2302 vdd_uq2366 vdd_uq2430 vdd_uq2494 vdd_uq2558
    vdd_uq2686 vdd_uq2750 vdd_uq2814 vdd_uq2878 vdd_uq3006
    vdd_uq3134 vdd_uq3198 vdd_uq3262 vdd_uq3326 vdd_uq3390
    vdd_uq3454 vdd_uq3518 vdd_uq3582 vdd_uq3646 vdd_uq3710
    vdd_uq3838 vdd_uq3966 vdd_uq4030 vdd_uq4094
  4. run_lvs_cif_fix.sh passes lvs with modified netlist where the above nets have been changed to vdd. magic-cif/sky130_sram_1kbyte_1rw1r_32x256_8.lvs.fix.report

sram_lvs_189_196.tar.gz

RTimothyEdwards commented 2 years ago

@d-m-bailey : Two things that would affect this: (1) I made a correction to magic two days ago that fixes an issue with connectivity through subcells in the SPICE netlist. That error might be implicated here. (2) I made a change to sky130A.tech a few weeks ago to make "sky130(vendor)" the default input style, since after some changes were made to the sky130(vendor) style maybe half a year ago, I no longer have any issues interpreting ports and labels from GDS files, whether they are from magic or elsewhere, so there doesn't appear to be any reason to use the sky130() (previously default) style.

d-m-bailey commented 2 years ago

@RTimothyEdwards The tar file was missing some files (.magicrc and sky130A.tech in the wrong directory). I recreated it with the latest setup and tech files. The title of this issue is now somewhat misleading. The with the current program, the current tech file yields an incorrect extraction.

The magic version I'm using is 8.3.196 (MAGIC_COMMIT=597ef4857a02f1d0748475dcff118801e91b0e88) which should be the most recent.

The tech file that openram was using did not have the vendor style as default. The tar file that I've attached is using a more recent version with vendor as the default. The data is taken, as is, from the sky130_sram_1kbyte_1rw1r_32x256_8 macro of the sky130_sram_macros repo. The run_ext.sh and run_lvs.sh scripts have been modified to write to a subdirectory, magic, and to use the installed versions of magic (8.3.196) and netgen (1.5.196).

./run_ext.sh
./run_lvs.sh
./run_lvs_fix.sh

The run_lvs.sh script does not pass as you can see in the magic/sky130_sram_1kbyte_1rw1r_32x256_8.lvs.report result. However, running run_lvs_fix.sh, which creates a modified netlist magic/sky130_sram_1kbyte_1rw1r_32x256_8.fix.spice, does pass as you can see in magic/sky130_sram_1kbyte_1rw1r_32x256_8.fix.lvs.report.

The netlist modifications are contained in the file vdd.sed and are connecting 29 vdd_uq* signals to vdd.

I was expecting run_lvs.sh to pass.

sram_lvs_196_196.tar.gz

RTimothyEdwards commented 2 years ago

Thanks for putting together the example tarball.

RTimothyEdwards commented 2 years ago

@d-m-bailey : I started off by writing my own version of the extract and LVS scripts. Then, seeing that I was using the default extraction style with the 1.0E6 scalefactor, I removed the "u" from all the dimensions in the LVS netlist. Then I ran extraction and LVS and everything matched. A bit surprised by that, given your result, I then switched back to your LVS netlist and changed my extraction script to use the ngspice(si) extract style. And then I ran extraction and LVS and got the same error as you did. The implication is that it's the extraction style that makes the difference. . . in spite of the fact that the only thing different about the extraction style is the scalefactor! I am now making two sets of all files for runs "good" and "bad", to see if I can see anything in the files that's different other than the scalefactor, and also to make sure that this result can be duplicated.

RTimothyEdwards commented 2 years ago

@d-m-bailey : I figured out why changing the style affects the outcome; the tech setup has an error that multiplies one of the scalefactors twice, such that changing the extract style in any way will change the extraction step size. That doesn't really solve the issue, which I'm aware of and which is caused by the extraction "cookie cutter" method. Solving that error is difficult; fixing the SRAM extraction by fixing the step size calculation and then choosing a step size that doesn't cause the issue to show up when extracting the SRAM is easier.

d-m-bailey commented 2 years ago

@RTimothyEdwards Is it easy to fix the tech file? The previous sram setup used the blank style and extracted the connectivity correctly. I'd like to see if fixing the tech file solves the problem.

RTimothyEdwards commented 2 years ago

@d-m-bailey : In this case, the schematic netlist has SI units for width and length, which makes it effectively unsimulatable. My preference would be to have Matt bring the schematic netlist in line with the SkyWater device models (e.g., "W=0.21", not "W=0.21u"). So you can extract with the default style, but you will get property errors for every transistor unless you change the schematic netlist. If you want it to extract correctly using SI units for length and width, then you can modify the tech file to specify the extract styles as "ngspice variants (si),(orig),()", which would make "ngspice(si)" the default. I want to reiterate that the problem is not with the tech file, the problem is in the magic source code which is (as I think I have figured it out, now) using an old scale value when computing the extraction step size when the extraction style is changed. I am working on fixing that problem now. But then I also want to reiterate that there is a more difficult underlying problem in that the extraction can get confused when a net is connected together in different levels of hierarchy and also extends through several stepped regions. You can see from the example that the problem occurs when using "extract unique", in which the power supply line is seen as six or seven separate nets and is given unique net names, so when the extraction steps through different areas, it sees the power net as, say "vdd_uq1" instead of "vdd", and doesn't merge it into a single net. Possibly there is something I can do to the way the "extract unique" command works that would fix the issue, but I haven't looked into that yet.

RTimothyEdwards commented 2 years ago

@d-m-bailey : Sorry I had to punt on this issue for a few weeks. I just looked into it more closely and realized that whenever the "extract style" command is used to set or change the extraction style, all the dimensional parameters of the extraction get scaled twice, so the extraction result will get screwed up in many ways. This should be fixed now (just pushed a fix to the repository on opencircuitdesign.com).