opt_accum_enable: fold a zero-gated register update into the enable - #297
Merged
Merged
Conversation
When a register's next value is `q OP z` for an identity operator and some condition holds `z` at zero, the register keeps its value: the condition is a hold, not arithmetic, so it belongs on the enable rather than in front of the adder. Folding it takes the condition's whole cone -- typically a wide compare -- off the path into the update and leaves a one-bit term on the enable. The rewrite only fires when every net the gate feeds is read by that one accumulate, so dropping the gate cannot be observed anywhere else, and the zero is traced forward through the network in between rather than only across the cell next to the operator. Tests prove the positive cases with a sequential miter (the zeroed update is an observability don't-care once the register holds, so gold and gate differ on internal nodes and only the outputs agree) and pin down the guards: a value read elsewhere, a non-zero off arm, an update that is not `q OP z`, a sum observed combinationally, a partial-width mask, and a gate shared by two accumulators. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryAdds the
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| passes/opt/opt_accum_enable.cc | Implements gated-accumulator detection, safety analysis, candidate selection, and enable rewriting; the prior selection-scope defect is resolved at the candidate-register boundary. |
| tests/opt/opt_accum_enable.ys | Covers supported accumulator forms, observability guards, shared gates, existing enables, synchronous resets, and inverted gating through structural assertions and sequential equivalence proofs. |
| passes/opt/CMakeLists.txt | Registers the new optimization pass as a build component. |
Reviews (2): Last reviewed commit: "opt_accum_enable: honor the cell selecti..." | Re-trigger Greptile
Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
@greptileai review |
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.
What
New pass
opt_accum_enable. When a register's next value isq OP z($add,$sub,$or,$xor) and some condition holdszat zero, the register keepsits value while that condition is off. That makes the condition a hold, not
arithmetic, so it belongs on the register's enable rather than in front of the
adder. The pass drops the zeroing gate from the datapath and ANDs its condition
onto the enable.
The win is that the condition's whole cone -- typically a wide compare -- leaves
the path into the update, and only a one-bit term lands on the enable. It also
exposes the register to clock gating.
Why it is sound
$muxwith one constant-zero arm, or an$andwhose maskis a full-width replication of one bit (a narrower mask also clears high bits
that a bypass would not reproduce).
values that change once the gate is gone are not observable anywhere else.
(one-hot spray, or/xor merge, select, shift), so the gate does not have to sit
next to the operator.
registers, are refused.
Tests
tests/opt/opt_accum_enable.ys. The update the gate used to zero becomes anobservability don't-care once the register holds, so gold and gate deliberately
differ on internal nodes and only the module outputs agree; positive groups
therefore prove equivalence with a sequential miter (
sat -tempinduct) ratherthan
equiv_opt. Seven proofs, six negatives:$andmask instead of a mux, and
$sub/$or/$xor.not
q OP z, sum observed combinationally, partial-width mask, gate shared bytwo accumulators.
Landing
Nothing in Yosys invokes the pass, so this is a no-op on its own and safe to
merge first. The Preqorsor half (Silimate/preqorsor) adds the invocation and a
regression; its CI will fail on
Unknown commanduntil this merges and a wheelships.
Measured there on a 64-bit counter whose increment is zeroed by a compare
against the counter's own high half: lol 34.75 -> 26.5, clk 486 -> 377, area
100 -> 97. An A/B over 13 other qor designs (barrel shifters, compressor trees,
carry-save, compare trees, prefix trees, accumulate loops) is bit-identical on
lol/clk/area -- the pass never fires on them.
Made with Cursor