google / skywater-pdk

Open source process design kit for usage with SkyWater Technology Foundry's 130nm node.
https://skywater-pdk.rtfd.io
Apache License 2.0
2.95k stars 384 forks source link

cell_footprint value in liberty files doesn't match name in verilog or GDS. #90

Open mithro opened 4 years ago

mithro commented 4 years ago

The cell_footprint name in the liberty files is just the cell name and not the full <library>__<cellname> used in the Verilog and GDS files.

See https://foss-eda-tools.googlesource.com/skywater-pdk/libs/sky130_fd_sc_hd/+/refs/heads/master/cells/a2111o/sky130_fd_sc_hd__a2111o_1__ff_100C_1v65.lib.json#3

Compared to https://foss-eda-tools.googlesource.com/skywater-pdk/libs/sky130_fd_sc_hd/+/refs/heads/master/cells/a2111o/sky130_fd_sc_hd__a2111o.functional.v#35

mithro commented 4 years ago

@msaligane -- Any idea why you haven't seen this problem?

msaligane commented 4 years ago

@mithro I didn't need to read verilog in my flow but I will need to read it or cdl at some point. I can try to look at this closely this evening.

mithro commented 4 years ago

@msaligane -- I would have thought this would be needed to match the GDS to generate the output?

msaligane commented 4 years ago

It is needed during the LVS checks (or digital functional simulation ). I usually use cdl files for both of these. I can read these files and see what issues I see but I am far from my machine right now.

mithro commented 4 years ago

@msaligane - No worries, it is a weekend after all!

rovinski commented 4 years ago

The only place where cell_footprint really shows up is vt-swapping. The property is used to identify if different cells have the same geometric footprint so that they can be swapped for in-place optimizations (such as swapping an RVT cell with its' LVT version). Only the cell name is used to match the liberty cell to the verilog module.

RTimothyEdwards commented 4 years ago

@mithro : I concur with the above information (also, I have told you this before. . . ). The cell_footprint is just a text string and does not have to match anything other than the name of other cell_footprint records in the same file.

msaligane commented 4 years ago

I agree as well. Based on this description: image (26)

As long as all the sub-cells in the same cell folder(same cell with different drives) have the same "cell_footprint" attribute, it will work. Also, reading the verilog didn't show any errors vs .lib

rovinski commented 4 years ago

@msaligane Do you mean different drive strengths or different VTs? Cells with different drive strengths often have different footprints.

msaligane commented 4 years ago

@rovinski I meant that since the cells are organized in a way that one cell has all its drives and views in one folder, it may be misleading. So I added that for Tim's reference. And yes you are right.

RTimothyEdwards commented 4 years ago

@msaligane : That description is completely at odds to the way I have seen "cell_footprint" used. The way I have seen it used is to assign the same footprint name to cells with the same function; that is, cells that can be swapped without changing the logical function of the circuit. Which doesn't really jibe with the term "footprint", but as I said, that's how I've seen it used, on well-vetted standard cell libraries from trusted foundry sources. According to that usage, cells with different drive strengths all have the same string for "cell_footprint", and the way they are being done now in the SkyWater standard cell libraries (e.g., cell "sky130_fd_sc_hd__a2111o_1" has cell_footprint "a2111o") is perfectly valid.

msaligane commented 4 years ago

@RTimothyEdwards I think the description from synopsys tool is exactly what you are saying. However, as Austin said as well, the cells usually have a different footprint when you change the drive. Logical since you have to increase/decrease the transistors sizes for that. A Vt swap makes more sense here. Anyway, I have run strive with the new libs, I see a small issue with footprint during optimizations that I am trying to trace back. I will update soon. Thanks.

rovinski commented 4 years ago

@RTimothyEdwards I could be wrong here, but I've only seen footprint to mean "LEF abstracts are identical", i.e. pins, obstructions, and dimensions are all identical. This is to allow swapping cells without any need to redo routing or placement. The only uses I've seen are for VT swapping and channel-length swapping (when the cells with different channel lengths have the same footprint).

rovinski commented 4 years ago

Actually I took a quick survey of commercial PDKs I have access to. Some adhere to my definition, some adhere to @RTimothyEdwards's definition. I found this post which seems to agree with my definition but indicates it doesn't matter anymore because tools usually look at the physical library instead of the lib.

msaligane commented 4 years ago

@mithro There is a real issue with cell_fooprint attribute. The attribute is not recognizable by innovus for some reason. So innovus uses its "footprintless" flow (check the doc). I have been trying a few things and found out that innovus recognizes the buffer cells (as an example) when I set the VPWR and VGND to vpwr and vgnd. That's really odd and I can't explain why this is happening yet. The ordering of cell_footprint should be as second after area. But changing that did not fix this issue. If you guys have ideas please let me know!

@rovinski check this: https://people.eecs.berkeley.edu/~alanmi/publications/other/liberty07_03.pdf There is only one definition. The debate is how it is organized in this repo. a cell can be swapped just by using their timing characteristic or geometric characteristic (at least that's how innovus looks at it). Unfortunately or fortunately, innovus seems to have a footprintless flow that doesn't really care about the cell_footprint in this case.

rovinski commented 4 years ago

I feel like the doc is conflicting. Under 1.6.11:

The in_place_swap_mode attribute specifies the criteria for cell swapping during in-place optimization. The basic criteria for cell swapping are:

  • The cells must have the same function.
  • The cells must have the same number of pins, and the pins must have the same pin names.

So nothing is mentioned about layout geometry. But in 2.1.2:

Use the cell_footprint attribute to assign a footprint class to a cell. Use this attribute to assign the same footprint class to all cells that have the same geometric layout characteristics.

So cells are expected to have the same geometry.

Like I said, I saw different commercial PDKs both adhering and not adhering to the "same geometric layout characteristics" criterion. I would suggest using whichever approach results in the fewest warnings / errors in commercial tools. I wouldn't doubt that this property is simply ignored most of the time.

mithro commented 4 years ago

The example in the document seems to be;

cell_footprint : 5MIL ;

Use this attribute to assign the same footprint class to all cells that have the same geometric layout characteristics.

Cells with the same footprint class are considered interchangeable and can be swapped during in-place optimization. Cells without cell_footprint attributes are not swapped during in-place optimization.

When you use cell_footprint, you also set the in_place_swap_mode attribute to match_footprint.

The way I read this is that all cells in cell_footprint group will be considered for replacement if their footprint matches.

If two cells have different cell_footprint values even if their footprint otherwise matches they can not be swapped.

Said another way, if you have two and4 gates with the same footprint then they are only swappable if their cell_footprint value also matches.

Does that make sense?

msaligane commented 4 years ago

Yes, this makes sense. The issue right now is that the buffers aren't recognized by innovus as such and when running a design the tools isn't able to insert buffers (@mithro see my email). When I use lower case vpwr/vgnd the tool recognizes it. The cell_footprint is actually important during all the optimizations that the tools do. So we should figure out how this issue should be solved. Easy fix: use lower case vpwr/vgnd (please don't ask me why).

rovinski commented 4 years ago

If two cells have different cell_footprint values even if their footprint otherwise matches they can not be swapped.

Said another way, if you have two and4 gates with the same footprint then they are only swappable if their cell_footprint value also matches.

@mithro I can agree on that.

The cell_footprint is actually important during all the optimizations that the tools do.

@msaligane I disagree on that. Buffering, resizing, etc. are all done without needing a cell_footprint attribute. nangate45 doesn't have any cell_footprint attributes but OpenROAD and Innovus can perform non-VT optimizations just fine.

I think the issues you are seeing are unrelated to the cell_footprint attribute. If you have to change the power pin names, it may have to do with the global net connections?

msaligane commented 4 years ago

@rovinski Thanks for checking. However, this is not my opinion, that's what innovus does. Try loading the libs with a design and see. There are a couple things that I shared through email to back this up. Of course innovus closes the design using its footprintless flow which does the job, IE optimization using both timing and geometric. But if we need clean libraries we need to figure out this issue. Cell_footprint is actually useful (that's all I am saying). Right now, innovus doesn't recognize any buffer.

I am going to bed now though 😴

msaligane commented 4 years ago

94 issue is related and fixes the footprint issue with buffer/delay/inverter cells.

ravi-varadarajan commented 3 years ago

@rovinski @msaligane Is there any resolution on this. I was setting up some experiments with skywater on innovus and am running into the same issue. Wiuth the 'hd' library, Innovus does not detect any any usable buffers or inverters, and the timing numbers are off. I also see that if I use the hs library it sees usable buffers, however I am having other issues with that library in that lc_shell does not read it completely to convert to .db file