Optimize sampling of array bins over consecutive values - #8467
Open
davidharrishmc wants to merge 3 commits into
Open
davidharrishmc wants to merge 3 commits into
davidharrishmc wants to merge 3 commits into
Conversation
An array bin such as 'bins b[] = {[lo:hi]}' was sampled with one
equality test per value, so a bin over thousands of values produced a
sample() function with thousands of tests. That function dominated C++
compile time: one RISC-V architectural test covergroup took 442 s to
compile. Test each run of consecutive values as one range and compute
the bin index from the sampled value's offset.
Signed-off-by: David Harris <David_Harris@hmc.edu>
Extend the range test for array bins to adjacent single-value bins whose values are consecutive, such as the automatic bins of an enum coverpoint over register names. The first bin of a run tests the whole run; the others need no test of their own. Signed-off-by: David Harris <David_Harris@hmc.edu>
Contributor
Author
|
The second commit adds significant new code for only a 10% speed up. It is beyond my skill set to fully understand its interactions with the rest of the system. I would not object to dropping the second commit if that boosts confidence in the correctness. |
davidharrishmc
marked this pull request as ready for review
September 23, 2026 19:05
Member
|
Do you mind dropping the second commit then, for now? |
…ive values" This reverts commit 1c7c5b3. Signed-off-by: David Harris <David_Harris@hmc.edu> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Dropped |
|
Patch coverage from PR workflow #5337 (code coverage of lines changed relative to 39e2d8e): lines: 100.00% (39 of 39 lines) branches: 100.00% (26 of 26 branches) Report: 35942100500 Please get to 100% line coverage, and understand all branches; see the developer docs PR historyWorkflow #5259 report: 35887451987 |
This branch has not been deployed
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
2 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.
An array bin such as
bins b[] = {[lo:hi]}was sampled with one equality test per value, so a bin over thousands of values generated asample()function with thousands ofifstatements. That function dominated C++ compile time: in the RISC-V architectural tests (riscv-arch-test), one covergroup with array bins over about 3000 CSR addresses took 442 s to compile at the default-Os. Itssample()was 10,530 lines.Each run of consecutive values is now tested as one range, and the bin index is computed from the sampled value's offset into the run:
if (lo <= v && v <= hi) cp.incrementBin(base + (v - lo)). Bin names, cross selections, and exclusion metadata are unchanged. Values that are not consecutive, contain X/Z, or belong to a signed or wide coverpoint keep one test per value.On that suite, total Verilator build time falls from 497 s to 35 s, with identical
coverage.dat.SECOND COMMIT REVERTED. Old notes about its effects for future reference
The second commit applies the same range test to adjacent single-value bins whose values are consecutive, such as the automatic bins of an enum coverpoint over register names (#8462). In the RISC-V architectural tests' Vx16 suite, 55% of all bin tests are such runs; the generated C++ shrinks from 38.9 MB to 33.2 MB and the C++ build from 196 s to 180 s, with identical
coverage.dat. A single-value bin joins a run only if it is a normal bin with one integral constant in the coverpoint's range (non-negative for a signed coverpoint) and noiffor wildcard.New test
t_covergroup_array_bins_rangecovers ranges, mixed lists of runs and single values, ignore and illegal array bins, aniffguard, a cross, runs of single-value bins (unsigned, signed, and wide coverpoints), and the non-range cases. Its bin counts match Questa.This change was developed with AI assistance (Claude Code). I reviewed and tested it.