Instance flattening (discussion)

[d-m-bailey]

Observations:

• Prematching flattens cells that don't have a match in the other netlist. This may cause long run times when comparing fully extracted layout to blackbox srams, for example. Proposal: Eliminate initial flattening of subcircuits with no corresponding names. edited
• After flattening an instance, the cell list for the parent cell is reprocessed from the beginning. O(n^2) Proposal: after flattening, reprocess cell list from last valid pointer. O(2n)
• Reassigning node numbers for flattened instance is done once per signal for each subinstance. O(n^2) Proposal: node numbers for flattened instances can be increased by an offset. O(n)
• Flattening a specific cell does a cell lookup for the same cell for each cell match. O(n) Proposal: Do one lookup for each subcircuit. O(1)

[RTimothyEdwards]

There are many instances in which flattening unmatched cells is mandatory. I've come across too many designs (including a number in the SkyWater PDK) that have cells in the layout with the same name as cells in the compared netlist, and yet they are not a topological match, and can only be matched by flattening. This happens so often that I've come to take it as a given that flattening to resolve mismatches is a necessity. I think the first issue is to eliminate the computational inefficiency of the flattening. The time it takes to flatten should not be the deciding issue. The main issue should be that it's a lot harder to figure out what's going on in the LVS report when all the cells have been dissolved down to their constituent transistors. A better approach might be to transform one cell to match the other, which might include separating some nets, or pulling out some devices; reporting what transformations were done to force a match, and keep going with the subcells altered but intact. I don't have a good feeling for how complicated that would be to implement.

[d-m-bailey]

I think we're on the same page here. The wording of the first proposal was confusing. I'm suggesting eliminating the initial flattening of cells that don't have corresponding names before LVS (FlattenUnmatched) and not the flattening of circuits that fail LVS match (flattenInstancesOf). There is a -noflatten option that appears to be used to explicitly stop flattening of specific cells. I don't think it's currently incorporated into the flow, though.

[RTimothyEdwards]

Netgen only compares by name matching, so cells that don't match by name are going to be mismatched by definition, so flattening up front before any comparison is more efficient. I do know the case with the SRAM that you are referring to, and the problem is that there is a procedure done up front that traverses the entire subcircuit hierarchy and flattens everything that doesn't match by name. The problem is that if one of the netlists has a lot of black-box entries and the other doesn't, then the one that doesn't gets traversed and everything gets flattened. The solution here, then, is to keep the procedure of flattening cells that don't match by name, but either (1) don't do it all at once at the beginning, but at each comparison, or (2) specifically watch for black-box entries in one netlist, and ignore everything inside those cells in the other netlist. That would prevent it from flattening cells that will never be seen during the comparison.

[d-m-bailey]

How about (1) don't do flattening for cells with unmatched (or unequated) names at the beginning AND (2) only flatten during comparison if the other netlist contains devices?

[RTimothyEdwards]

Okay, I implemented that; new version is 1.5.212 and seems to do the right thing. If that looks good, I can just remove the FlattenUnmatched() routine altogether. It is something that I wrote before I wrote the pre-match code. The pre-match code does more and is more efficient, so I agree with you that there is no longer any need for it. I also added the check for one side being a black-box circuit in PrematchLists() which prevents the unnecessary flattening of cells that will never be compared.

[d-m-bailey]

Excellent! I'll test 1.5.212.

I attempted dynamic hash resizing, but it's segfaulting on the caravel storage block when releasing the hash for a placeholder.

Also, I'm thinking that maybe the FlattenUnmatched routine can be modified to remove disconnected pins in circuits that aren't black boxes during preprocessing. Currently, cells are examined deepest first, but I think that can lead to problems with cells that are used in multiple levels of the hierarchy if a higher level instance is encountered before the lowest level instance. There's Recurse routine called that processes all the instances, including the previously encountered ones which may be in hash lists that are currently being processed somewhere in the call stack. Maybe a flag in the Cell hash that could be reset before processing so that pin removal is only executed once per cell and not once per instance. That way only the first instance encountered would need to be processed.

Do you know of any need to keep disconnected pins? I believe you mentioned an unconnected port being discovered in one example. Might be wise to leave a log of what ports were removed from each cell.

[d-m-bailey]

Haven't tested 1.5.212 directly, but noticed similar changes that I made aren't working in some cases. For example, in the caravel's storage macro, the layout side contains a pk_sram_1rw1r_32_256_8_sky130 cell under sram_1rw1r_32_256_8_sky130 which is not getting flattened. Previously, I believe this was flattened in the initial FlattenUnmatched routine. I should be able to provide more feedback today.

[d-m-bailey]

With 1.5.212, I get the same result. Expecting pk_sram_1rw1r_32_256_8_sky130 to be flattened, but it isn't.

Contents of circuit 1:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 1 device instances.
  Class: pk_sram_1rw1r_32_256_8_sky130 instances:   1
Circuit contains 125 nets.
Contents of circuit 2:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 87953 device instances.
  Class: sky130_fd_pr__nfet_01v8 instances: 612
  Class: sky130_fd_pr__special_pfet_pass instances: 33672
  Class: pand2_0               instances:   2
  Class: pinv_0                instances:   2
  Class: pinv_dec              instances: 302
  Class: pnand2_1              instances:   1
  Class: single_level_column_mux_array_0 instances:   1
  Class: pinv_dec_0            instances: 256
  Class: pinvbuf               instances:   2
  Class: dff                   instances:  14
  Class: data_dff              instances:   1
  Class: pdriver_1             instances:   1
  Class: pdriver_2             instances:   1
  Class: pdriver_5             instances:   1
  Class: pdriver_6             instances:   1
  Class: sky130_fd_pr__special_nfet_latch instances: 51872
  Class: write_mask_and_array  instances:   1
  Class: dff_buf_array_0       instances:   1
  Class: sky130_fd_pr__pfet_01v8 instances: 612
  Class: dff_buf_array         instances:   1
  Class: precharge_array_0     instances:   1
  Class: wmask_dff             instances:   1
  Class: pinv_20               instances:  45
  Class: col_addr_dff          instances:   2
  Class: pand3_0               instances:   1
  Class: nand3_dec             instances: 272
  Class: single_level_column_mux_array instances:   1
  Class: nand2_dec             instances: 272
  Class: precharge_array       instances:   1
  Class: pand3                 instances:   1
Circuit contains 19403 nets.

Circuit 1 contains 1 devices, Circuit 2 contains 87953 devices. *** MISMATCH ***
Circuit 1 contains 125 nets,    Circuit 2 contains 19399 nets. *** MISMATCH ***

Flattened mismatched instances and attempting compare again.

Contents of circuit 1:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 1 device instances.
  Class: pk_sram_1rw1r_32_256_8_sky130 instances:   1
Circuit contains 125 nets.
Contents of circuit 2:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 88237 device instances.
  Class: sky130_fd_pr__nfet_01v8 instances: 625
  Class: sky130_fd_pr__special_pfet_pass instances: 33672
  Class: pand2_0               instances:   2
  Class: pinv_0                instances:   2
  Class: pinv_6                instances:   3
  Class: single_level_column_mux instances:  64
  Class: pinv_dec              instances: 302
  Class: pnand2_1              instances:   1
  Class: pinv_dec_0            instances: 256
  Class: pinvbuf               instances:   2
  Class: dff                   instances:  14
  Class: data_dff              instances:   1
  Class: pdriver_6             instances:   1
  Class: sky130_fd_pr__special_nfet_latch instances: 51872
  Class: single_level_column_mux_0 instances:  64
  Class: dff_buf_0             instances:   2
  Class: dff_buf_array_0       instances:   1
  Class: sky130_fd_pr__pfet_01v8 instances: 625
  Class: precharge_0           instances:  65
  Class: precharge_1           instances:  65
  Class: wmask_dff             instances:   1
  Class: pinv_20               instances:  45
  Class: col_addr_dff          instances:   2
  Class: pand3_0               instances:   1
  Class: nand3_dec             instances: 272
  Class: nand2_dec             instances: 272
  Class: pand2                 instances:   4
  Class: pand3                 instances:   1
Circuit contains 19416 nets.

Circuit 1 contains 1 devices, Circuit 2 contains 88237 devices. *** MISMATCH ***
Circuit 1 contains 125 nets,    Circuit 2 contains 19412 nets. *** MISMATCH ***

[d-m-bailey]

On case 2 and case 3 of the flattenInstancesOf, I added a check to always flatten unmatched cells (default is to flatten - match = 1, so skipping the loop results in flattening).

@@ -1715,6 +1728,7 @@ PrematchLists(char *name1, int file1, char *name2, int file2)
                        (ecomp->num2 != 0) && (ecomp->cell2->class == CLASS_SUBCKT)) {
            ecomp->add2 = -ecomp->num2;
            match = 1;
+           if (ecomp->num1 > 0) 
            for (ob2 = ecomp->cell2->cell; ob2; ob2 = ob2->next) {
                if (ob2->type == FIRSTPIN) {
                    ncomp = (ECompare *)HashInt2Lookup(ob2->model.class,
@@ -1773,6 +1787,7 @@ PrematchLists(char *name1, int file1, char *name2, int file2)
                        (ecomp->num1 != 0) && (ecomp->cell1->class == CLASS_SUBCKT)) {
            ecomp->add1 = -ecomp->num1;
            match = 1;
+           if (ecomp->num2 > 0)
            for (ob2 = ecomp->cell1->cell; ob2; ob2 = ob2->next) {
                if (ob2->type == FIRSTPIN) {
                    ncomp = (ECompare *)HashInt2Lookup(ob2->model.class,

However, this led to interesting results.

Subcircuit summary:
Circuit 1: sram_1rw1r_32_256_8_sky130      |Circuit 2: sram_1rw1r_32_256_8_sky130
-------------------------------------------|-------------------------------------------
pk_sram_1rw1r_32_256_8_sky130 (1)          |(no matching element)
(no matching element)                      |sky130_fd_pr__special_nfet_latch (51872)
(no matching element)                      |sky130_fd_pr__special_pfet_pass (33672)
(no matching element)                      |precharge_array (1)
(no matching element)                      |sky130_fd_pr__nfet_01v8 (785)
(no matching element)                      |sky130_fd_pr__pfet_01v8 (785)
(no matching element)                      |write_mask_and_array (1)
(no matching element)                      |single_level_column_mux_array (1)
(no matching element)                      |precharge_array_0 (1)
(no matching element)                      |single_level_column_mux_array_0 (1)
(no matching element)                      |pinv_dec (302)
(no matching element)                      |nand2_dec (272)
(no matching element)                      |nand3_dec (272)
(no matching element)                      |pinv_dec_0 (256)
(no matching element)                      |pinvbuf (2)
(no matching element)                      |dff_buf_array (1)
(no matching element)                      |pdriver_1 (1)
(no matching element)                      |pinv_0 (2)
(no matching element)                      |pand2_0 (2)
(no matching element)                      |pdriver_2 (1)
(no matching element)                      |pand3 (1)
(no matching element)                      |pand3_0 (1)
(no matching element)                      |pinv_20 (45)
(no matching element)                      |pnand2_1 (1)
(no matching element)                      |pdriver_5 (1)
(no matching element)                      |col_addr_dff (2)
(no matching element)                      |wmask_dff (1)
(no matching element)                      |data_dff (1)
Number of devices: 1 **Mismatch**          |Number of devices: 88283 **Mismatch**
Number of nets: 125 **Mismatch**           |Number of nets: 19571 **Mismatch**
---------------------------------------------------------------------------------------
Flattening instances of single_level_column_mux_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of write_mask_and_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pk_sram_1rw1r_32_256_8_sky130 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of single_level_column_mux_array_0 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of precharge_array_0 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of dff_buf_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_1 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_2 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_5 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of precharge_array in cell sram_1rw1r_32_256_8_sky130 makes a better match

pk_sram_1rw1r_32_256_8_sky130 is flattened as intended, but so are other cells from the source side. For example, precharge_array_0. And then in the next iteration

Subcircuit summary:
Circuit 1: sram_1rw1r_32_256_8_sky130      |Circuit 2: sram_1rw1r_32_256_8_sky130
-------------------------------------------|-------------------------------------------
wmask_dff (1)                              |wmask_dff (1)
data_dff (1)                               |data_dff (1)
pand3_0 (1)                                |pand3_0 (1)
dff_buf_array (1)                          |(no matching element)
pdriver_5 (1)                              |(no matching element)
pand3 (1)                                  |pand3 (1)
pand2_0 (2)                                |pand2_0 (2)
pdriver_1 (1)                              |(no matching element)
pinv_20 (45)                               |pinv_20 (45)
pnand2_1 (1)                               |pnand2_1 (1)
pdriver_2 (1)                              |(no matching element)
pinv_0 (2)                                 |pinv_0 (2)
sky130_fd_pr__pfet_01v8 (785)              |sky130_fd_pr__pfet_01v8 (798) **Mismatch**
sky130_fd_pr__nfet_01v8 (785)              |sky130_fd_pr__nfet_01v8 (798) **Mismatch**
col_addr_dff (2)                           |col_addr_dff (2)
precharge_array_0 (1)                      |(no matching element)
single_level_column_mux_array_0 (1)        |(no matching element)
single_level_column_mux_array (1)          |(no matching element)
precharge_array (1)                        |(no matching element)
write_mask_and_array (1)                   |(no matching element)
pinvbuf (2)                                |pinvbuf (2)
pinv_dec (302)                             |pinv_dec (302)
nand3_dec (272)                            |nand3_dec (272)
nand2_dec (272)                            |nand2_dec (272)
pinv_dec_0 (256)                           |pinv_dec_0 (256)
sky130_fd_pr__special_nfet_latch (51872)   |sky130_fd_pr__special_nfet_latch (51872)
sky130_fd_pr__special_pfet_pass (33672)    |sky130_fd_pr__special_pfet_pass (33672)
(no matching element)                      |precharge_0 (65)
(no matching element)                      |pand2 (4)
(no matching element)                      |single_level_column_mux (64)
(no matching element)                      |precharge_1 (65)
(no matching element)                      |single_level_column_mux_0 (64)
(no matching element)                      |dff_buf_0 (2)
(no matching element)                      |pinv_6 (3)
Number of devices: 88283 **Mismatch**      |Number of devices: 88567 **Mismatch**
Number of nets: 19571 **Mismatch**         |Number of nets: 19584 **Mismatch**
---------------------------------------------------------------------------------------
Flattening instances of single_level_column_mux_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of write_mask_and_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of single_level_column_mux_array_0 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of precharge_array_0 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of dff_buf_array in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_1 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_2 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of pdriver_5 in cell sram_1rw1r_32_256_8_sky130 makes a better match
Flattening instances of precharge_array in cell sram_1rw1r_32_256_8_sky130 makes a better match

precharge_array_0 is flattened in the layout side. Not wrong, but unnecessary if the source were to remain unflattened.

[d-m-bailey]

Maybe this is a better approach, but it gives the same results.

@@ -1715,6 +1728,7 @@ PrematchLists(char *name1, int file1, char *name2, int file2)
                        (ecomp->num2 != 0) && (ecomp->cell2->class == CLASS_SUBCKT)) {
            ecomp->add2 = -ecomp->num2;
            match = 1;
+           if (ecomp->cell2->flags & CELL_MATCHED)  // unmatched cells automatically flattened
            for (ob2 = ecomp->cell2->cell; ob2; ob2 = ob2->next) {
                if (ob2->type == FIRSTPIN) {
                    ncomp = (ECompare *)HashInt2Lookup(ob2->model.class,
@@ -1773,6 +1787,7 @@ PrematchLists(char *name1, int file1, char *name2, int file2)
                        (ecomp->num1 != 0) && (ecomp->cell1->class == CLASS_SUBCKT)) {
            ecomp->add1 = -ecomp->num1;
            match = 1;
+           if (ecomp->cell1->flags & CELL_MATCHED)  // unmatched cells automatically flattened
            for (ob2 = ecomp->cell1->cell; ob2; ob2 = ob2->next) {
                if (ob2->type == FIRSTPIN) {
                    ncomp = (ECompare *)HashInt2Lookup(ob2->model.class,

[d-m-bailey]

There was a change incorporated not to long ago that flattened cells that didn't have a match. This was related to the sram control_logic_r cell being completely flattened in the layout but having a hierarchy with depth greater than 1 in the netlist. The change to flatten non matching cells fixed that problem, but unnecessarily flattens some cells which do have a correspondence at a lower level (the example above). I get the feeling that there is no trivial solution. Should SurveyCell return the whole hierarchy? Should unmatched cells in the current hierarchy only be flattened if the other hierarchy contains devices only?

[RTimothyEdwards]

Okay, my expectation was that the last modification would not change the behavior. If there are cells that are not being flattened, then something is not quite right with the code. I think that in principle, the approach that you suggested and that I implemented (or tried to, anyway) is the correct approach. I will debug from there, but if I can't figure it out immediately, I'm going to revert to the previous commit and re-apply the last change as a new branch so I can continue working on it without affecting the main distribution.

[RTimothyEdwards]

I think I just missed putting the "flatten-non-name-matching cells" routine into the PrematchLists() routine which is where it needs to be to flatten non-name-matching cells. I just fixed this---actually it needs to be done between a check for black-box entries and PrematchLists(). Apart from a new bug in magic that prefers the substrate name over the ground net name, netgen does appear to be working now, generating the same results as before but without unneeded flattening.

[RTimothyEdwards]

Scratch the part about the bug in magic---the bug was in my shell script (used Makefile syntax in a bash script).

[RTimothyEdwards]

Another idea: Expand prematch so that if there are two cells, one in netlist 1 and one in netlist 2, with different cell names, but they both contain the same number of the same kind of device, and no other cells contain the same number of the same kind of device, then attempt to match them. In the worst case, if they really shouldn't be matched, then they will fail matching and get flattened as they would have anyway. In the best case, this method "discovers" matching cells with different names in the two netlists. It would, for example, handle the case in the SRAM where the layout cells are prefixed with sky130-something and the same cells on the "schematic" side aren't, without resorting to flattening them all.

[d-m-bailey]

I'm compiling 83dce151d87e909da6414632fa784dcadf5adcb0 now (the version number is still at 1.5.212) and will use that for the old caravel storage module.

I'd attempted to do the noncorresponding cell flattening in SurveyCell, but that didn't work as well as I'd hoped. It's not very intuitive either.

What's the reasoning behind the deepest first flattening in FlattenUnmatched? I wonder why a simpler recursion wouldn't be sufficient. I think deepest first can lead to problems when a cell is used at multiple levels of the hierarchy.

[d-m-bailey]

Commit 83dce15 ended with a segfault.

Contents of circuit 1:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 1 device instances.
  Class: pk_sram_1rw1r_32_256_8_sky130 instances:   1
Circuit contains 125 nets.
Contents of circuit 2:  Circuit: 'sram_1rw1r_32_256_8_sky130'
Circuit sram_1rw1r_32_256_8_sky130 contains 87989 device instances.
  Class: sky130_fd_pr__nfet_01v8 instances: 631
  Class: sky130_fd_pr__special_pfet_pass instances: 33672
  Class: pand2_0               instances:   2
  Class: pinv_0                instances:   2
  Class: pinv_dec              instances: 302
  Class: pnand2_1              instances:   1
  Class: single_level_column_mux_array_0 instances:   1
  Class: pinv_dec_0            instances: 256
  Class: pinvbuf               instances:   2
  Class: dff                   instances:  14
  Class: data_dff              instances:   1
  Class: pdriver_1             instances:   1
  Class: pdriver_2             instances:   1
  Class: pdriver_5             instances:   1
  Class: sky130_fd_pr__special_nfet_latch instances: 51872
  Class: write_mask_and_array  instances:   1
  Class: sky130_fd_pr__pfet_01v8 instances: 631
  Class: dff_buf_array         instances:   1
  Class: precharge_array_0     instances:   1
  Class: wmask_dff             instances:   1
  Class: pinv_20               instances:  45
  Class: col_addr_dff          instances:   2
  Class: pand3_0               instances:   1
  Class: nand3_dec             instances: 272
  Class: single_level_column_mux_array instances:   1
  Class: nand2_dec             instances: 272
  Class: precharge_array       instances:   1
  Class: pand3                 instances:   1
Circuit contains 19422 nets.

Circuit 1 contains 1 devices, Circuit 2 contains 87989 devices. *** MISMATCH ***
Circuit 1 contains 125 nets,    Circuit 2 contains 19417 nets. *** MISMATCH ***

Segmentation fault (core dumped)

[d-m-bailey]

Here's some gdb output. I'll try again with the new 1.5.213.

(gdb) bt
#0  0x00007febd69b1079 in vfprintf () from /lib64/libc.so.6
#1  0x00007febd69d543b in vsprintf () from /lib64/libc.so.6
#2  0x00007febd69b75d7 in sprintf () from /lib64/libc.so.6
#3  0x00007febd3f3bd43 in SortFanoutLists (nlist1=0x1dca7e90, nlist2=0x1dca4800) at netcmp.c:1233
#4  0x00007febd3f3c291 in SortUnmatchedLists (nlists1=nlists1@entry=0x102993c10, nlists2=nlists2@entry=0x102997c20, n1max=n1max@entry=125, n2max=n2max@entry=19261)
    at netcmp.c:1390
#5  0x00007febd3f3e160 in FormatIllegalNodeClasses () at netcmp.c:1932

from netcmp.c

1228        else {
1229            matched = (int *)CALLOC(nlist1->fanout, sizeof(int));
1230        total = 0;
1231    
1232            for (f1 = 0; f1 < nlist1->fanout; f1++) {
1233            sprintf(pinname, "%s/%s", nlist1->flist[f1].model,
1234                nlist1->flist[f1].name);
1235            HashPtrInstall(pinname, (void *)((long)f1 + 1), &f1hash);
1236        }
(gdb) p f1
$1 = 0
(gdb) p *nlist1
$3 = {name = 0x1db7a5f0 "din0[23]", fanout = 1, flist = 0x1dca7c10}
(gdb) p *(nlist1->flist)
$4 = {model = 0x1db79aa0 "\260\320\267\035", name = 0x400000003 <Address 0x400000003 out of bounds>, permute = 1 '\001', count = 1}

[RTimothyEdwards]

Yes, version 1.5.212 is known to segfault if it hits the section where prematch finds something to optimize. Have you had a chance to try 1.5.213?

[d-m-bailey]

1.5.213 runs to completion on the storage module of caravel, but LVS does not pass although the device and net counts are the same.

Contents of circuit 1:  Circuit: 'storage'
Circuit storage contains 176070 device instances.
  Class: sky130_fd_pr__nfet_01v8 instances: 1262
  Class: sky130_fd_pr__special_pfet_pass instances: 67343
  Class: pand2_0               instances:   4
  Class: sky130_fd_sc_hd__buf_4 instances:   1
  Class: sky130_fd_sc_hd__buf_8 instances:   2
  Class: pinv_0                instances:   4
  Class: sky130_fd_sc_hd__conb_1 instances:  10
  Class: pinv_dec              instances: 604
  Class: pnand2_1              instances:   2
  Class: single_level_column_mux_array_0 instances:   2
  Class: sky130_fd_sc_hd__decap_3 instances:   1
  Class: sky130_fd_sc_hd__decap_4 instances:   1
  Class: sky130_fd_sc_hd__decap_6 instances:   1
  Class: sky130_fd_sc_hd__decap_8 instances:   1
  Class: pinv_dec_0            instances: 512
  Class: pinvbuf               instances:   4
  Class: dff                   instances:  28
  Class: data_dff              instances:   2
  Class: pdriver_1             instances:   2
  Class: pdriver_2             instances:   2
  Class: pdriver_5             instances:   2
  Class: sky130_fd_pr__special_nfet_latch instances: 103744
  Class: write_mask_and_array  instances:   2
  Class: sky130_fd_pr__pfet_01v8 instances: 1262
  Class: dff_buf_array         instances:   2
  Class: precharge_array_0     instances:   2
  Class: wmask_dff             instances:   2
  Class: pinv_20               instances:  90
  Class: sky130_fd_sc_hd__diode_2 instances:  75
  Class: col_addr_dff          instances:   4
  Class: pand3_0               instances:   2
  Class: nand3_dec             instances: 544
  Class: single_level_column_mux_array instances:   2
  Class: nand2_dec             instances: 544
  Class: precharge_array       instances:   2
  Class: sky130_fd_sc_hd__decap_12 instances:   1
  Class: pand3                 instances:   2
Circuit contains 38821 nets.
Contents of circuit 2:  Circuit: 'storage'
Circuit storage contains 176070 device instances.
  Class: sky130_fd_pr__nfet_01v8 instances: 1262
  Class: sky130_fd_pr__special_pfet_pass instances: 67343
  Class: pand2_0               instances:   4
  Class: sky130_fd_sc_hd__buf_4 instances:   1
  Class: sky130_fd_sc_hd__buf_8 instances:   2
  Class: pinv_0                instances:   4
  Class: sky130_fd_sc_hd__conb_1 instances:  10
  Class: pinv_dec              instances: 604
  Class: pnand2_1              instances:   2
  Class: single_level_column_mux_array_0 instances:   2
  Class: sky130_fd_sc_hd__decap_3 instances:   1
  Class: sky130_fd_sc_hd__decap_4 instances:   1
  Class: sky130_fd_sc_hd__decap_6 instances:   1
  Class: sky130_fd_sc_hd__decap_8 instances:   1
  Class: pinv_dec_0            instances: 512
  Class: pinvbuf               instances:   4
  Class: dff                   instances:  28
  Class: data_dff              instances:   2
  Class: pdriver_1             instances:   2
  Class: pdriver_2             instances:   2
  Class: pdriver_5             instances:   2
  Class: sky130_fd_pr__special_nfet_latch instances: 103744
  Class: write_mask_and_array  instances:   2
  Class: sky130_fd_pr__pfet_01v8 instances: 1262
  Class: dff_buf_array         instances:   2
  Class: precharge_array_0     instances:   2
  Class: wmask_dff             instances:   2
  Class: pinv_20               instances:  90
  Class: sky130_fd_sc_hd__diode_2 instances:  75
  Class: col_addr_dff          instances:   4
  Class: pand3_0               instances:   2
  Class: nand3_dec             instances: 544
  Class: single_level_column_mux_array instances:   2
  Class: nand2_dec             instances: 544
  Class: precharge_array       instances:   2
  Class: sky130_fd_sc_hd__decap_12 instances:   1
  Class: pand3                 instances:   2
Circuit contains 38813 nets.

Circuit 1 contains 176070 devices, Circuit 2 contains 176070 devices.
Circuit 1 contains 38803 nets,    Circuit 2 contains 38803 nets.

Result: Netlists do not match.
Logging to file "~/mpw-2/caravel-lvs/openlane/storage/runs/cvc/results/lvs/storage.lvs.gds.log" disabled
LVS Done.
LVS reports:
    net count difference = 0
    device count difference = 0
    unmatched nets = 805
    unmatched devices = 150
    unmatched pins = 0
    property failures = 0

Total errors = 955

[RTimothyEdwards]

Can you post the script or sequence of commands for the extraction, so I can make sure I can duplicate this exactly?

[RTimothyEdwards]

Also the commit ID and URL where the storage.gds layout comes from. I'm working from my caravel MPW-two branch which I think should match what you used, but I want to make sure.

[d-m-bailey]

The following should work, but let me know if you run into trouble. Takes a couple hours to extract and compare.

https://github.com/efabless/caravel.git commit 40f091a8f84fd2bd5d80ade98f6b401e80aa6c68

Changes to storage/config.tcl

@@ -73,3 +73,8 @@ set ::env(EXTRA_LEFS) "\

 set ::env(EXTRA_GDS_FILES) "\
        $script_dir/../../gds/sram_1rw1r_32_256_8_sky130.gds"
+
+set ::env(LVS_EXTRA_STD_CELL_LIBRARY) "
+       \$::env(PDK_ROOT)/\$::env(PDK)/libs.ref/sky130_sram_macros/spice/sram_1rw1r_32_256_8_sky130.spice"
+
+

I changed the bus indices in sram_1rw1r_32_256_8_sky130.spice to be in reverse order.

bash-4.2$ magic -dnull -noc
Magic 8.3 revision 241 - Compiled on Tue Dec 14 08:37:27 UTC 2021.
bash-4.2$ netgen -batch
Netgen 1.5.213 compiled on Mon Dec 20 11:56:45 UTC 2021

After making the addition to storage/config.tcl, in docker I execute the following:

cd openlane
gunzip ../gds/storage.gds
flow.tcl -design storage -tag cvc -lvs -gds ../gds/storage.gds -net ../verilog/gl/storage.v

[RTimothyEdwards]

@d-m-bailey : Can you please post the LVS output file?

[d-m-bailey]

Sure. This is with format 60. Looks like the ground connection counts to the memory cells source|drain don't match. storage.lvs.gds.log.gz

[d-m-bailey]

I may have modified the spice file for the sram macro. Here's my current version. sram_1rw1r_32_256_8_sky130.spice.gz

Previously, I got the storage module to pass LVS by deleting the parasitic mosfets in both the layout and source. This time, I'm attempting to include the parasistic mosfets.

[RTimothyEdwards]

Thanks. There are two issues, actually, the other of which is that the SRAM block from MPW-one is no longer in the library as it has been superseded by the "approved" version, so thanks for posting the netlist as well.

[RTimothyEdwards]

So looking at this, a major issue that I see is that the bit cell overlaps two of the "drainOnly" devices. This is a terrible thing to do in a layout, because the cell itself has 16 devices, not the 14 in the compared netlist. Magic has no way of describing these devices other than (1) flagging it as an error that there are transistor gates overlapping between cells, and (2) extracting them as devices that are in parallel. I don't think magic is guaranteed to understand or extract that geometry in any meaningful way. At best, it would assume they are devices in parallel and get the W/L wrong, which could be adjusted in the opposing netlist.

[RTimothyEdwards]

On closer inspection of the netlists, it appears that magic disagrees with the interpretation of the parasitic nFET. The extracted netlist has the source and drain of the device at bl1 and gnd (or br1 and gnd). The opposing netlist has the source and drain of the device at gnd and gnd. Since the device is parasitic and the gate is grounded, I guess this is pretty subjective, but I would personally go with the interpretation that came from the extracted netlist. I could probably figure some way to make magic extract the device the way it is written in the netlist, though. But the bottom line is that on the face of it, this does not appear to be a netgen issue, which should be able to be proven by modifying the bitcell netlists to change the source/drain connections of the "drainOnly NMOS" devices (bitcell, dummy bitcell, replica bitcell)

[d-m-bailey]

The newer version of the sram passes LVS (by flattening all the memory cells), so I'm going to try putting the newer version of the parasitic mosfet into the source for the storage module.

Before (old):

X12 bl1 gnd bl1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1
X14 br1 gnd br1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1

After (new):

X12 gnd gnd bl1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1
X14 gnd gnd br1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1

X12a gnd gnd bl1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1
X14a gnd gnd br1 gnd sky130_fd_pr__special_nfet_latch W=0.21 L=0.08 m=1 mult=1

[d-m-bailey]

The previous changes yield a LVS match, but the parasitic pmos have size errors.

There were property errors.
sky130_fd_bd_sram__openram_dp_cell_replica_0/sky130_fd_pr__special_pfet_pass4 vs. sky130_fd_bd_sram__openram_dp_cell_replicarbc_2/sky130_fd_pr__special_pfet_pass10:
 l circuit1: 0.15   circuit2: 0.08   (delta=60.9%, cutoff=1%)
 w circuit1: 0.07   circuit2: 0.14   (delta=66.7%, cutoff=1%)

I'll adjust the source and rerun.

[d-m-bailey]

Success! :) 2 hours. :(

Contents of circuit 1:  Circuit: 'storage'
Circuit storage contains 95 device instances.
  Class: sky130_fd_sc_hd__buf_4 instances:   1
  Class: sky130_fd_sc_hd__buf_8 instances:   2
  Class: sky130_fd_sc_hd__conb_1 instances:  10
  Class: sky130_fd_sc_hd__decap_3 instances:   1
  Class: sky130_fd_sc_hd__decap_4 instances:   1
  Class: sky130_fd_sc_hd__decap_6 instances:   1
  Class: sky130_fd_sc_hd__decap_8 instances:   1
  Class: sram_1rw1r_32_256_8_sky130 instances:   2
  Class: sky130_fd_sc_hd__diode_2 instances:  75
  Class: sky130_fd_sc_hd__decap_12 instances:   1
Circuit contains 215 nets.
Contents of circuit 2:  Circuit: 'storage'
Circuit storage contains 95 device instances.
  Class: sky130_fd_sc_hd__buf_4 instances:   1
  Class: sky130_fd_sc_hd__buf_8 instances:   2
  Class: sky130_fd_sc_hd__conb_1 instances:  10
  Class: sky130_fd_sc_hd__decap_3 instances:   1
  Class: sky130_fd_sc_hd__decap_4 instances:   1
  Class: sky130_fd_sc_hd__decap_6 instances:   1
  Class: sky130_fd_sc_hd__decap_8 instances:   1
  Class: sram_1rw1r_32_256_8_sky130 instances:   2
  Class: sky130_fd_sc_hd__diode_2 instances:  75
  Class: sky130_fd_sc_hd__decap_12 instances:   1
Circuit contains 215 nets.

Circuit 1 contains 95 devices, Circuit 2 contains 95 devices.
Circuit 1 contains 215 nets,    Circuit 2 contains 215 nets.

Netlists match uniquely.
Result: Circuits match uniquely.
Logging to file "/home/user/mpw-2/caravel-lvs/openlane/storage/runs/cvc/results/lvs/storage.lvs.gds.log" disabled
LVS Done.
LVS reports no net, device, pin, or property mismatches.

Total errors = 0
[INFO]: No LVS mismatches.

[RTimothyEdwards]

We have made progress at the expense of our sanity.

[RTimothyEdwards]

Now there is a question of how magic came up with W=0.15, L=0.07 for the parasitic pFET, but that's an issue for magic (and has to do with how it scans device perimeters). I'm assuming the "2 hours" figure was mostly from magic's extraction?

Apparently klayout does extraction and LVS now, and there's an LVS deck for klayout for sky130. I'm interested in how klayout deals with the super-ornery layouts like this SRAM bitcell and the GPIO pad.

The solution here for the overlapping FETs would have to be handled by magic, by detecting overlapping devices and flattening the cells that they occur in during extraction. Or use the method of "extresist", for which there is a "killfet" statement in the .ext file that subtracts a device out of a subcell. But again, that's an issue for magic.

[d-m-bailey]

2 hours was netgen LVS only. magic extraction wasn't that long - probably 20-30 minutes.

I just created a PR for netgen that will drop the LVS compare time to under 20 minutes.

[d-m-bailey]

Here's the test data and scripts to run magic extract and netgen compare. test-storage.tar.gz

[RTimothyEdwards]

@d-m-bailey : Currently lvs ... -blackbox doesn't do much of anything; it sets a variable auto_blackbox to TRUE, which in turn sets the class of circuits read from SPICE that have no contents to type "module". What I think it should be doing instead (or in addition) is to affect the behavior of HasContents(), such that HasContents() always returns TRUE if auto_blackbox = FALSE. That will cause empty cells matched against non-empty cells to be flattened instead of forcing both to be treated as black boxes. Probably auto_blackbox should be set to TRUE by default, and then the switch to the lvs command should be -noblackbox. Does that sound sensible to you?

[d-m-bailey]

@RTimothyEdwards I've been trying to think of applicable use cases - when is treating an empty subcircuit as a black-box appropriate and when is it not. Case 1. The same subcircuits exist in both netlists. Both are empty. -> no flattening. match as black boxes. Case 2. The same subcircuits exist in both netlists. Only one is empty. -> could result from LEF based layout or proprietary source. I can't think of a case where it would be beneficial to flatten the non-empty netlist. However, I don't think that black-box matching in this case should always result in a clean LVS result. Maybe something like this:

Netlists match uniquely.
Result: Netlists matched with non-empty black-boxes.

The following subcells were matched as black-boxes:
 sky130_sram(0) non-empty
 sky130_sram(1) empty

Where the -allow_blackbox option would return 0 in this case, but the default would be non-zero.

In other LVS tools, I think you can define cells (with contents) as black-boxes so the LVS will pass even though the black-boxed subcircuits do not. With netgen, the -noflatten option comes close to providing this functionality, but I think it still sets a non-zero return code. This may not be necessary right now.

Netlists match uniquely.
Result: Cells failed matching, or top level cell failed pin matching.

The following subcells failed to match:
 DFFRAM(0)
 DFFRAM(1)

[RTimothyEdwards]

@d-m-bailey : The model <name> blackbox command should be the one to use and would make a cleaner approach, and probably a better one, than the one I used with the lvs -noflatten option.