Sky130 - Convert to Pydantic, support sky130_scl#848
Sky130 - Convert to Pydantic, support sky130_scl#848harrisonliew wants to merge 75 commits intomasterfrom
Conversation
nayiri-k
left a comment
nayiri-k
left a comment
There was a problem hiding this comment.
Passed through commercial/openroad e2e flow
| 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.
@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.
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.
| - name: Checkout | ||
| uses: actions/checkout@v4.1.6 | ||
|
|
||
| - 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.
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.
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.
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.
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
elamdf
changed the title
|
will make a separate PR for CI smoketest |
| used_overrides.append((field_name, fname)) | ||
| new_path = cached_paths[0] | ||
| self.logger.info( | ||
| f"overriding {default_path} with cache entry {new_path}" |
There was a problem hiding this comment.
this kind of blows up the log since there are a bunch of instances of the same library...
| ) | ||
| return True | ||
|
|
||
| def efabless_ring_io(ht: HammerTool) -> bool: |
There was a problem hiding this comment.
I think we should create a separate PR for a robust toplevel-ready sky130 flow. Recent tapeouts have required a bunch of additional hooks in hammer-driver, I think just having this in sky130/init is a little misleading (since other stuff not currently in hammer is also required for a clean run)
4 participants
gen_configfor tech plugins to override and generate their own config (TechJSONinstance, i.e. directly creating the Pydantic model). This enables multi-library support within the same tech plugin.sky130.tech.jsonLEFUtils(unfortunately can't create Metal object directly or else it has a circular import)gds_map_filereading in OpenROAD plugintechnology.override_libraries: []andtechnology.manually_override_pdk_collateral: boolTODOs:
Related PRs / Issues
Type of change:
Impact:
Contributor Checklist:
masteras the base branch?poetry.lockfile if you updated the requirements inpyproject.toml?e2e/if this feature depends on updated plugins?