-
Notifications
You must be signed in to change notification settings - Fork 60
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
Sky130 - Convert to Pydantic, support sky130_scl, e2e CI smoketest #848
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passed through commercial/openroad e2e flow
@@ -53,4 +53,6 @@ def get_headers(source: str) -> List[str]: | |||
# TODO: handle other compressed file types? Tools only seem to support gzip. | |||
fd = os.open(source, os.O_RDONLY) | |||
lines = os.pread(fd, nbytes, 0).splitlines() | |||
return list(map(lambda l: l.decode('ascii', errors='ignore'), lines)) | |||
lines = list(map(lambda l: l.decode('ascii', errors='ignore'), lines)) | |||
all_lines = '\n'.join(lines).replace('\\\n','') # combine lines that are split by \ character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harrisonliew This might be too hacky, but basically in one of the sky130 lib files (sky130_ef_io__gpiov2_pad_wrapped_ss_ss_100C_1v60_3v00.lib
) the capacitive_load_unit
declaration spanned 2 lines and it messed up the parsing in LIBUtils.get_cap_unit()
capacitive_load_unit(1.000000, \
"pf");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's valid. But maybe the better approach is to remove line continuations (\
) before processing lines. Can you also fix the type checker error?
synthesis runs until genus breaks on trying to do clock gating with no latches- asking cadence folks on how to fix.
b38dc05
to
8500f3b
Compare
130a IO cells anyways
…rect antenna diode cell name
…n LVS (and this casuses import error)
…to filler" This reverts commit bb03824.
This will be merged once the cadence stdcells can produce a drc/lvs (with calibre and ideally pegasus too) design of reasonable complexity |
* efabless ring IO template + hooks * consolidate docs * add sky130_fd_io library from open_pdks, fix template IO file * fix corner cell, make met5 stripes like caravel * met5 rings now connected to clamped3 pads * add set_dont_touch on tie_hi/lo_esd output pins * Update IO file instructions * typos, move section * More clarity * updated sram22 naming convention * Update compiler to check for both PEX and schematic LIBs * lib path change * fix SRAM caches and compiler logs * patch PAD class on connect_* IO spacers * update sram cache to include latest SRAM varieties * add back all srams * fix sram compiler * add SRAM aggregated SPICE netlist for LVS * Update tech for LVS and DRC fixes * fix sram cache * Expose additional CORE pins * style fixes * remove ctsbuffer cell that breaks synthesis * TODO clean this up, flow at least works, but LVS has issues with trailing colons (yet no virtual connect) and drc has some random met1 spacing violations around srams * remove redundant code, allow for automatic overrides of library default paths with cache or env yml * add fillers with drc in innovus, this is required for a clean design * remove sram with inconsistent ss and tt lib * fixes for scl, par at least runs but with congestion issues * revert to upstream readme * include met1 in default routing layers, otherwise cadence stdcells have horrible QoR * add calibre decks to sky130_scl flow, try to blackbox io cells for pegasus run * remove dead code * readd cadence pdk part of readme * oops add newlines * remove hardcoded cache overrides * remove unreleased srams * more robust override/cache path decision making in tech init * fix extra sram cache --------- Co-authored-by: Harrison Liew <[email protected]> Co-authored-by: Nayiri K <[email protected]> Co-authored-by: rohanku <[email protected]> Co-authored-by: Ethan Wu <[email protected]>
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Run syn (Genus), par (Innovus), drc (Pegasus), and lvs (Pegasus) on the GCD design in sky130, using sky130_scl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the CI is fully reproducible. I don't know whats in the e2e
folder or what poetry
installs but in general I would make sure it's clear what tools are installed by default and what you are installing in the CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fair, we can remove the poetry
call since this will only be run by one daemon on one machine, that is have it assume there's a poetry environment already set up- is that what you're asking? (It's unreasonable to have this run in isolation/install everything from scratch anyways since it relies on NFS paths for pdk collateral and tool binaries)
echo "log file is $tempfile on $(hostname)" | tee -a $tempfile | ||
echo "End-to-end CAD flow CI smoketest running on" ${{ github.head_ref }}.${{ github.sha }} > $tempfile 2>&1 | ||
make build >> $tempfile 2>&1 | ||
echo "running par.." | tee -a $tempfile | ||
make par >> $tempfile 2>&1 | ||
echo "running drc.." | tee -a $tempfile | ||
make drc >> $tempfile 2>&1 | ||
echo "running lvs.." | tee -a $tempfile | ||
make lvs >> $tempfile 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of dumping something out to a file if it's sensitive. What happens if a current Hammer user accidentally removes one of these echos on accident and info is leaked? Maybe it's better to try to disable actions from logging entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I see your point but I don't see how that reasoning doesn't apply to any output masking/secret hiding in an action. I've added a warning comment to hopefully dissuade accidental leaking, I'm not really sure how to actually resolve this concern- maybe only allow maintainers to manually kick off the workflow once they've checked if no malicious changes to the actions? That seems like a single point of failure though
gen_config
for tech plugins to override and generate their own config (TechJSON
instance, i.e. directly creating the Pydantic model). This enables multi-library support within the same tech plugin.sky130.tech.json
LEFUtils
(unfortunately can't create Metal object directly or else it has a circular import)gds_map_file
reading in OpenROAD pluginTODOs:
Related PRs / Issues
Type of change:
Impact:
Contributor Checklist:
master
as the base branch?poetry.lock
file if you updated the requirements inpyproject.toml
?e2e/
if this feature depends on updated plugins?