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

scripts: load CLKGATE_MAP_FILE as lib and techmap it #2439

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

Conversation

widlarizer
Copy link
Contributor

The main idea is that if you use yosys clockgate to map ICGs to OPENROAD_CLKGATE, you need a blackbox to be present, which is missing at that time since currently it's loaded with -defer. That leads to check failures due to missing cells after abc which this PR fixes. Credit for the idea goes to @povik

Signed-off-by: Emil J. Tywoniak <emil@tywoniak.eu>
@maliberty
Copy link
Member

We are not currently using clockgate so why does this PR lead to failures?

@povik
Copy link
Contributor

povik commented Oct 10, 2024

Looks like the synthesis netlist is chaotically altered and that propagates into some designs not meeting QoR limits.

@maliberty
Copy link
Member

Why is there a difference in the netlist versus what we get without this PR?

@povik
Copy link
Contributor

povik commented Oct 10, 2024

Different identifiers inserted by techmap -map $::env(CLKGATE_MAP_FILE) vs what we previously got when the intermediate clock gating cell was part of the design hierarchy. This can affect ordering of PIs/POs as the design is loaded into abc, and from there abc can make different mapping choices.

I think those designs affected might be those which instantiate OPENROAD_CLKGATE, I am not sure.

@widlarizer
Copy link
Contributor Author

With yosys and openroad built from source, I ran test/test_helper.sh swerv_wrapper nangate45 locally on this PR. I couldn't reproduce the routing failure of that particular design. Running yosys command stat on 1_synth.v on this PR vs master results in the following logs:

swerv-master.txt
swerv-pr.txt

They show a 3% increase in cell count and 8% increase in wire bits. Most modules experience no change or minor decrease/increase in cells / wire bits, but ifu_bp_ctl goes from 5.1k wire bits to 8.2k, ifu_aln_ctl goes from 5.7k to 7.6k, ifu from 5.4k to 6.6k. Looking at their cell stats it looks like those modules at this PR use fewer _X2 cells and more cells overall. The most extreme isolated cell type is DFFR_X1 in ifu_bp_ctl going from 519 instances to 1507. This is the only DFFR_* cell type used in both synthesis results. Looking into it.

@povik
Copy link
Contributor

povik commented Oct 11, 2024

@maliberty For the nangate45/swerv design something else is happening.

In the design OPENROAD_CLKGATE is used for functional purposes (see e.g. the rvdffe_WIDTH32 module in the sources: https://raw.githubusercontent.com/The-OpenROAD-Project/OpenROAD-flow-scripts/refs/heads/master/flow/designs/src/swerv/swerv_wrapper.sv2v.v), but for NANGATE45 the clock gating cell is implemented as a feedthrough connection, so the design is being miscompiled on that platform.

Before the PR the feedthrough connection was inserted before equivalent flip-flops would have a chance to get merged, which would also happen for flip-flops with the same data input and the same clock but different enable (since the enable distinction was lost). After the PR the implementation of OPENROAD_CLKGATE is applied later, so this no longer happens.

This can explain what @widlarizer observed

The most extreme isolated cell type is DFFR_X1 in ifu_bp_ctl going from 519 instances to 1507.

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.

3 participants