Pin match

[d-m-bailey]

This removes the proxy pin code and forces pin matching.
Any pin mismatches (even if disconnected) cause cell flattening.

There may be some additional cosmetic changes.

Highly recommend NOT merging as is.

[RTimothyEdwards]

I will not do a direct merge of this pull request. I am breaking up the pull request into two parts, as noted in the commit messages to my pushes. The PinMatch() modifications are significantly changing the behavior of netgen. The rest of the changes look much less controversial. So I will do the PinMatch() modifications in a follow-up commit which will make it easier to deal with if I decide that the proxy pin method needs to be put back for some reason. The first merge is version 1.5.229.

[RTimothyEdwards]

@d-m-bailey : Note that I have already merged your code.

Please take a look at the following case:
From https://github.com/lankasaicharan/vezzal directory testcases/netgen/case6

The case6 failure highlights an issue where the originaly proxy pin handling was correctly resolving the issue but your code doesn't: The "sky130_fd_sc_hd__conb_1" cell, in verilog, can have implicit pins, and in this case only the "HI" pin is enumerated in the verilog, and not the "LO" pin.

Analysis: This test case situation is specific to verilog's (unfortunate) syntax, and adding a proxy pin was my original way of working around the issue, which is why your code undermines this test case.
At issue is the fact that verilog does not require a pin to be explicitly mentioned in an instance if that pin is not connected to anything (as if verilog wasn't already full of dumb syntax ideas). So if a netlist is read without first reading a verilog library of components, then while it looks like all cells should be fully described because each instance has to specify both the pin name and the net name connecting to the pin, it's possible to miss pins in the cell because they were always implicit in the netlist. In case6, the verilog netlist uses cell sky130_fd_sc_hd__conb_1, but only pin HI is connected, so pin LO is never mentioned in the verilog netlist. So the match of these cells results in:

Subcircuit pins:
Circuit 1: sky130_fd_sc_hd__conb_1         |Circuit 2: sky130_fd_sc_hd__conb_1
-------------------------------------------|-------------------------------------------
VGND                                       |VGND
VNB                                        |VNB
VPB                                        |VPB
VPWR                                       |VPWR
HI                                         |HI
LO                                         |**no match**
---------------------------------------------------------------------------------------
Attempt to match empty cell to non-empty cell.

Previously, my proxy pin handling would have said "pin list altered to match" and added a pin "proxyLO" to Circuit2.

I think the correct solution here is to allow the proxy pin to be made, but only for the specific case that the missing pin comes from a verilog placeholder cell, which is the only situation in which it would be allowed to just add a missing pin.

[d-m-bailey]

Doing some regression testing on the mpw-7 projects.

There are a few designs that pass with version 1.5.229 but fail with 1.5.232.

Looks like it has to do with trying to match an empty subcircuit with a non-empty subcircuit. Probably something we want to allow with a warning. Currently, I post process the log file to generate warnings.

Here's 1.5.229 - 1.5.232 vimdiff results

  Subcircuit pins:                                                                  |  Subcircuit pins:
  Circuit 1: wb_bridge_2way                                                         |  Circuit 1: wb_bridge_2way                                                         
  ----------------------------------------------------------------------------------|  ----------------------------------------------------------------------------------
  vccd1                                                                             |  vccd1                                                                             
  wb_clk_i                                                                          |  wb_clk_i (disconnected)                                                           
  wb_rst_i                                                                          |  wb_rst_i (disconnected)                                                           
  wbm_a_ack_i                                                                       |  wbm_a_ack_i                                                                       
  wbm_a_adr_o[0]                                                                    |  wbm_a_adr_o[0]                                                                    
  wbm_a_adr_o[10]                                                                   |  wbm_a_adr_o[10]                                                                   
  wbm_a_adr_o[11]                                                                   |  wbm_a_adr_o[11]                                                                   
  wbm_a_adr_o[12]                                                                   |  wbm_a_adr_o[12]                                                                   
  wbm_a_adr_o[13]                                                                   |  wbm_a_adr_o[13]                                                                   
+ +--281 lines: wbm_a_adr_o[14]                                                     |+ +--281 lines: wbm_a_adr_o[14]                                                     
  wbs_sel_i[2]                                                                      |  wbs_sel_i[2]                                                                      
  wbs_sel_i[3]                                                                      |  wbs_sel_i[3]                                                                      
  wbs_stb_i                                                                         |  wbs_stb_i                                                                         
  wbs_we_i                                                                          |  wbs_we_i                                                                          
  vssd1                                                                             |  vssd1                                                                             
  ----------------------------------------------------------------------------------|  ----------------------------------------------------------------------------------
  Cell pin lists are equivalent.                                                    |  Attempt to match empty cell to non-empty cell.                                    
  Device classes wb_bridge_2way and wb_bridge_2way are equivalent.                  |  ----------------------------------------------------------------------------------
                                                                                    |  
...
  ----------------------------------------------------------------------------------|  ----------------------------------------------------------------------------------
  Cell pin lists are equivalent.                                                    |  Cell pin lists for user_project_wrapper and user_project_wrapper do not match.    
  Device classes user_project_wrapper and user_project_wrapper are equivalent.      |                                                                                    
                                                                                    |  Final result: Netlists do not match.                                              
  Final result: Circuits match uniquely.                                            |  ----------------------------------------------------------------------------------

The new version (my changes), don't equate when one netlist is non-empty.

I think I have a solution (equate empty cells if pins match) and posted a pull request #62 .

[d-m-bailey]

Found another discrepancy when the circuits match but not the top level ports.
The pin_match branch gives

Cell pin lists for user_project_wrapper and user_project_wrapper do not match.

Final result: Circuits match uniquely with port errors.

The following cells had property errors:
 sky130_fd_sc_hd__diode_2

While the current 1.5.232 version gives

Cell pin lists for user_project_wrapper and user_project_wrapper do not match.

Final result: Circuits match uniquely.
.

The following cells had property errors:
 sky130_fd_sc_hd__diode_2

Having the final result message include the port error message makes it simpler to check for a 'perfect' match, I think.
Also the newest version has a single '.' on the line following the 'Final result' message. Is this intentional?

I'll look into a fix and submitting a pull request.

[RTimothyEdwards]

@d-m-bailey : This issue is now stuck in limbo due to the need to implement proxy pins. I'm inclined to revert back to the state before I merged this code and then figure out how to work back in your checks for NC->legalpartition which is the main difference that allows the report to show that a pin matches an internal net that has been disconnected from the intended matching pin. What do you think?

[RTimothyEdwards]

@d-m-bailey: Only 2 1/4 years to get around to fixing this properly (I hope)! Anyway, yesterday I looked into why adding proxy pins was failing to allow a match on trivial cases, and I was surprised to find that it was not what I was expecting at all. I think I have it working now where the proxy pin method works as it should, and the pin matching now reports and handles everything symmetrically regardless of which netlist is 1st and which is 2nd, and also reports the net name of the equivalent net on each side when the net does not connect to a pin. I tested this on the first Chipalooza test chip and it seems to be working as expected. The disconnected substrate pins that magic now creates (which are rather hard to get rid of in magic) are being recognized by netgen as not being connected to any devices, and they get ignored even when the connectivity runs through several levels of hierarchy. This avoids a lot of unnecessary flattening and makes the output much more readable.

[d-m-bailey]

@RTimothyEdwards Excellent (and long awaited) news!