Timing report HTML generation (integration demo)#9770
Timing report HTML generation (integration demo)#9770oharboe wants to merge 10 commits intoThe-OpenROAD-Project:masterfrom
Conversation
No GUI can fix the most pressing problem: click and wait. In a GUI,
you've lost before you've started. In the age of Claude, a
self-contained HTML artifact generated as part of the build is
strictly superior: instant, shareable, diffable, CI-native, air-gap
safe.
This commit adds 5 new methods to the ord::Timing class that expose
the same timing data the Qt GUI uses (STAGuiInterface), but as POD
structs accessible from Python via SWIG — no framework inversion,
no Qt dependency:
- getWorstSlack(MinMax) — WNS
- getTotalNegativeSlack(MinMax) — TNS
- getEndpointCount() — number of timing endpoints
- getEndpointSlackMap(MinMax) — (pin_name, slack) for all
endpoints (histogram data source, mirrors chartsWidget.cpp)
- getClockInfo() — clock domain metadata
- getTimingPaths(MinMax, max_paths, slack_threshold) — full path
detail with per-arc delay/slew/load/fanout (mirrors
TimingPathsModel + TimingPathDetailModel from the Qt GUI)
Three new structs (ClockInfo, TimingArcInfo, TimingPathInfo) return
plain-old-data with strings and floats — no opaque STA pointers, no
lifetime issues, trivial JSON serialization.
New Python/Tcl API additions are now free to implement and maintain.
It is policy, not work, whether we expose timing data or not. This
commit demonstrates the pattern with unit tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds the timing report pipeline: Python script + Bazel targets
to generate self-contained HTML timing reports and PR-ready markdown
from any ORFS stage.
The HTML replaces Qt GUI timing features in a browser:
- Endpoint slack histogram (click to filter, red/green bars)
- Sortable timing path table with all columns from TimingPathsModel
- Arc waterfall detail (cell/net delay bars, per-arc tooltip)
- Cell type breakdown chart
- Zero external dependencies, works offline
The markdown (<10KB) is optimized for GitHub PR comments:
- Summary table, ASCII histogram, top failing paths
- Auto-pruned to fit 65KB limit
Bazel integration via timing.bzl macro (no bazel-orfs patches needed):
- orfs_timing_stages() creates {name}_{stage}_timing for all stages
- Uses filegroup output_group to extract only ODB+SDC (lightweight)
- 'bazelisk run' opens browser (only depends on HTML, not ODB)
Usage:
cd tools/OpenROAD
bazelisk build //test/orfs/gcd:gcd_synth_timing
bazelisk run //test/orfs/gcd:gcd_synth_timing
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous commit used `openroad -python` which doesn't work in Bazel (no Python compiled in). This rewrites the pipeline: - Replace genrule with custom Starlark rule (`_timing_gen`) that accesses `PdkInfo.libs` for liberty files and `OrfsInfo` for ODB/SDC - Use `py_binary` (//etc:timing_report) with //python/openroad dep - Filter PdkInfo.libs to NLDM FF corner only (loading all corners OOMs) - Move ClockInfo/TimingArcInfo/TimingPathInfo structs to namespace level for SWIG compatibility (nested structs don't get Python attrs) - Extract timing paths via Tcl `report_checks -format json` (the C++ getTimingPaths() aborts after getWorstSlack() — STA search state reuse bug, documented in docs/timing_api_bugs.md) - Extract clock info via Tcl `all_clocks` / `get_property` - Switch HTML theme from dark to white to match OpenROAD - Add mock-array timing targets (Element + MockArray 4x4_base) Known bugs documented in docs/timing_api_bugs.md: - C++ getTimingPaths()/getClockInfo() abort after getWorstSlack() - Likely fix: call sta->searchPreamble() before findPathEnds() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
- All values in picoseconds (×1e12) matching Qt GUI staToUser() - Histogram: nice bucket algorithm from chartsWidget.cpp (snapInterval with 10 default buckets, red/green bars) - Path table: columns match Qt TimingPathsModel — Capture Clock, Required, Arrival, Slack, Skew, Logic Delay, Logic Depth, Fanout, Start, End - Path detail: Pin (with cell), Fanout, ↑/↓, Time, Delay, Slew, Load matching TimingPathDetailModel - Dropdowns: Setup Slack, No Path Group, All Clocks (from chartsWidget) - Draggable sash between path table and detail panel - Extracted timing paths via Tcl report_checks -format json - Updated TODO with Qt GUI discrepancy audit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Qt source-of-truth comment added to timing_report.py. Fixes applied (matching chartsWidget.cpp / staGui.cpp behavior): - Filter unconstrained endpoints (infinite slack) from histogram - Show "Number of unconstrained pins: N" below histogram - All path groups from STA (including empty ones like in2out) - Tooltip format: "Number of Endpoints: N / Interval: [lo, hi) ps" - Bar colors match Qt: #f08080/#8b0000 neg, #90ee90/The-OpenROAD-Project#6400 pos - <No clock> for unconstrained paths in Capture Clock column - Logic Delay column fixed (was showing arrival) - Tiebreaker sort by endpoint name for equal slack - Histogram click filter with float precision epsilon - Double-click histogram to clear filter - Sash between histogram and timing report panels - X-axis labels use actual bin interval Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Qt source-of-truth comment added to timing_report.py. Fixes applied (matching chartsWidget.cpp / staGui.cpp behavior): - Filter unconstrained endpoints (infinite slack) from histogram - Show "Number of unconstrained pins: N" below histogram - All path groups from STA (including empty ones like in2out) - Tooltip format: "Number of Endpoints: N / Interval: [lo, hi) ps" - Bar colors match Qt: #f08080/#8b0000 neg, #90ee90/The-OpenROAD-Project#6400 pos - <No clock> for unconstrained paths in Capture Clock column - Logic Delay column fixed (was showing arrival) - Tiebreaker sort by endpoint name for equal slack - Histogram click filter with float precision epsilon - Double-click histogram to clear filter - Sash between histogram and timing report panels - X-axis labels use actual bin interval Note: C++ getSlackHistogram() was attempted but crashes due to the same STA search state issue as getTimingPaths(). The JS bucketing algorithm (snapInterval) matches the C++ logic but may produce slightly different results due to float precision. The C++ approach is saved in git stash for when the STA crash is resolved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request introduces an impressive feature for generating self-contained HTML timing reports. The implementation is comprehensive, covering C++ API extensions, Python scripting for report generation, and Bazel rules for integration. My review focuses on improving the correctness of the generated reports, enhancing robustness, and increasing maintainability. I've identified a few areas for improvement, including the calculation of logic delay, error handling, and configuration management.
| except Exception: | ||
| return [] |
There was a problem hiding this comment.
Catching a generic Exception can hide unexpected errors and make debugging more difficult. It's better to catch specific exceptions that you expect to handle, such as json.JSONDecodeError if the file content is not valid JSON, or IOError for file-related issues.
| except Exception: | |
| return [] | |
| except json.JSONDecodeError as e: | |
| print(f"[timing_report] ERROR: Failed to parse JSON from report_checks output: {e}", file=sys.stderr) | |
| return [] |
| # Compute logic depth | ||
| cells = set() | ||
| for a in p["arcs"]: | ||
| if not a["net"] and a["cell"]: | ||
| cells.add(a["to"]) | ||
| p["logic_depth"] = len(cells) |
There was a problem hiding this comment.
The logic_delay for each path is not being calculated, causing the report to incorrectly display the arrival time instead. You can calculate logic_delay here by summing the delays of the non-net arcs in the path.
After this change, you should also update line 524 in the Javascript template from <td>${{ps(p.logic_delay || p.arrival)}}</td> to <td>${{ps(p.logic_delay)}}</td> to ensure the correct value is displayed.
| # Compute logic depth | |
| cells = set() | |
| for a in p["arcs"]: | |
| if not a["net"] and a["cell"]: | |
| cells.add(a["to"]) | |
| p["logic_depth"] = len(cells) | |
| # Compute logic depth and logic delay | |
| cells = set() | |
| logic_delay = 0.0 | |
| for a in p["arcs"]: | |
| if not a["net"] and a["cell"]: | |
| cells.add(a["to"]) | |
| logic_delay += a["delay"] | |
| p["logic_depth"] = len(cells) | |
| p["logic_delay"] = logic_delay |
| file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| lib_files = os.environ.get("LIB_FILES", "").split() |
There was a problem hiding this comment.
If the LIB_FILES environment variable is not set or is an empty string, os.environ.get("LIB_FILES", "").split() will result in a list containing an empty string ['']. This could cause issues later when trying to read the liberty file. It's safer to handle the case of an empty string explicitly.
| lib_files = os.environ.get("LIB_FILES", "").split() | |
| lib_files_str = os.environ.get("LIB_FILES", "") | |
| lib_files = lib_files_str.split() if lib_files_str else [] |
| if (!is_net && !pin_is_clock && inst) { | ||
| if (logic_insts.find(inst) == logic_insts.end()) { | ||
| logic_insts.insert(inst); | ||
| logic_depth_count++; | ||
| logic_delay_total += pin_delay; | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of logic_delay_total includes delays from all instance arcs that are not on a clock path. However, the standard definition of logic delay typically excludes buffers and inverters. To improve accuracy, consider checking if the instance is a buffer or inverter and excluding its delay from logic_delay_total.
You could use design_->getResizer()->isBuffer(lib_cell) and is_inverter for this check, which would require including rsz/Resizer.hh.
| lib_files = [ | ||
| f | ||
| for f in stage[PdkInfo].libs.to_list() | ||
| if "NLDM" in f.path and "_FF_" in f.path | ||
| ] |
There was a problem hiding this comment.
Hardcoding the filter for liberty files based on "NLDM" and "_FF_" in the path is brittle and may not work for all PDKs or corners. As noted in your documentation, this should be improved. A more robust solution would be to propagate the correct list of library files from the orfs_flow rule, possibly through a custom provider, rather than relying on string matching within this rule.
| } | ||
| } | ||
| for (const sta::Pin* pin : clk->pins()) { | ||
| info.sources.push_back(network->pathName(pin)); |
There was a problem hiding this comment.
warning: use emplace_back instead of push_back [modernize-use-emplace]
| info.sources.push_back(network->pathName(pin)); | |
| info.sources.emplace_back(network->pathName(pin)); |
|
|
||
| const bool is_setup = (minmax == Max); | ||
| sta::SceneSeq scenes = sta->scenes(); | ||
| sta::StringSeq group_names; |
There was a problem hiding this comment.
warning: no header providing "sta::StringSeq" is directly included [misc-include-cleaner]
src/Timing.cc:35:
- #include "sta/TimingArc.hh"
+ #include "sta/StringUtil.hh"
+ #include "sta/TimingArc.hh"| sta::StringSeq group_names; | ||
|
|
||
| sta::Search* search = sta->search(); | ||
| sta::PathEndSeq path_ends = search->findPathEnds( |
There was a problem hiding this comment.
warning: no header providing "sta::PathEndSeq" is directly included [misc-include-cleaner]
src/Timing.cc:35:
- #include "sta/TimingArc.hh"
+ #include "sta/SearchClass.hh"
+ #include "sta/TimingArc.hh"|
|
||
| for (auto& path_end : path_ends) { | ||
| TimingPathInfo path_info; | ||
| sta::Path* path = path_end->path(); |
There was a problem hiding this comment.
warning: no header providing "sta::Path" is directly included [misc-include-cleaner]
src/Timing.cc:27:
- #include "sta/PathEnd.hh"
+ #include "sta/Path.hh"
+ #include "sta/PathEnd.hh"| } | ||
| if (node_fanout > max_fanout) { | ||
| max_fanout = node_fanout; | ||
| } |
There was a problem hiding this comment.
warning: use std::max instead of > [readability-use-std-min-max]
| } | |
| max_fanout = std::max(node_fanout, max_fanout); |
Qt becomes a developer-only dependency. Python is build-time only. The static HTML replicates all user-facing GUI features with zero runtime dependencies. Prioritized feature list from minimum churn (Tcl-only fixes) to full layout viewer parity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
a9639df to
4e36086
Compare
Summary
No more click and wait in timing reports. Run a build overnight and get histogram and timing reports in seconds without having to load timing information.
This is the first demo of a grander vision of static HTML GUI and the Qt GUI being a developer tool and the single source of truth for the HTML GUI.
Self-contained HTML timing reports generated from any ORFS stage, matching the OpenROAD Qt GUI (Charts + Timing Report widgets) as closely as possible without modifying
src/sta.This is an integration demo. Individual features and bug fixes will be broken out into separate PRs. This branch will be force-rebased on top of those changes as they land, serving as a living status report.
Usage
Adding timing reports to any ORFS design is one line in BUILD:
What it produces
A single self-contained HTML file (~300KB for MockArray) with:
chartsWidget.cpp, red/green bars, dropdowns for path group and clock filteringTimingPathsModel: Capture Clock, Required, Arrival, Slack, Skew, Logic Delay, Logic Depth, Fanout, Start, End (all in ps)TimingPathDetailModel)Known Issues
STA search state crash (blocker for C++ extension points)
The C++
Timing::getTimingPaths(),getClockInfo(), andgetSlackHistogram()abort when called from the Bazel Python API. The root cause is that STA's internal search state is left in a condition where subsequent calls crash.ensureGraph()+searchPreamble()don't help.Workaround: timing paths extracted via Tcl
report_checks -format json. Histogram bucketing done in JS.Fix needed: changes to
src/stato properly reset search state between Python API calls.Histogram buckets differ slightly from Qt
The JS
snapIntervalalgorithm matcheschartsWidget.cppbut unconstrained endpoint filtering differs slightly.See
docs/timing_report_todo.mdfor the full Qt GUI discrepancy list.An .md file for pull requests?
There's also an .md file with information that could be used in a pull request, but that's a little bit off topic, but here is what it looks like currently. Perhaps drop this and focus on .html in this PR.
PASS Timing —
MockArray—asap7| Setup WNS0.0000 nsTNS0.0000 ns| Hold WNS0.0000 ns0.0000 ns0.0000 ns0.0000 ns0.0000 nsClock Domains
clock250.0Cell Type Breakdown (top 10 by delay)
MockArray0.00000.0000Full interactive report: see
1_timing.htmlartifactTest plan
bazelisk build //test/orfs/gcd:gcd_synth_timingpassesbazelisk build //test/orfs/mock-array:MockArray_4x4_base_synth_timingpassesbazelisk runopens HTML in browser with histogram + path table + detail🤖 Generated with Claude Code