LEF 5.6/5.7 support for tapcells class: CORE WELLTAP

[msaligane]

Expected Behavior

Tapcells have class CORE and should be CORE WELLTAP which is a LEF 5.6 construct and the LEF is 5.5

The script that prepares the LEF files from magic, which would need some additional handling to note that the cell name is "tap" as follows: "property LEFclass {CORE WELLTAP}".

Actual Behavior

When using the current LEF files: check_placement (openroad) reports a bunch of overlaps because the tapcells have class CORE in the LEF file.

@RTimothyEdwards says we could change the LEF version from 5.5 to 5.7 in the file but there are small incompatibilities such as "SOURCE" line, it needs to be removed as it was deprecated in 5.6.

[mithro]

Current file;

VERSION 5.7 ;
BUSBITCHARS "[]" ;
DIVIDERCHAR "/" ;
PROPERTYDEFINITIONS
  MACRO maskLayoutSubType STRING ;
  MACRO prCellType STRING ;
  MACRO originalViewName STRING ;
END PROPERTYDEFINITIONS
MACRO sky130_fd_sc_hdll__tapvgnd_1
  ORIGIN  0.000000  0.000000 ;
  CLASS CORE ;
  SYMMETRY X Y R90 ;
  SIZE  0.460000 BY  2.720000 ;
  SITE unithd ;
  PIN VGND
    DIRECTION INOUT ;
    USE GROUND ;
  END VGND
  PIN VPWR
    DIRECTION INOUT ;
    USE POWER ;
  END VPWR
  PROPERTY maskLayoutSubType "abstract" ;
  PROPERTY prCellType "standard" ;
  PROPERTY originalViewName "layout" ;
END sky130_fd_sc_hdll__tapvgnd_1
END LIBRARY

You want this changed to the following?

VERSION 5.7 ;
BUSBITCHARS "[]" ;
DIVIDERCHAR "/" ;
PROPERTYDEFINITIONS
  MACRO maskLayoutSubType STRING ;
  MACRO prCellType STRING ;
  MACRO originalViewName STRING ;
END PROPERTYDEFINITIONS
MACRO sky130_fd_sc_hdll__tapvgnd_1
  ORIGIN  0.000000  0.000000 ;
  CLASS CORE WELLTAP;
  SYMMETRY X Y R90 ;
  SIZE  0.460000 BY  2.720000 ;
  SITE unithd ;
  PIN VGND
    DIRECTION INOUT ;
    USE GROUND ;
  END VGND
  PIN VPWR
    DIRECTION INOUT ;
    USE POWER ;
  END VPWR
  PROPERTY maskLayoutSubType "abstract" ;
  PROPERTY prCellType "standard" ;
  PROPERTY originalViewName "layout" ;
END sky130_fd_sc_hdll__tapvgnd_1
END LIBRARY

diff --git a/libraries/sky130_fd_sc_hdll/v0.1.1/cells/tapvgnd/sky130_fd_sc_hdll__tapvgnd_1.lef b/libraries/sky130_fd_sc_hdll/v0.1.1/cells/tapvgnd/sky130_fd_sc_hdll__tapvgnd_1.lef
index c5ece42561a..7a6fc170c05 100644
--- a/libraries/sky130_fd_sc_hdll/v0.1.1/cells/tapvgnd/sky130_fd_sc_hdll__tapvgnd_1.lef
+++ b/libraries/sky130_fd_sc_hdll/v0.1.1/cells/tapvgnd/sky130_fd_sc_hdll__tapvgnd_1.lef
@@ -24,7 +24,7 @@ PROPERTYDEFINITIONS
 END PROPERTYDEFINITIONS
 MACRO sky130_fd_sc_hdll__tapvgnd_1
   ORIGIN  0.000000  0.000000 ;
-  CLASS CORE ;
+  CLASS CORE WELLTAP ;
   SYMMETRY X Y R90 ;
   SIZE  0.460000 BY  2.720000 ;
   SITE unithd ;

[msaligane]

Yes this looks correct. I also noticed you removed the SOURCE USER ; line @mithro Just pushed a fixed version here :

https://github.com/The-OpenROAD-Project/OpenROAD-flow/blob/sky130/flow/platforms/sky130/lef/merged.lef

[mithro]

MACRO macroName
[CLASS
 { COVER [BUMP]
 | RING
 | BLOCK [BLACKBOX | SOFT]
 | PAD [INPUT | OUTPUT |INOUT | POWER | SPACER | AREAIO]
 | CORE [FEEDTHRU | TIEHIGH | TIELOW | SPACER | ANTENNACELL | WELLTAP]
 | ENDCAP {PRE | POST | TOPLEFT | TOPRIGHT | BOTTOMLEFT | BOTTOMRIGHT}
 }
;]

[msaligane]

@mithro the cell I am using is :

MACRO sky130_fd_sc_hs__conb_1
  16000   CLASS CORE WELLTAP ;

[mithro]

That doesn't look like a tap to me?

libraries/sky130_fd_sc_ms/v0.0.1/cells/tapmet1/sky130_fd_sc_ms__tapmet1_2.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapmet1/sky130_fd_sc_ms__tapmet1_2.magic.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvgnd2/sky130_fd_sc_ms__tapvgnd2_1.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvgnd2/sky130_fd_sc_ms__tapvgnd2_1.magic.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvgnd/sky130_fd_sc_ms__tapvgnd_1.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvgnd/sky130_fd_sc_ms__tapvgnd_1.magic.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvpwrvgnd/sky130_fd_sc_ms__tapvpwrvgnd_1.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tapvpwrvgnd/sky130_fd_sc_ms__tapvpwrvgnd_1.magic.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tap/sky130_fd_sc_ms__tap_1.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tap/sky130_fd_sc_ms__tap_1.magic.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tap/sky130_fd_sc_ms__tap_2.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/tap/sky130_fd_sc_ms__tap_2.magic.lef

[mithro]

libraries/sky130_fd_sc_ms/v0.0.1/cells/conb/sky130_fd_sc_ms__conb_1.lef
libraries/sky130_fd_sc_ms/v0.0.1/cells/conb/sky130_fd_sc_ms__conb_1.magic.lef

[msaligane]

oops .. that's the tie hi/lo cell sky130_fd_sc_ms__conb_1 The syntax for that is this:

  CLASS CORE TIELOW ;
  CLASS CORE TIEHIGH ;

But the issue for this cell is that it is combining both

the iss

[mithro]

So, for the tap cells you want CLASS CORE WELLTAP ; and for the conb cell you want CLASS CORE TIELOW ; \n CLASS CORE TIEHIGH ;?

[mithro]

The document at http://www.ispd.cc/contests/18/lefdefref.pdf says;

TIEHIGH,TIELOW—Used for connecting unused I/O terminals to the power or ground bus.

[msaligane]

So, for the tap cells you want CLASS CORE WELLTAP ; and for the conb cell you want CLASS CORE TIELOW ; \n CLASS CORE TIEHIGH ;?

Correct.

[mithro]

This should be done in the next push after the SOURCES change.

[RTimothyEdwards]

@mithro : @msaligane keeps not answering this; but "CLASS CORE TIELOW ; \nCLASS CORE TIEHIGH ;" is invalid LEF syntax. The LEF cell can only have one defined "CLASS". The problem is that LEF (as usual) has not anticipated the possibility that a cell can be both. BUT there are also potential issues with OpenROAD that need to be checked with the OpenROAD and openlane developers about whether any of those tools can make use of a combined TIEHIGH/TIELOW cell. Possibly not. It seems to be that the solution is to copy the "conb" cell into two new cells "tielow" and "tiehigh". In "tielow", remove the HI pin, and in "tiehigh", remove the LOW pin. The rest of the geometry can remain the same. Then mark "conb" as "dont_use" in the liberty files, and mark cell "tiehigh" as "CLASS CORE TIEHIGH" and mark cell "tielow" as "CLASS CORE TIELOW". Then all of the tools should be happy.

[rovinski]

There's only 1 tie cell for both low and high? That's so weird...

[msaligane]

@RTimothyEdwards It would really make sense to invest in making two new cells for the tie hi/lo. One other thing that really bothers me is the resistor used there. I would expect a pull-up / pull-down circuit. Something that looks like this: https://electronics.stackexchange.com/questions/195887/basic-tie-high-and-tie-low-circuits-in-digital-vlsi-design

As @rovinski mentioned, not a very conventional approach.

[mithro]

@msaligane - I'm going to split the ties into the separate issue.

[20Mhz]

As mentioned in slack group, I'm also worried that this type (poly resistor based) may be bad for antenna or may hide the issue from the tools, just a thought.

[mithro]

This should be fixed in #124.

[rbarzic]

There's only 1 tie cell for both low and high? That's so weird...

@rovinski, @RTimothyEdwards : Having one tiehigh/tielow cell is essential when you want to have some sort of configurability. It is quite common to make a design that can be configured using only one mask change (typically metal1). It is also very useful when doing a 2nd tapeout with only metal changed (a typical metal fix flow). If your chip contains some sort of serial number/JTAD ID, you can adjust the serial number reflecting the new version with just one layer change - no new layout needed. So we need tiehigh, tielow but also a tiehighlow cell (without the LEF attributes TIEHIGH/TIELOW of course because this will be a manually instanciated cell) - All of this if of course assuming that the original conb cell layout allows such a use)

[rovinski]

@rbarzic I know I'm not familiar with every design paradigm, but I've still worked with multiple kits from multiple vendors and this is the first I've ever seen a TIEHI/TIELO together in a single cell. I don't see a reason to remove this cell, but there should definitely be separate TIEHI/TIELO cells available to use. Tools rely on the TIELOW/TIEHIGH property to be able to properly identify them.