From a5839b8ec043d674407de54c44e6106916a279eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Harboe?= Date: Fri, 15 May 2026 12:48:32 +0200 Subject: [PATCH 1/3] flow: record yosys netlist hashes in rules-base.json (warning-level) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Øyvind Harboe --- flow/util/checkMetadata.py | 9 ++++++++- flow/util/genMetrics.py | 23 +++++++++++++++++++++++ flow/util/genRuleFile.py | 36 +++++++++++++++++++++++++++++------- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/flow/util/checkMetadata.py b/flow/util/checkMetadata.py index 84173158b6..1a525d88a8 100755 --- a/flow/util/checkMetadata.py +++ b/flow/util/checkMetadata.py @@ -106,8 +106,15 @@ def try_number(string): PRE = "[INFO]" CHECK = "pass" elif rule.get("level") == "warning": + # Warning-level rules never fail the build, but the prior + # message ("[WARN] field pass test: a == b") was misleading + # when a != b -- the build_value clearly differed from the + # rule_value yet "pass" implied a match. Say "differs" + # instead so the diagnostic reads naturally for fields like + # the netlist hash where the user wants visibility without + # an error. PRE = "[WARN]" - CHECK = "pass" + CHECK = "differs" WARNS += 1 else: PRE = "[ERROR]" diff --git a/flow/util/genMetrics.py b/flow/util/genMetrics.py index e9c32879c1..45e0d688d5 100755 --- a/flow/util/genMetrics.py +++ b/flow/util/genMetrics.py @@ -5,6 +5,7 @@ # information in specific files using regular expressions # ----------------------------------------------------------------------------- +import hashlib import os import shutil from datetime import datetime, timedelta @@ -190,6 +191,18 @@ def git_head_commit(git_exe, folder): ) +def file_sha1(path): + """SHA-1 of `path`, or "N/A" if absent. Read in chunks so large + netlists don't blow the heap.""" + if not os.path.isfile(path): + return "N/A" + hasher = hashlib.sha1() + with open(path, "rb") as f: + for chunk in iter(lambda: f.read(16 * 1024 * 1024), b""): + hasher.update(chunk) + return hasher.hexdigest() + + def merge_jsons(root_path, output, files): paths = sorted(glob(os.path.join(root_path, files))) for path in paths: @@ -249,6 +262,16 @@ def extract_metrics( rptPath + "/synth_stat.txt", ) + # Netlist hashes: fingerprints of the canonical RTLIL (pre-ABC) and + # the final post-synthesis Verilog so the rules-base.json check + # (level=warning) flags when bazel-built vs make-built yosys + # disagree for the same RTL. See flow/README.md "Triaging a failing + # _test" → "Yosys-environment false positive" for context. + metrics_dict["synth__canonical_netlist__hash"] = file_sha1( + resultPath + "/1_1_yosys_canonicalize.rtlil" + ) + metrics_dict["synth__netlist__hash"] = file_sha1(resultPath + "/1_2_yosys.v") + # Clocks # ========================================================================= clk_list = read_sdc(resultPath + "/2_floorplan.sdc") diff --git a/flow/util/genRuleFile.py b/flow/util/genRuleFile.py index 885b4a473d..358d59231e 100755 --- a/flow/util/genRuleFile.py +++ b/flow/util/genRuleFile.py @@ -46,8 +46,11 @@ def gen_rule_file( # dict format # 'metric_name': { - # 'mode': , one of ['direct', 'sum_fixed', 'period', 'padding', - # 'period_padding', 'abs_padding', 'metric'] + # 'mode': , one of ['direct', 'literal', 'sum_fixed', 'period', + # 'padding', 'period_padding', 'abs_padding', + # 'metric']. 'literal' propagates the metric + # value verbatim (e.g. a hash string) and + # skips all numeric padding/rounding. # 'padding': , percentage of padding to use # 'fixed': , sum this number instead of using % padding # 'round_value': , use the rounded value for the rule @@ -71,6 +74,21 @@ def gen_rule_file( "level": "warning", }, # synth + # Yosys netlist hash fingerprints. `mode: literal` propagates + # the string value verbatim; `level: warning` means a mismatch + # surfaces as a [WARN] diagnostic in checkMetadata.py without + # failing the build, matching how rules-base.json already + # treats warning counts. + "synth__canonical_netlist__hash": { + "mode": "literal", + "compare": "==", + "level": "warning", + }, + "synth__netlist__hash": { + "mode": "literal", + "compare": "==", + "level": "warning", + }, "synth__design__instance__area__stdcell": { "mode": "padding", "padding": 15, @@ -279,7 +297,7 @@ def gen_rule_file( if ":" in field: field = field.replace(":", "__") processed_fields.add(field) - if isinstance(metrics[field], str): + if isinstance(metrics[field], str) and option["mode"] != "literal": print(f"[WARNING] Skipping string field {field} = {metrics[field]}") continue @@ -291,6 +309,9 @@ def gen_rule_file( if option["mode"] == "direct": rule_value = metrics[field] + elif option["mode"] == "literal": + rule_value = metrics[field] + elif option["mode"] == "sum_fixed": rule_value = metrics[field] + option["padding"] @@ -342,10 +363,11 @@ def gen_rule_file( print(f"[ERROR] Metric {field} has invalid mode {option['mode']}.") sys.exit(1) - if option["round_value"] and not isinf(rule_value): - rule_value = int(round(rule_value)) - else: - rule_value = float(f"{rule_value:.3g}") + if option["mode"] != "literal": + if option["round_value"] and not isinf(rule_value): + rule_value = int(round(rule_value)) + else: + rule_value = float(f"{rule_value:.3g}") preserve_old_rule = ( True From 3fde4a2c234251b9fe9ff735b072a3c0c1f65b5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Harboe?= Date: Fri, 15 May 2026 12:51:12 +0200 Subject: [PATCH 2/3] flow: hash .v / .rtlil / .odb / .sdc separately in elapsed-time table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Øyvind Harboe --- flow/test/test_genElapsedTime.py | 30 +++++++++++++ flow/util/genElapsedTime.py | 73 ++++++++++++++++++++++---------- 2 files changed, 81 insertions(+), 22 deletions(-) diff --git a/flow/test/test_genElapsedTime.py b/flow/test/test_genElapsedTime.py index 36da00209e..33a06dbd64 100755 --- a/flow/test/test_genElapsedTime.py +++ b/flow/test/test_genElapsedTime.py @@ -79,6 +79,36 @@ def test_no_elapsed_time(self, fake_err_output): genElapsedTime.scan_logs(["--logDir", str(self.tmp_dir.name), "--noHeader"]) self.assertIn("No elapsed time found in", fake_err_output.getvalue()) + @patch("sys.stdout", new_callable=StringIO) + def test_emits_one_row_per_result_extension(self, mock_stdout): + # logs/.../1_2_yosys.log accompanied by both .v and .sdc result + # files should produce two rows: .v with the elapsed/peak, + # .sdc with empty elapsed/peak (the row sharing the stage). + log_dir = os.path.join(self.tmp_dir.name, "logs", "p", "d", "base") + res_dir = os.path.join(self.tmp_dir.name, "results", "p", "d", "base") + os.makedirs(log_dir) + os.makedirs(res_dir) + log_path = os.path.join(log_dir, "1_2_yosys.log") + with open(log_path, "w") as f: + f.write("Elapsed time: 00:00:10[h:]min:sec. Peak memory: 51200KB.\n") + with open(os.path.join(res_dir, "1_2_yosys.v"), "w") as f: + f.write("module foo\nendmodule\n") + with open(os.path.join(res_dir, "1_2_yosys.sdc"), "w") as f: + f.write("create_clock -period 10\n") + genElapsedTime.scan_logs(["--logDir", log_dir, "--noHeader"]) + out = mock_stdout.getvalue() + lines = [l for l in out.splitlines() if "1_2_yosys" in l] + self.assertEqual(len(lines), 2, out) + self.assertIn(".v", lines[0]) + self.assertIn(".sdc", lines[1]) + # elapsed (10) and peak (50) show only on the .v row + self.assertIn("10", lines[0]) + self.assertIn("50", lines[0]) + # the .sdc row repeats neither the elapsed (10) nor peak (50) + # but does include its own hash; check absence of those tokens + self.assertNotIn(" 10 ", " " + lines[1] + " ") + self.assertNotIn(" 50 ", " " + lines[1] + " ") + def tearDown(self): self.tmp_dir.cleanup() diff --git a/flow/util/genElapsedTime.py b/flow/util/genElapsedTime.py index b5f4fc1800..026b048f1f 100755 --- a/flow/util/genElapsedTime.py +++ b/flow/util/genElapsedTime.py @@ -14,24 +14,41 @@ # ============================================================================== -def get_hash(f): - # content hash for the result file alongside .log file is useful to - # debug divergent results under what should be identical - # builds(such as local and CI builds) - for ext in [".odb", ".rtlil", ".v"]: +# 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"] + + +def get_hashes(f): + """Return [(ext, sha1), ...] for every result file alongside log + `f` whose extension is in RESULT_EXTS. A yosys stage typically + produces both `.v` and `.sdc`; a floorplan/route stage produces + `.odb` (and often `.sdc`); the canonicalize stage produces + `.rtlil`. Hashing each separately makes "the netlist changed" + distinguishable from "the SDC changed" in the elapsed-time table + used to triage divergent local vs CI builds. + + Falls back to a single ("", "N/A") entry when no result file + exists so the caller always emits at least one row per stage. + """ + results = [] + for ext in RESULT_EXTS: result_file = pathlib.Path( str(f).replace("logs/", "results/").replace(".log", ext) ) if result_file.exists(): hasher = hashlib.sha1() - with open(result_file, "rb") as odb_f: + with open(result_file, "rb") as rf: while True: - chunk = odb_f.read(16 * 1024 * 1024) + chunk = rf.read(16 * 1024 * 1024) if not chunk: break hasher.update(chunk) - return hasher.hexdigest() - return "N/A" + results.append((ext, hasher.hexdigest())) + if not results: + results.append(("", "N/A")) + return results def print_log_dir_times(logdir, args): @@ -87,37 +104,49 @@ def print_log_dir_times(logdir, args): ) break - odb_hash = get_hash(f) + hashes = get_hashes(f) if not found: print("No elapsed time found in", str(f), file=sys.stderr) continue - # Print the name of the step and the corresponding elapsed time - format_str = "%-25s %10s %14s %20s" + # Print the name of the step and the corresponding elapsed time. + # One row per (stage, result-file-ext); only the first row of a + # stage shows elapsed/peak. + format_str = "%-25s %-6s %10s %14s %20s" if elapsedTime is not None and peak_memory is not None: if first and not args.noHeader: print( format_str - % ("Log", "Elapsed/s", "Peak Memory/MB", "sha1sum result [0:20)") + % ( + "Log", + "Ext", + "Elapsed/s", + "Peak Memory/MB", + "sha1sum result [0:20)", + ) ) first = False - print( - format_str - % ( - stem, - elapsedTime, - peak_memory, - odb_hash[0:20], + stage_first = True + for ext, h in hashes: + print( + format_str + % ( + stem, + ext, + elapsedTime if stage_first else "", + peak_memory if stage_first else "", + h[0:20], + ) ) - ) + stage_first = False if elapsedTime is not None: totalElapsed += elapsedTime if peak_memory is not None: total_max_memory = max(total_max_memory, int(peak_memory)) if totalElapsed != 0 and not args.match: - print(format_str % ("Total", totalElapsed, total_max_memory, "")) + print(format_str % ("Total", "", totalElapsed, total_max_memory, "")) def scan_logs(args): From 31fdacec42065f1f83470a178a318be4b1e998c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Harboe?= Date: Sat, 16 May 2026 12:14:15 +0200 Subject: [PATCH 3/3] fix: address maliberty review nits on #4232 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - genElapsedTime.py: hash .def / .spef / .gds in addition to .v / .rtlil / .odb / .sdc so the elapsed-time table covers all primary flow artifacts, not just the synth + odb subset. - genMetrics.py: drop the flow/README.md "Triaging a failing _test" pointer next to the netlist-hash metrics; the triage section will land in a later PR. Signed-off-by: Øyvind Harboe --- flow/util/genElapsedTime.py | 9 +++++---- flow/util/genMetrics.py | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/flow/util/genElapsedTime.py b/flow/util/genElapsedTime.py index 026b048f1f..209404c394 100755 --- a/flow/util/genElapsedTime.py +++ b/flow/util/genElapsedTime.py @@ -14,10 +14,11 @@ # ============================================================================== -# 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"] +# Primary data artifacts first, then derived/exported artifacts and +# the SDC constraint file: yosys emits .v / .rtlil; OpenROAD stages +# emit .odb (and often .def / .sdc); routing emits .spef; finish +# emits .gds. +RESULT_EXTS = [".v", ".rtlil", ".odb", ".def", ".spef", ".gds", ".sdc"] def get_hashes(f): diff --git a/flow/util/genMetrics.py b/flow/util/genMetrics.py index 45e0d688d5..6424712e27 100755 --- a/flow/util/genMetrics.py +++ b/flow/util/genMetrics.py @@ -265,8 +265,7 @@ def extract_metrics( # Netlist hashes: fingerprints of the canonical RTLIL (pre-ABC) and # the final post-synthesis Verilog so the rules-base.json check # (level=warning) flags when bazel-built vs make-built yosys - # disagree for the same RTL. See flow/README.md "Triaging a failing - # _test" → "Yosys-environment false positive" for context. + # disagree for the same RTL. metrics_dict["synth__canonical_netlist__hash"] = file_sha1( resultPath + "/1_1_yosys_canonicalize.rtlil" )