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

Fix #555 full-chip STA: Flatten GL netlist array instances for decaps/fills #556

Merged

Conversation

amm-efabless
Copy link

Flattens array instances of decap/fill cells in gate-level netlists, to make them compatible again with full-chip STA flows. Addresses #555 and thus efabless/caravel_user_project#377

@DavidRLindley DavidRLindley requested review from DavidRLindley and removed request for DavidRLindley October 28, 2024 12:32
@DavidRLindley
Copy link
Contributor

DavidRLindley commented Oct 28, 2024

Looks good to me but I'm not authorized to merge it.

@DavidRLindley
Copy link
Contributor

@amm-efabless Is this still needed?

@amm-efabless
Copy link
Author

I believe this is still required (but I haven't tried testing full-chip STA again recently). At the time, it was failing because this instance array syntax is not supported by the scripts (even though it's valid Verilog-1995): sky130_ef_sc_hd__decap_12 EF_decap_12[65364:0] (.VGND(vssd),

So far as I can tell, this unsupported syntax still appears at the bottom of the GL files, e.g. in caravel/verilog/gl/caravel_core.v:

⋮
 sky130_ef_sc_hd__decap_12 EF_decap_12[65364:0] (.VGND(vssd),
    .VNB(vssd),
    .VPB(vccd),
    .VPWR(vccd));
 sky130_ef_sc_hd__fill_4 EF_fill_4[57192:0] (.VGND(vssd),
    .VNB(vssd),
    .VPB(vccd),
    .VPWR(vccd));
 sky130_ef_sc_hd__fill_8 EF_fill_8[57192:0] (.VGND(vssd),
    .VNB(vssd),
    .VPB(vccd),
    .VPWR(vccd));
endmodule

The 4 GL files that my PR changes have had no other updates since my PR; they were last updated 6mo ago when @DavidRLindley and @d-m-bailey needed to remove ~50% of the decap cells, and full-chip STA broke as a result of the change.

At some point, we should probably understand why the full-chip STA flow doesn't support this syntax.

@DavidRLindley DavidRLindley changed the base branch from main to ci2504-DEV February 7, 2025 17:05
Copy link
Contributor

@DavidRLindley DavidRLindley left a comment

Choose a reason for hiding this comment

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

Per Anton this is still needed. I'm going to merge it into the DEV branch for testing.

@DavidRLindley DavidRLindley merged commit d3709fb into efabless:ci2504-DEV Feb 7, 2025
2 checks passed
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.

4 participants