opt_carry_select pass#189
Conversation
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge; the carry-select decomposition is mathematically correct and the peepopt guard prevents regression. The only actionable items are non-blocking style gaps and a missing multi-adder test. The transformation logic, arrival-time model, and peepopt guard are all correct. The SAT-based equivalence tests validate functional correctness for the 32- and 64-bit cases. The open items are: function-level comments absent from the two core routines (qualifies, apply), a minor formula divergence in log2p1 vs the sibling pass, silent acceptance of negative CLI arguments, and a missing multi-adder integration test. None of these cause incorrect netlists, but they could trip up future maintainers or hide misconfigured invocations. passes/silimate/opt_carry_select.cc warrants a second look for the missing function comments and the argument-validation gap; tests/silimate/opt_carry_select.ys could benefit from an additional multi-adder test case. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["$add (wide_early A + narrow_late B)\nA_WIDTH=W, B_WIDTH=k, Y_WIDTH=w"]
A -->|"opt_carry_select fires\narr_narrow > arr_wide"| B
subgraph CarrySelect["Carry-Select Structure (tagged carry_select)"]
B["cs_lo_add\nA[k-1:0] + B → losum[k:0]"]
C["cs_hi_inc_add carry_select\nA[w-1:k] + 1 → hiinc[w-k-1:0]"]
D["cs_hi_mux carry_select\ncarry ? hiinc : A[w-1:k]"]
E["concat\n{ hi[w-k-1:0], losum[k-1:0] } → Y"]
B -->|"losum[k-1:0] (low sum)"| E
B -->|"losum[k] (carry)"| D
C -->|"hiinc"| D
D -->|"hi"| E
end
E --> F["Y (w bits)"]
G["peepopt -muxorder\nwould fold cs_hi_inc_add + cs_hi_mux\nback into A[w-1:k] + carry"]
D -.->|"carry_select attr blocks muxorder"| G
C -.->|"carry_select attr blocks muxorder"| G
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["$add (wide_early A + narrow_late B)\nA_WIDTH=W, B_WIDTH=k, Y_WIDTH=w"]
A -->|"opt_carry_select fires\narr_narrow > arr_wide"| B
subgraph CarrySelect["Carry-Select Structure (tagged carry_select)"]
B["cs_lo_add\nA[k-1:0] + B → losum[k:0]"]
C["cs_hi_inc_add carry_select\nA[w-1:k] + 1 → hiinc[w-k-1:0]"]
D["cs_hi_mux carry_select\ncarry ? hiinc : A[w-1:k]"]
E["concat\n{ hi[w-k-1:0], losum[k-1:0] } → Y"]
B -->|"losum[k-1:0] (low sum)"| E
B -->|"losum[k] (carry)"| D
C -->|"hiinc"| D
D -->|"hi"| E
end
E --> F["Y (w bits)"]
G["peepopt -muxorder\nwould fold cs_hi_inc_add + cs_hi_mux\nback into A[w-1:k] + carry"]
D -.->|"carry_select attr blocks muxorder"| G
C -.->|"carry_select attr blocks muxorder"| G
Reviews (1): Last reviewed commit: "opt_carry_select pass" | Re-trigger Greptile |
If your work is part of a larger effort, please discuss your general plans on Discourse first to align your vision with maintainers.
What are the reasons/motivation for this change?
Explain how this is achieved.
Make sure your change comes with tests. If not possible, share how a reviewer might evaluate it.
These template prompts can be deleted when you're done responding to them.