dralabeing
commented
2 years ago - Continuing on 8 above... The para on the use of the back slash is not clear. What is the impact of not using the backslash escape before the instance names?
Omitting FP_PDN_MACRO_HOOKS definitions for connections results in LVS errors. This is conflated/confused with the \ issue. Please clean up this section- also check grammar. Show the PDN connections images for both cases with and without the hooks defined for clarity.
- Floorplanning "In order to achieve this, keep the area almost the same, but resize the DIE_AREA to a rectangle that allows 100um all around each macro for standard cells. In the next step the location of macro blocks will be selected."
Die area can also be controlled through utilization. It will be useful to also suggest setting sizing to relative and allow utilization to drive DIE AREA. This statement is not very clear->This value is carefully constructed. If it is set to big value then you are going to have routing/placement/timing issues. On the other hand setting the value too low will cause placement and routing congestion issues. Simply add a note: Choose an optimal DIE area to avoid oversizing to cause timing issues and undersizing to avoid routing congestion.
Routing around macros should be controlled through halos not by manipulating placement density. Show the use of : FP_PDN_HORIZONTAL_HALO and FP_PDN_VERTICAL_HALO. Perhaps this should go under macro placement section.
- Macro Cell Placement "The cells need to be placed inside the DIE_AREA, however the automatic placement does not account the I/O placement when selecting sram placement.
It is causing the SRAM component to be placed on the edge of the macro. As a result the I/O power usage is going to be increased, because there is a long net that goes over the subcomponents."
Rewrite this for clarity In this section, we will use manual macro placement to place the macros. Automatic macro placement does not account for I/O pin placement causing the SRAMs to be placed at the edge of the DIE which result in sub-optimal I/O power usage due to long nets.
-
Resolving issues This looks like a list of problems to avoid before macro hardening. Change title to Common Problems to Avoid
-
Before hardening the macro block define the I/O pin placement.
vijayank88
kareefardi
This feedback is for : https://armleo-openlane.readthedocs.io/en/merge-window-4/docs/source/openram.html
This is the first reference to OpenRAM in OpenLane documents. Add a note describing how these OpenRAM cells were created and how these cells are available for use within OpenLane.
This instruction should be based on using commands to copy the indicated relevant sections with parameter changes as required. "Copy the relevant sections from pdks/sky130B/libs.ref/sky130_sram_macros/verilog/sky130_sram_1kbyte_1rw1r_32x256_8.v <which relevant sections, be specific- use commands wherever possible>. Then add (blackbox) attribute. This is specified, to let the synthesis tool know that this module is a blackbox. If the module is empty it is assumed to be blackbox anyway, but specifying the attribute makes it more clear."
"VERILOG_FILES_BLACKBOX": "dir::sky130_sram_1kbyte_1rw1r_32x256_8.bb.v",
Is .json the preferred config file vs .tcl? Users are likely to be more familiar with tcl.
What is this variable and where is it to be defined? It should refer to the configuration file with a doc link here: https://openlane.readthedocs.io/en/latest/reference/configuration.html?highlight=verilog_files#macros-chip-integration
remove any such forward looking statements referring to GH issues. The documentation should not make any such promises regarding development plans etc. In the future liberty with proper power/ground pins support will be added, so the creation of blackbox is no longer required. See issue 1273.
Spell check!! <missmatch,behavor> Please spellcheck before asking for a review ! Remove reference to issue no. Add a note instead of this blob of text.
It is also users responsibility to avoid name collisions between the blackbox macro blocks. If two blackbox modules with the same module name, but different set of parameters exist, then it is possible to get RTL behavor missmatch without any warning. See issue 1291. Change to: Note: Check for name collisions between the blackboxed macro blocks that have same name but different parameters, to avoid a behavioral mismatch.
Connect the layout files and abstracts The paras don't follow guidelines. e.g : In this case absolute path is used, if the PDK location is different then path needs to be changed. -> Use absolute paths if PDK location is different from default location Connect GDS files with the subcomponent.-> what is the meaning of this what is subcomponent. Such statements should be followed by a command not just instructions.
This should be a warning: If you ran the design without this configuration you will get following error: Please use suitable demarcations in the document for warnings, notes and commands. This guideline is not being followed despite multiple reminders.
Also, the user hasn't run the flow yet so why refer to " ran the design.." instead simply issue the warning to use the right pathnames.
There is an abrupt reference to Caravel. Not all OpenLane users use Caravel. Remove this, it is confusing.
This para is confusing: If you unconditionally define the USE_POWER_PINS then powered netlist is generated properly, but the synthesis netlist is generated with signal port vccd1, which should not exist because it is power/ground connection in non powered netlist.
Again, use Warning if you wish to draw the attention of the user to something they should or shouldn't do.
"If the cell is referenced in the submodule then it has the prefix with the submodule name and escaped slash . As can be seen there is two cells sky130_sram_1kbyte_1rw1r_32x256_8 with instance names \submodule.sram0, \submodule.sram1. Directly copy the instance names without the prefix escape symbol: submodule.sram0, submodule.sram1, avoid guessing it." Grammar incorrect, too much text, use commands line cp rather than text instructions .
make this a warning-> If these configuration is missing then power/ground will be missing between netlist and PDN, therefore creating an LVS issue. -> Warning : Omitting explicit power/ground connections as shown, will lead to LVS errors due to missing net connections between netlist and PDN.
Remaining section reviews will be added below.