store original pin names during mbff clustering, add regressions, #10093
Conversation
…te sta with reporting changes Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the MBFF::ModifyPinConnections function by adding logic to store original flip-flop instance and port names as properties on the new tray instances. This allows timing reports to display these original names, improving traceability. The pin classification logic was also refactored for clarity. A new integration test, mbff_orig_name, has been added to verify this functionality, including a DEF file, a TCL script to run the test and generate a timing report, and an .ok file with the expected output. Additionally, the src/sta submodule has been updated, likely to support the display of these new orig_name properties in timing reports. There is no feedback to provide on the review comments.
| const std::string orig_port_name = iterm->getMTerm()->getName(); | ||
|
|
||
| dbNet* net = iterm->getNet(); | ||
| while (net) { |
There was a problem hiding this comment.
@maliberty Why a while loop to disconnect the iterm? An iterm cannot have multiple connections in OpenDB, can it? If an dbITerm connects to exactly zero or one dbNet, then this while loop runs exactly once always. This was an existing structure so I did not touch it, but have this question.
There was a problem hiding this comment.
@therealshreyas I believe question goes back to your original implementation. Do you recall the reasoning?
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Need to register the new test cases for Bazel |
| - ff4 DFFHQNx1_ASAP7_75t_L + PLACED ( 6000 4000 ) N ; | ||
| END COMPONENTS | ||
|
|
||
| PINS 1 ; |
There was a problem hiding this comment.
Trivial. Pin count is wrong
| END PINS | ||
|
|
||
|
|
||
| NETS 1 ; |
There was a problem hiding this comment.
Net count is also wrong.
| Average slot-to-flop displacement: 0.865 | ||
| Final Objective Value: 97.228 | ||
| Sizes used | ||
| 2-bit: 2 |
There was a problem hiding this comment.
Minor. How about adding 4-bit test too?
|
Thank you @dsengupta0628. We really appreciate it. |
5 participants
Summary
When OpenROAD's MBFF clustering pass (cluster_flops) merges single-bit flip-flops into multi-bit tray instances, the original instance and pin names are lost. Timing reports show names like _tray_size4_940/Q2 which are meaningless to the designer. The original name — e.g. reg_b/Q — is needed to correlate timing violations back to the RTL.
This PR is for changes in OpenROAD side:
Type of Change
Impact
This change preserves original flip-flop instance/pin names through MBFF clustering by storing per-pin orig_name_* properties on the created tray instances, and exposes the data in OpenSTA timing reports via a new opt-in report field (orig_name).
Verification
./etc/Build.sh).