[ENG-2598] Summarize unmatched-register warnings in reg_rename - #296
[ENG-2598] Summarize unmatched-register warnings in reg_rename#296akashlevy wants to merge 1 commit into
Conversation
Per-cell warnings flooded the log on large designs; keep a few examples and a total count instead. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThe PR changes
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking opportunity to bound diagnostic memory usage as well as warning output. Warning aggregation preserves matching and annotation behavior, and the new regression test exercises the intended output, but retaining three strings for every unmatched cell adds avoidable scaling overhead. Files Needing Attention: passes/silimate/reg_rename.cc
|
| Filename | Overview |
|---|---|
| passes/silimate/reg_rename.cc | Correctly bounds warning output, but unnecessarily retains full string records for every unmatched cell rather than counting entries after the first eight. |
| tests/silimate/reg_rename.ys | Adds a harness-discovered regression script that consistently expects eight examples and one summary for eleven unmatched cells. |
| tests/silimate/reg_rename.il | Provides twelve named one-bit flip-flops for the warning-aggregation test. |
| tests/silimate/reg_rename_unmatched.vcd | Dumps only q0 so the test fixture deterministically produces eleven unmatched register cells. |
Reviews (1): Last reviewed commit: "[ENG-2598] Summarize unmatched-register ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (layout_it == reg_layouts.end()) { | ||
| log_warning("Unable to find matching register %s in VCD for cell %s in scope %s\n", | ||
| reg.c_str(), log_id(cell->name), vcd_scope.c_str()); | ||
| unmatched.push_back({reg, log_id(cell->name), vcd_scope}); |
There was a problem hiding this comment.
For a large design with many unmatched flip-flops, this appends three owned strings for every cell even though only the first eight records are displayed; retain only those examples and increment a separate total to avoid unnecessary O(N) diagnostic memory and allocations.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
I don't think this is critical, having the warnings is good for now, because customer can just share the log |
1 participant
Summary
reg_renamelogged oneUnable to find matching registerwarning per unmatched FF cell, which produced tens of thousands of lines on large designs even when matching under the power scope was complete.Fixes ENG-2598.
Test plan
make -C tests/silimate reg_rename.ys(12 named FFs, VCD dumps one of them → 8 example warnings +total unmatched: 11)Made with Cursor