Skip to content

opt_vps: fold uniform gathers over multi-bit elements - #295

Closed
akashlevy wants to merge 1 commit into
mainfrom
qor/opt-vps-multibit-gather
Closed

opt_vps: fold uniform gathers over multi-bit elements#295
akashlevy wants to merge 1 commit into
mainfrom
qor/opt-vps-multibit-gather

Conversation

@akashlevy

Copy link
Copy Markdown

Summary

The uniform-gather fold in opt_vps only matched $bmux cells with WIDTH == 1. Any table whose elements are wider than a bit — a byte FIFO staging window, a packed struct array — had every lane skipped, and bmuxmap -pmux + pmuxtree then expanded each one into a one-hot $eq + mux farm over the whole table.

$bmux is entry-major (Y = A[S*WIDTH +: WIDTH]), so a WIDTH=W cell is W independent gathers over stride-W slices of the table, all sharing one index. Registering one candidate per element bit lets the existing grouping, affine analysis, wrap guards and both emitters carry over unchanged. W barrels over M entries each cost what one barrel over W*M bits would, so splitting the fold per bit is free.

Two details fall out of one cell feeding W groups:

  • Removal is deferred until every group has been emitted. The first group used to retire the cell, after which later groups read freed memory for the cell's index and src attribute.
  • Output bits nothing reads are skipped rather than folded.

Measurements

A 32-lane sliding window over a 128-entry byte table, two access sites (read window plus a scatter's read-back), through bmuxmap -pmux; pmuxtree; techmap; aigmap:

$bmux $shr AIG cells AIG AND nodes
before (fold skipped) 64 0 1,858,838 752,240
after 0 16 104,641 44,184

17.8x fewer AIG cells. End to end in Preqorsor the same design went from 8m37s and 12,187 area to landing 14% under a hand-written barrel-shift reference.

Test plan

  • tests/silimate/opt_vps.ys gains two cases, on a new opt_vps_gather_wide.sv: a SAT self-equivalence proof before/after the pass, and cell counts pinning 16 WIDTH=4 $bmux folding to 8 $shr with none left, plus a -min_gather 0 control proving the fold is what does it.
  • Full tests/silimate suite passes (mux_push.ys fails on unrelated in-flight work in mux_push.cc, and never invokes opt_vps).

Made with Cursor

The uniform-gather fold only matched WIDTH==1 $bmux, so a table of anything
wider than a bit -- a byte FIFO, a packed struct array -- had every lane
skipped and left for bmuxmap/pmuxtree to expand into a one-hot $eq + mux farm
per lane. A 32-lane window over a 128-entry byte table cost 1.86M AIG nodes
that way, against 105K once folded.

$bmux is entry-major, so a WIDTH=W cell is W independent gathers over stride-W
slices of the table sharing one index. Register one candidate per element bit
and the existing grouping, affine analysis and wrap guards carry over
unchanged: W barrels over M entries each cost what one barrel over W*M bits
would, so splitting the fold per bit is free.

One cell now feeds W groups, so removal is deferred until every group has been
emitted -- the later ones still read the cell for its index and src attribute.
Dead output bits are skipped rather than folded.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

The PR generalizes uniform-gather folding so wide $bmux elements become independent barrel shifts over strided table slices, with new equivalence and structural tests. The deferred retirement logic does not preserve live slices belonging to groups that fail the independent emission threshold.

  • Splits each wide $bmux into one gather candidate per live element bit.
  • Defers source-cell removal until all selected groups have been processed.
  • Adds SAT equivalence and cell-count coverage for fully foldable four-bit gathers.

Confidence Score: 4/5

The PR should not merge until partial per-bit group eligibility can no longer cause removal of a $bmux that still drives unmatched live outputs.

Wide $bmux slices are grouped and admitted independently, but emitting any slice schedules the entire source cell for removal, which can leave live sibling outputs without drivers.

Files Needing Attention: passes/silimate/opt_vps.cc

Important Files Changed

Filename Overview
passes/silimate/opt_vps.cc Generalizes gather folding to wide $bmux cells, but whole-cell retirement can delete unmatched live output slices when only some per-bit groups emit.
tests/silimate/opt_vps.ys Adds equivalence and cell-count tests for wide gathers, though both exercise fully foldable groups and miss partial per-bit group eligibility.
tests/silimate/opt_vps_gather_wide.sv Provides a focused wide-element gather fixture with two affine-index banks and fully live four-bit outputs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    B[Wide bmux] --> S[Create one strided slice per live output bit]
    S --> G1[Slice group reaches min_gather]
    S --> G2[Sibling group below min_gather]
    G1 --> E[Reconnect emitted output bit]
    E --> D[Mark entire bmux dead]
    G2 --> K[Keep original output driver required]
    D --> R[Remove entire bmux]
    R --> U[Sibling live output becomes undriven]
Loading

Reviews (1): Last reviewed commit: "opt_vps: fold uniform gathers over multi..." | Re-trigger Greptile

Comment on lines +1041 to +1042
if (gather_dead.insert(c.cell).second)
pmux_replaced++;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Partial groups lose drivers

When only some live output-bit groups from a wide $bmux meet min_gather, emitting one group adds the entire source cell to gather_dead, while reconnecting only that group's output bit. The later whole-cell removal therefore deletes the drivers for live sibling bits whose groups were skipped, changing those outputs.

Knowledge Base Used: Synthesis transformation pipeline

@akashlevy

Copy link
Copy Markdown
Author

Closing as a duplicate of #294, which was opened ~20 minutes earlier and implements the same feature: extending the opt_vps uniform-gather fold from WIDTH == 1 $bmux to WIDTH > 1. Two agents produced these in parallel without knowledge of each other. #294 is the one to review — it is already reviewed clean (5/5, two rounds, no open threads).

Beyond the duplication, the grouping key chosen here is not safe.

The difference

For a WIDTH=W cell, this branch pushes W candidates, one per read output bit b, each into a group keyed on the strided slice {A[(M-1)*W+b], ..., A[W+b], A[b]}. #294 keys on the whole A port, so all W element bits of a cell live in a single group and fold as a unit, with the emitter producing one barrel per element bit inside that group, all sharing one shift amount.

Why per-element-bit keying is unsafe

Two $bmux cells whose A ports overlap but are not identical co-group on the element bits where their strided slices happen to be the same SigSpec, and do not co-group on the bits where the slices differ. If one bit-group reaches min_gather and folds while another bit-group of the same cell stays below it, the cell is retired anyway (every candidate of a folding group is added to gather_dead) and the output bits belonging to the non-folded bit-groups are left with no driver. Deferring removal until after all groups are emitted correctly avoids a use-after-free, but it does not prevent this.

Under #294's whole-A-port key this state is unrepresentable: a cell belongs to exactly one group, and emit_modular_gather drives every read bit of every candidate in it before anything is retired.

Reproduction

Four WIDTH=2, S_WIDTH=2 $bmux reading at idx + lane. Element bit 0 of every entry is shared[e] in all four lanes; element bit 1 is privN[e], unique per lane — so the bit-0 slices are one identical SigSpec (group of 4, folds at the default min_gather 4) and each bit-1 slice is unique (groups of 1, skipped):

module opt_vps_gather_partial (
	input  wire [1:0] idx,
	input  wire [3:0] shared,
	input  wire [3:0] priv0, priv1, priv2, priv3,
	output wire [1:0] y0, y1, y2, y3
);
	wire [3:0][1:0] t0, t1, t2, t3;
	genvar e;
	generate
		for (e = 0; e < 4; e = e + 1) begin : entries
			assign t0[e] = {priv0[e], shared[e]};
			assign t1[e] = {priv1[e], shared[e]};
			assign t2[e] = {priv2[e], shared[e]};
			assign t3[e] = {priv3[e], shared[e]};
		end
	endgenerate
	assign y0 = t0[2'(idx + 2'd0)];
	assign y1 = t1[2'(idx + 2'd1)];
	assign y2 = t2[2'(idx + 2'd2)];
	assign y3 = t3[2'(idx + 2'd3)];
endmodule
verific -cfg veri_optimize_wide_selector 1
verific -cfg db_infer_wide_muxes_post_elaboration 0
read -sv opt_vps_gather_partial.sv
verific -import opt_vps_gather_partial
proc; opt_clean
select -assert-count 4 t:$bmux
opt_vps; opt_clean
check

On this branch, all four cells are retired for the sake of the one bit-group that folded, and half the output bits are dropped:

VPS gather: 4 modular bit-select(s) (M=4) -> $shr src=7, out=4, amt=2 bit(s)
...
1 cells
1   $shr

  cell $shr $auto$opt_vps.cc:1036:emit_modular_gather$vps_gather_shr$2
    connect \A { \shared [2:0] \shared }
    connect \B \idx
    connect \Y { \y3 [0] \y2 [0] \y1 [0] \y0 [0] }
  end

Warning: Wire opt_vps_gather_partial.\y0 [1] is used but has no driver.
Warning: Wire opt_vps_gather_partial.\y1 [1] is used but has no driver.
Warning: Wire opt_vps_gather_partial.\y2 [1] is used but has no driver.
Warning: Wire opt_vps_gather_partial.\y3 [1] is used but has no driver.
Found and reported 4 problems.

priv0..priv3 are simply gone from the netlist. It is a real functional miscompile, not just a check complaint — a miter -equiv of the unoptimized design against the opt_vps'd one (with opt_vps scoped to the gate module) fails:

SAT proof finished - model found: FAIL!

  Signal Name             Dec       Hex           Bin
  --------------- ----------- --------- -------------
  \in_idx                   3         3            11
  \in_priv0                 8         8          1000
  \in_shared               11         b          1011

On #294's code the same design folds nothing (four distinct A ports, four groups of one, none reaching min_gather), all four $bmux survive, check reports 0 problems, and the SAT equivalence passes.

Worth noting for #294's reviewers: this branch passes #294's full tests/silimate/opt_vps.ys, including its test 17, because that fixture uses two fully disjoint tables. Only overlapping-but-not-identical tables expose the difference, so #294 is safe by construction rather than by test — a fixture along the lines above would be a reasonable addition there.

Not deleting qor/opt-vps-multibit-gather, in case anything from it is worth salvaging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant