Repository navigation
Conversation
….2.2, §7.2, aionhw#991) `always_comb c = q;` and `always @* c = q;` depend on the member signals of the unpacked struct `q`. The map from a struct name to those signals was built in `Simulator::new` before its identity port-net collapse, and `compile` can run the collapse again. The collapse points the member names of a continuous copy such as `assign q = p;` at its source, so the process waited on the copy's original member signals, which nothing writes after the collapse, and `c` missed every change. The map is now built right after the last collapse call in `compile`; its only reader, `build_comb_entries`, runs after that point. It is still built once per compile. The prepared combinational cache format is bumped by one, since caches made before this change hold those stale edges (IEEE 1800-2023 9.2.2.2, 9.4.2.2, 10.3.2 and 7.2).
…2, §7.2, aionhw#991) Covers copy chains of one to three steps read by `always_comb` and `always @*`, nested and `real` members changed one at a time, copies inside a child instance and through an output port, and clocked and initial-block readers as controls (IEEE 1800-2023 9.2.2.2, 9.4.2.2 and 7.2).
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
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.
Fixes #991
always_combandalways @*missed changes on a whole read of an unpacked struct that is a continuous copy of another and is folded into it by the identity port-net collapse (assign q = p; always_comb c = q;). The testcase is in the issue.Cause. A whole-struct read depends on the struct's member signals. The map from a struct name to those signals (
unpacked_struct_sens) was built inSimulator::newbefore its identity port-net collapse, andcompilecan run the collapse again. The collapse points the member names of a continuous copy at its source, so the procedures waited on the copy's original member signals, which nothing writes after the collapse.Change (code and tests in separate commits).
compile, and no longer inSimulator::new. Its only reader,build_comb_entries, runs after that point.PREPARED_COMB_MAGICgoes fromXZCMB030toXZCMB031, as for An always_comb that writes one packed element and reads another never wakes (§9.2.2.2.1) #844, because caches made before this change hold the stale edges.Testing
New
tests/scheduling/unpacked_struct_copy_chain.rs:always_combandalways @*;realmembers changed one at a time;With the
src/change reverted, three of the four tests fail; the clocked/initial control passes either way.All nine generated test programs pass slang 12.1 with no diagnostics.
Full suites on this branch:
Same output on a struct-nettype charge-pump regulator model. Three alternating runs on a shared machine give medians of 100.5 s on main and 101.2 s with this change, which is within noise. WSL has no hardware counters, so there is no
perf statcount. The change is outside the event loop.🤖 Generated with Claude Code