Skip to content

Netlist hash diagnostic#4232

Open
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:netlist-hash-diagnostic
Open

Netlist hash diagnostic#4232
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:netlist-hash-diagnostic

Conversation

@oharboe
Copy link
Copy Markdown
Collaborator

@oharboe oharboe commented May 15, 2026

Reduce wild goose chases with helpful diagnostics

oharboe and others added 2 commits May 15, 2026 12:48
flow/README.md "Triaging a failing _test" → "Yosys-environment false
positive" already calls out that bazel-built yosys and make-built
yosys can produce different 1_2_yosys.v for the same RTL, drifting
QoR past rules-base.json thresholds even though OpenROAD itself is
bit-deterministic.  Today the only way to spot the drift is to
re-run `@bazel-orfs//:make-yosys-netlist`; if a designer has a
freshly-failing _test in front of them, they have no way to see
"the yosys netlist changed" vs "a real regression".

Persist a fingerprint in rules-base.json instead so the next _test
just prints it:

  - genMetrics.py: emit `synth__canonical_netlist__hash` (SHA-1 of
    1_1_yosys_canonicalize.rtlil, the canonical RTLIL pre-ABC) and
    `synth__netlist__hash` (SHA-1 of 1_2_yosys.v, post-ABC).  Having
    both lets the user see whether divergence is in the front-end
    flow or downstream of ABC.

  - genRuleFile.py: new `literal` mode passes the metric value
    through verbatim (no padding / rounding / float coercion).  The
    two hash fields use it with `level: warning` + `compare: ==`,
    so `_update` writes them into rules-base.json and `_test`
    treats a hash mismatch as a diagnostic, not a failure.

  - checkMetadata.py: a warning-level mismatch previously printed
    "[WARN] field pass test: a == b" — confusing when a != b yet
    "pass" implied a match.  Say "differs" instead so the hash
    mismatch reads naturally without changing the no-error contract
    (still doesn't increment ERRORS).

rules-base.json files are unchanged in this commit; the next
`_update` per design adds the two hash fields automatically, and
existing _test runs are unaffected until that happens.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
genElapsedTime.py used to pick exactly one of .odb / .rtlil / .v as
"the result file" for a log and hash that.  Synth produces both
1_2_yosys.v and 1_2_yosys.sdc; OpenROAD stages produce both an
.odb and a .sdc.  Folding all of them into one column means a
divergent .sdc (an SDC-generator change) and a divergent .odb (a
real flow change) look identical in the elapsed-time triage table.

Replace `get_hash` with `get_hashes` that returns every result-file
extension that exists, and emit one row per (stage, extension)
with the elapsed time and peak memory on the stage's first row
only.  Column order is `.v / .rtlil / .odb / .sdc` so the primary
data artifact comes first and the constraint file last.

Added a regression test (`test_emits_one_row_per_result_extension`)
to lock the two-row-per-stage behaviour in; existing tests cover
the no-result-file fallback (single row with ext="" and hash=N/A)
since the table now has an Ext column.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe requested a review from maliberty May 15, 2026 11:00
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the flow's reporting and metadata tracking by introducing multi-file hashing and a "literal" rule mode. Key changes include updating genElapsedTime.py to report hashes for multiple result extensions (including .sdc), adding netlist hash extraction to genMetrics.py, and implementing a "literal" mode in genRuleFile.py to support non-numeric metrics like SHA-1 fingerprints. Additionally, checkMetadata.py was updated to provide clearer warning messages when metadata values differ, and a new test case was added to verify the multi-row output in elapsed time reports. I have no feedback to provide as there were no review comments.

# Primary data artifacts first, then the SDC constraint file: yosys
# emits .v / .rtlil; OpenROAD stages emit .odb; both often emit a
# .sdc alongside.
RESULT_EXTS = [".v", ".rtlil", ".odb", ".sdc"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing def, gds, spef

Comment thread flow/util/genMetrics.py
Comment on lines +268 to +269
# disagree for the same RTL. See flow/README.md "Triaging a failing
# _test" → "Yosys-environment false positive" for context.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see no such section in README.md

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