Repository navigation
read binary liberty files from a cache shared by all designs - #4720
eder-matheus wants to merge 12 commits into
Conversation
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
…ate/OpenROAD-flow-scripts into secure-blib_shared_cache
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a binary liberty (.blib) caching mechanism to optimize OpenROAD stages by reading binary files instead of parsing text libraries. It adds configuration variables BINARY_LIBS_DIR and USE_BINARY_LIBS, updates documentation, and includes scripts to manage and load the cached files. The review feedback points out a potential issue in the Makefile where BINARY_LIBS_DIR should be quoted during rm -rf to prevent accidental deletions if the path contains spaces.
|
|
||
| .PHONY: clean_binary_libs | ||
| clean_binary_libs: | ||
| rm -rf $(if $(strip $(BINARY_LIBS_DIR)),$(BINARY_LIBS_DIR),$(error BINARY_LIBS_DIR is empty)) |
There was a problem hiding this comment.
If BINARY_LIBS_DIR contains spaces, the rm -rf command will undergo word splitting, potentially leading to accidental deletion of unintended files or directories. Quoting the expanded path prevents this issue.
rm -rf "$(if $(strip $(BINARY_LIBS_DIR)),$(BINARY_LIBS_DIR),$(error BINARY_LIBS_DIR is empty))"
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 10 with regression(s), 1 without a comparable baseline. ❌ asap7/chi_async_bridge base — 3 failing metric(s)
❌ asap7/ethmac_lvt base — 3 failing metric(s)
❌ asap7/mock-alu base — 2 failing metric(s)
❌ asap7/riscv32i-mock-sram base — 1 failing metric(s)
❌ gf180/aes-hybrid base — 1 failing metric(s)
❌ gt2n/jpeg base — 1 failing metric(s)
❌ sky130hd/aes base — 1 failing metric(s)
❌ sky130hd/gcd base — 2 failing metric(s)
❌ sky130hd/microwatt base — 1 failing metric(s)
❌ sky130hs/gcd base — 2 failing metric(s)
|
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
|
@oharboe The new |
|
A suggestion that would let flows which already provide binary liberty files work with the defaults, without opting out.
This happens in bazel-orfs. It converts each platform library once, as a cached build action, and gives the OpenROAD stages a copy of the platform where every liberty file is binary under its original name, so Proposed change: if a library is already binary, use it as given. proc lib_file_to_read { lib_file } {
if { ![env_var_equals USE_BINARY_LIBS 1] || ![file exists $lib_file] } {
return $lib_file
}
# Already binary: read it as given, whatever its name
if { ![catch { liberty_binary_info $lib_file }] } {
return $lib_file
}
...and the same check at the top of the loop in With this, Nit: I would discourage the new FLW-0026/0027/0028 messages. Message IDs belong to OpenROAD, where each tool owns a range and uniqueness is checked. Nothing checks ORFS's numbers: |
|
@eder-matheus Thanks for adding
With both in place, |
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
|
@oharboe Thanks, both comments are addressed in 95f03da:
The binary check turned out to be unnecessary:
Following the Makefile contract, |
|
@eder-matheus Thanks, this is much simpler now. bazel-orfs is fine as it is, both in how it works with this and for its users: it runs only The suggestions below are for ORFS users: fewer things to know about and less to maintain.
Minor: |
…cache Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com> # Conflicts: # flow/scripts/util.tcl
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
…act asks Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
|
@oharboe Thanks, all four points are addressed:
On the minor point: I also merged master and, following the Tcl contract, removed the |
|
@maliberty @dsengupta0628 I believe this PR is ready for review |
|
Summary
OpenROAD stages read binary liberty (
.blib) files from a cache shared by all designs, instead of parsing the text libraries in every stage. Supersedes #4682.objects/binary_libs/<platform>/<library>@<size>-<mtime>-<dirhash>.blib, so a changed library gets a new entry. Libraries in the design directory, or generated by the flow, are cached in the run's ownobjectsdirectory.1_synth.odb, the first OpenROAD step, and again when one changes. A no-opmakeconverts nothing.do-<step>,runand the GUI targets never convert. Yosys reads the text libraries..blibis missing or unreadable is read as given, so flows that bring their own binary libraries, like bazel-orfs, need no setting.make binary-libsconverts on demand, andmake clean_binary_libsdeletes the cache.genReport.pyreports the most recently written log as the last one.Type of Change
New feature.
Impact
Liberty reads are about 5x faster: 677 s to 135 s over 16 stages on a private PDK with 44 libraries.
Verification
BLOCKSandCORNERS) give the same results as with the text libraries. Also tested: concurrent conversions, changed libraries, entries from another format version, an empty cache and a no-opmake.