Skip to content

rsz: Convert buffering to fixed-point delays#7648

Merged
maliberty merged 8 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:rsz-fixed-point
Jul 1, 2025
Merged

rsz: Convert buffering to fixed-point delays#7648
maliberty merged 8 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:rsz-fixed-point

Conversation

@openroad-ci
Copy link
Copy Markdown
Collaborator

This is to address

[ERROR RSZ-0501] buffering pin gen_tiles\[0\].i_tile.i_tile/gen_cores\[1\].gen_mempool_cc.riscv_core.i_snitch/_10200_/ZN failed: area recovery cannot reproduce solution

and errors of this form

Also integrate Rebuffer2 into repair_timing. We can retire the former content of Rebuffer.cc and move Rebuffer2.cc into its place.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/rsz/src/BufferMove.hh Outdated
Comment thread src/rsz/src/BufferedNet.hh Outdated
Comment thread src/rsz/src/Rebuffer.hh Outdated
@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 24, 2025

Secure CI came back green but on public CI this raised a few repair failures similar to The-OpenROAD-Project/OpenROAD-flow-scripts#3245. With @eder-matheus we have taken a look at one instance of this failure and we think we understand the cause due to a bug in GRT.

I think we should wait on the GRT fix before merging this change.

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 24, 2025

I think we should wait on the GRT fix before merging this change.

The prospective fix: #7650

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 25, 2025

Public CI on designs is down to a fixable EQY failure, this is ready to merge pending review

@povik povik requested a review from maliberty June 25, 2025 10:56
@maliberty
Copy link
Copy Markdown
Member

Are you fixing the "fixable EQY failure"?

@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 25, 2025

Yes, I'll open a PR on the ORFS side

@maliberty
Copy link
Copy Markdown
Member

Please explain how this conversion fixes the issue you mentioned. Its is helpful to give more explanation for a larger PR.

@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 25, 2025

I think this PR is easier to review going per commits. The main change is in the first commit, and the changes to Rebuffer.cc from the first commit can be ignored since the file gets replaced with Rebuffer2.cc later in the commit sequence. I can rewrite the git history to get rid of these extra changes and to make it more readable if requested.

Please explain how this conversion fixes the issue you mentioned.

The issue is caused by rounding errors putting the algorithm in an inconsistent state. By using fixed-point delay and slack values this is mostly avoided. Inconsistencies might still be introduced by way of the gate delay calculator but it's unclear it's an issue in practice and there are ways we could fix it.

More elaborate explanation: Area recovery is tasked with building up a buffer tree which is no worse in timing than the current candidate tree. To that effect it assigns timing thresholds to each node of a Steiner tree, and then it enumerates buffer tree options requiring the local thresholds to have been met. Since the thresholds were assigned based on an existing candidate tree (with some relaxation applied) we are in principle guaranteed at least one viable solution. Due to rounding errors the enumeration process can fail coming up with no viable solution. The slacks and delays can be one or two orders of magnitude apart, this makes it challenging to come up with a good tolerance scheme to prevent failures due to rounding errors.

povik added 8 commits July 1, 2025 00:55
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
This tests functionality not exercised in the flow, and which doesn't
work at the moment. We can revert once this works again.

Signed-off-by: Martin Povišer <povik@cutebit.org>
Use exact comparison to fix repair_setup6 which sets reduced RC values.

Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Martin Povišer <povik@cutebit.org>
@povik povik requested review from maliberty and removed request for maliberty June 30, 2025 23:01
@povik
Copy link
Copy Markdown
Contributor

povik commented Jun 30, 2025

@maliberty I have introduced strong typing for the fixed-point delays and rewrote the git history to aid review

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@maliberty maliberty enabled auto-merge June 30, 2025 23:37
@maliberty maliberty disabled auto-merge July 1, 2025 23:44
@maliberty maliberty merged commit 36d3b0c into The-OpenROAD-Project:master Jul 1, 2025
10 of 11 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.

3 participants