Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow: Use Yosys/abc native functions for dont_use #1807

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rovinski
Copy link
Member

@rovinski rovinski commented Feb 17, 2024

  • Change the Makefile target for DONT_USE_LIBS to DECOMPRESSED_LIBS
    • Removes the call to markDontUse.py, replaces it with either gunzip or ln
  • Switch yosys/abc to use the "dont use" flags available
  • Switch all uses of DONT_USE_SC_LIB to SC_LIB
  • Add missing DFF_LIB_FILE var to asap7

Closes #1502

@maliberty
Copy link
Member

Ready for review?

@rovinski
Copy link
Member Author

Not yet, ASAP7 needs fixing

@rovinski
Copy link
Member Author

rovinski commented Feb 18, 2024

I went down a bit of a rabbit hole. It appears that markDontUse.py was fulfilling multiple duties. Not only was it altering the .lib properties, but it was also uncompressing any Gzipped files first. Neither Yosys nor abc support native decompression, so this is still required. ASAP7 is the only platform with Gzipped Liberty files right now. GF180 uses gzipped files too.

But, there is also a step which does a merging of the liberty files. I wondered if this step is still required, and as far as I can tell it is. The main step that requires all the LIB_FILES is the abc command from inside yosys. This command allegedly does support multiple .lib files because you can repeat the -liberty with new files and each one gets pushed to a vector. This gets translated to abc via multiple read_lib commands in the generated abc script. This does not work as intended, because abc apparently overwrites or at least only looks at the most recent .lib file that was read.

ABC: ** cmd error: aborting 'source /home/rovinski/OpenROAD-flow-scripts/flow/scripts/abc_speed.script'
ABC: ** cmd error: aborting 'source <abc-temp-dir>/abc.script'
ABC: Error: Cannot find buffer gate in the library.

abc does read the ASAP7 BUFINV liberty, but it wasn't last, and it complains about all the other libraries not having a buffer.

Weirdly enough, it looks like abc supports specifically 1-2 lib files.

Usage: read_lib [-SG float] [-M num] [-dnuvwh] [-X cell_name] <file> <file2>
                   reads Liberty library from file
        -S float : the slew parameter used to generate the library [default = 0.00]
        -G float : the gain parameter used to generate the library [default = 0.00]
        -M num   : skip gate classes whose size is less than this [default = 0]
        -X name  : adds name to the list of cells ABC shouldn't use. Flag can be passed multiple times
        -d       : toggle dumping the parsed library into file "*_temp.lib" [default = no]
        -n       : toggle replacing gate/pin names by short strings [default = no]
        -u       : toggle setting unit area for all cells [default = no]
        -v       : toggle writing verbose information [default = yes]
        -w       : toggle writing information about skipped gates [default = no]
        -h       : prints the command summary
        <file>   : the name of a file to read
        <file2>  : the name of a file to read (optional)

But if I try using any more than 2 files it prints the usage message instead.

So the conclusion is that while we can eliminate modifying the .lib dont_use properties, we still need to decompress files, and we still need to merge libs into one standard cell .lib.

It will have to stay this way until abc adds support for reading >2 liberty files. It is also problematic that yosys allows passing of multiple liberty files to abc because it is very misleading.

@maliberty
Copy link
Member

@QuantamHD perhaps you can ask Alan about this abc behavior.

@rovinski rovinski marked this pull request as ready for review February 18, 2024 06:11
* Change the Makefile target for `DONT_USE_LIBS` to `DECOMPRESSED_LIBS`
  * Removes the call to markDontUse.py, replaces it with either gunzip or ln
* Switch yosys/abc to use the "dont use" flags available
* Switch all uses of `DONT_USE_SC_LIB` to `DECOMPRESSED_SC_LIB`
* Add missing `DFF_LIB_FILE` var to asap7

Signed-off-by: Austin Rovinski <rovinski@nyu.edu>
Comment on lines +378 to +384
$(DECOMPRESSED_LIBS): $$(filter %$$(@F) %$$(@F).gz,$(LIB_FILES))
mkdir -p $(OBJECTS_DIR)/lib
if [[ $^ == *.gz ]]; then \
gunzip -c $^ > $@; \
else \
ln -s $$(realpath --relative-to=$(dir $@) $^) $@; \
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is one additional issue here.
You still need to use the mark don't use script to ensure the yosys call to map dffs doesn't use any FFs that are marked as don't use.
Mark don't use also does some additional cleanup which has caused ABC to crash before on other PDKs.

In order to actually get rid of this, you will need:

  1. yosys support for adding don't use to the dff call in that script (otherwise it's a loss of functionality).
  2. abc and yosys some improvements to their liberty parsers to not choke on pdks. https://github.com/siliconcompiler/siliconcompiler/blob/55d8ff05d42f55ac45970641a51f0817a9918b3f/siliconcompiler/tools/yosys/prepareLib.py#L43 https://github.com/siliconcompiler/siliconcompiler/blob/55d8ff05d42f55ac45970641a51f0817a9918b3f/siliconcompiler/tools/yosys/prepareLib.py#L51 https://github.com/siliconcompiler/siliconcompiler/blob/55d8ff05d42f55ac45970641a51f0817a9918b3f/siliconcompiler/tools/yosys/prepareLib.py#L58

@@ -53,7 +53,7 @@ if {[info exist ::env(LATCH_MAP_FILE)]} {
if {[info exist ::env(DFF_LIB_FILE)]} {
dfflibmap -liberty $::env(DFF_LIB_FILE)
} else {
dfflibmap -liberty $::env(DONT_USE_SC_LIB)
dfflibmap -liberty $::env(DECOMPRESSED_SC_LIB)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs dont_use

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened YosysHQ/yosys#4219 to fix, but it may take a bit to get approved then upstreamed.

@rovinski
Copy link
Member Author

I opened YosysHQ/yosys#4219 to fix the dont_use issue with dfflibmap. It works on my system. @QuantamHD maybe you can help review?

I also see an easy path to supporting multiple libs in dfflibmap. This would eliminate one half of the need to do lib merging, but as mentioned above ABC needs to fix its multiple lib parsing. I'm not sure I want to try that... it looks like a lot of ABC PRs just get stuck.

@rovinski
Copy link
Member Author

I think for right now because of everything @gadfort mentioned I am going to revert this PR to only adding the dont_use flags in the script on top of the current method of marking them in lib. This will be duplicate, but it will be in preparation for eventually removing liberty merging / preprocessing if the upstream is ever fixed.

@rovinski
Copy link
Member Author

I've decided to break this PR down into smaller pieces and only merge the ones that make sense.
#1809 covers using the dont_use flags for abc which is originally what #1502 was about.
When YosysHQ/yosys#4219 is merged and then downstreamed to ORFS, then dfflibmap can be updated and dont_use modification in .lib can be totally removed. The other library prep steps will have to remain for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update scripts to use Ethan's new dont_use functionality
4 participants