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

resized regfiles to align with synth-based sizes #192

Closed

Conversation

jeffng-or
Copy link
Collaborator

Resized reg files at align with synth-based sizes per email discussion.

WNS: -1862 (vs -1877)
No change to DRCs
GPL run time increase: 2:05 vs 1:42

Image of placement density:

resized_regfiles

Artifacts should exist (was accurate to yesterday's main, but rebased this morning prior to this PR)

Copy link
Collaborator

@oharboe oharboe left a comment

Choose a reason for hiding this comment

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

Add a comment to clarify if the new area corresponds to the mock/ or rtl/ version.

On the one hand, mock/ underestimates the area, on the other the rtl/ versions might overestimate sizes on OpenROAD.

"floorplan": [io_constraint],
"place": [io_constraint],
"floorplan": [ram_data["io_constraint"]],
"place": [ram_data["io_constraint"]],
},
verilog_files = ["mock/" + ram + ".sv"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are mocked register files, i.e. they do not have all the rows. Use "rtl/" for original RTL.

So this leaves me with a question: what size do the new sizes correspond to? The mock/ or rtl/ version?

@oharboe
Copy link
Collaborator

oharboe commented Oct 30, 2024

What is the WNS of here in the comment?

The nubmer is close to what I recall from floorplan after retiming, whereas CTS WNS is much worse as we pick up a lot of WNS in placement.

There should be a table output when the build completes with the WNS from CTS.

@jeffng-or
Copy link
Collaborator Author

Per meeting discussion, cancel this PR and re-submit new one with existing reg file sizes (400 x 400), but with core area that extends more closely to the die boundary.

@jeffng-or jeffng-or closed this Oct 30, 2024
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.

2 participants