gui: display macro clock insertion latency in timing reports#9683
Merged
maliberty merged 7 commits intoThe-OpenROAD-Project:masterfrom Mar 12, 2026
Merged
gui: display macro clock insertion latency in timing reports#9683maliberty merged 7 commits intoThe-OpenROAD-Project:masterfrom
maliberty merged 7 commits intoThe-OpenROAD-Project:masterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds the display of source and target clock insertion delays to the timing reports GUI. The changes are well-implemented, adding new data members to TimingPath, plumbing the data from the STA engine, and displaying it in new columns in the timing paths table, which are hidden by default as intended. The clock summary row is also correctly annotated with this new information. I have one suggestion to improve the robustness of the code.
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Annotate pins in the Data/Capture Path Details views with the internal clock latency from their Liberty cell's max_clock_tree_path / min_clock_tree_path timing groups, shown inline as [clk ins: <value>]. Closes The-OpenROAD-Project#9681 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
Author
|
@maliberty something like this? |
Member
|
are you asking me to review this? |
Collaborator
Author
Yes please.. Of course I have no idea what I'm doing here. Claude might. What this PR offers is a demonstration of what I think would be a useful way to present this information for my use-case. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
maliberty
reviewed
Mar 10, 2026
Revert unrelated bounds-check change in timingWidget.cpp. Highlight pins with clock insertion delay in blue so the annotation is visually noticeable in the timing path details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Update the documentation screenshot to show the [clk ins: 70.099] annotation in blue text, making it visually distinct as requested in review. Cross-checked against Element_typ.lib: the max_clock_tree_path cell_rise value is 70.09948, matching the GUI display of 70.099. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
Author
|
@maliberty unrelated CI errors, I think. These are GUI changes. |
maliberty
approved these changes
Mar 11, 2026
Member
|
@luarss I see an rather uninformative message in the CI: Please clarify what is going on and improve the message. |
Collaborator
Author
Revert timing_report_clk_insertion.png to unblock CI doc checking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Collaborator
Author
|
@maliberty automerge? @luarss The doc CI problems is a separate concern, I have reverted doc updates from this PR. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
@oharboe @maliberty I will be opening a separate PR to address the issues. Thanks for raising this. |
Contributor
|
Adding some postmortem here, looking at the failed runs - debug logs do appear in the https://jenkins.openroad.tools/job/OpenROAD-Public/job/PR-9683-head/6/stages/?selected-node=1005 But still I can make it display something like this. |
Collaborator
Author
|
@rovinski Better? 
Clock Insertion Latency TooltipWhen viewing timing path details, hovering over any pin of a macro The tooltip displays:
This delay can exceed the data path delay through the macro. |
Replace the inline "[clk ins: ...]" annotation and blue text coloring with a tooltip on macro pins in the timing path detail view. The tooltip shows clock insertion latency from the liberty model's max_clock_tree_path (data path) or min_clock_tree_path (capture path) attribute. The tooltip appears on any pin of a macro instance that has a clock port with non-zero clkTreeDelay, so users see the information when hovering over the data output pin where the macro's delay is visible. The tooltip includes the macro name and explains that this is the internal clock tree delay to sequential elements. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
Author
|
@maliberty @vvbandeira macos ci download outage |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
Author
CI Failure Report: PR #9683 — Mac-BuildSummaryThe Mac-Build GitHub Actions job failed due to a GitHub download outage (502 Bad Gateway) when fetching a Boost dependency. This is an infrastructure issue, not a code issue. Links
What FailedDuring Bazel analysis of Root CauseGitHub returned 502 Bad Gateway for the Boost archive download. This is a transient GitHub infrastructure issue. Other CI Status
Suggested FixRe-run the Mac-Build job. Optionally, add a mirror URL for |
maliberty
merged commit abd8278
into
The-OpenROAD-Project:master
15 of 16 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add source and target clock insertion delay columns to the timing paths table, hidden by default. The values come from PathEnd's sourceClkInsertionDelay/targetClkInsertionDelay (liberty max/min_clock_tree_path timing groups). Also annotate the clock summary row in the detail view with the insertion delay when non-zero.
Closes #9681