opt_shift: add -expand-keep-arith to keep a variable barrel off an add tree - #298
Merged
Conversation
…d tree -expand rewrites (a OP b) << c into (a << c) OP (b << c) for every OP it handles, without looking at what the amount costs. That is right for a constant amount, which is free rewiring, and for the bitwise ops, where the amount's cone gains a single gate level by moving ahead of OP. It is wrong for a variable amount on $add/$sub: the amount is a barrel, so the rewrite duplicates it, and it lands ahead of the carry chain, so the amount's own cone gains the adder's whole depth instead of bypassing it. On an N-operand add tree the rewrite cascades down every link, ending with N barrels in front of the tree rather than one behind it. -expand-keep-arith restricts -expand to reject exactly that case. It defaults off, so -expand is bit-exact unchanged without it. The motivating shape is a gated accumulate under a variable barrel: six enabled terms summed into a 40-bit accumulator, then shifted by bw + bh. -expand fired six times down the tree, so the shift-amount input drove six barrels and then the whole carry-save tree and its final 39-bit adder, instead of driving one barrel hanging off the adder output. The guard puts the barrel back behind the tree. Tests cover both halves of the condition -- a variable amount on $add and on $sub is held, a constant amount still expands, bitwise ops are untouched -- plus a four-operand tree that has to come out with one barrel rather than four. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThe PR adds an opt-in restriction that prevents variable shifts from being expanded across addition and subtraction while preserving existing default behavior.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or compatibility failures identified. The option defaults off, its state is assigned after each PMG reset, and the guarded pattern preserves the documented constant, bitwise, addition, and subtraction behavior with focused equivalence tests.
|
| Filename | Overview |
|---|---|
| passes/silimate/opt_shift.cc | Adds option help, parsing, and correctly restores the PMG udata flag after each setup. |
| passes/silimate/peepopt_expand_shifts.pmg | Adds a narrowly scoped guard that retains variable shifts after $add and $sub cells. |
| tests/silimate/opt_expand_shifts.ys | Adds equivalence and cell-count coverage for all principal branches of the new option. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Match shifted binary operation] --> B{keep_arith enabled?}
B -- No --> E[Expand shift across operation]
B -- Yes --> C{Operation is add or sub?}
C -- No --> E
C -- Yes --> D{Shift amount fully constant?}
D -- Yes --> E
D -- No --> F[Reject rewrite and retain one output shifter]
Reviews (1): Last reviewed commit: "opt_shift: add -expand-keep-arith to kee..." | Re-trigger Greptile
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.
Summary
-expandrewrites(a OP b) << cinto(a << c) OP (b << c)for everyOPithandles, without looking at what the amount costs.
OPis one gate level, so the amount's cone barely notices moving ahead of it.$add/$subis a barrel. Expanding duplicates it andlands it ahead of the carry chain, so the amount's own cone gains the adder's whole
depth instead of bypassing it. On an N-operand add tree the rewrite cascades down
every link, ending with N barrels in front of the tree rather than one behind it.
-expand-keep-arithrestricts-expandto reject exactly that last case. Itdefaults off, so
-expandis bit-exact unchanged without it and this PR is ano-op to merge on its own.
Motivating shape
A gated accumulate under a variable barrel: six enabled terms sum into a 40-bit
accumulator, then the sum is shifted by
bw + bh.-expandfired six times down thetree, so the shift-amount input drove six barrels and then the entire carry-save tree
and its 39-bit final adder, instead of driving one barrel hanging off the adder output.
Measured in Preqorsor on that design:
lol29.0 -> 28.25,clk449.3 -> 425.0,area295.0 -> 168.7 (the six duplicated barrels were most of the area).Test plan
tests/silimate/opt_expand_shifts.ys— 5 new cases, eachequiv_opt -assert:$addis held (1$add, 1$shl)$subis held$shl)-expandand still pass,including "SHL across ADD" asserting 2
$shl— the default really is unchanged.opt_combine_shifts,opt_chain_shifts,opt_descale_shifts,opt_fuse_shifts,opt_sink_shifts,opt_expand.bit-identical, 2 small area wins, 0 regressions.
Preqorsor side that turns it on: Silimate/preqorsor#2284.
Made with Cursor