Skip to content

opt_argmax pass#180

Merged
akashlevy merged 1 commit into
mainfrom
opt_argmax
Jun 2, 2026
Merged

opt_argmax pass#180
akashlevy merged 1 commit into
mainfrom
opt_argmax

Conversation

@akashlevy
Copy link
Copy Markdown

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.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR adds opt_argmax, a new Yosys optimization pass that detects combinational masked-argmax loops (priority selection with validity bits, value lookup, and a strict-< update condition) and replaces the serial loop-carried cone with a balanced binary compare tree of {valid, value, index} records. Detection is guarded by structural shape checks (bmux + lt presence, power-of-two width 4–64, matching index width) followed by a behavioral fingerprint via ConstEval over a set of covering test vectors.

  • Core pass (opt_argmax.cc): builds a driver map and output cone, enumerates (valid, index, values) port triples, validates candidates with fingerprinting, then emits a balanced tree of $not / $lt / $and / $or / $mux / $bmux cells and disconnects the old output cone.
  • Tests (opt_argmax.sv / opt_argmax.ys): 15 test cases including SAT-based equivalence proofs for several widths and value-width combinations, plus negative tests that confirm non-matching patterns are left untouched.

Confidence Score: 4/5

The pass is safe to merge for typical use; the rewrite logic and fingerprint tests are thorough, and the non-matching cases are well-covered.

The core detection and tree-emission logic is correct and guarded by a behavioral fingerprint over covering test vectors. Existing negative tests confirm the pass leaves non-matching designs untouched. The findings are all quality or documentation issues rather than incorrect transformations, and they do not affect correctness of the emitted netlist.

passes/opt/opt_argmax.cc — the dead cell member, the early addWire in disconnect_old_output, and the undocumented value_width > 62 limit are worth a second look before this ships to a broader audience.

Important Files Changed

Filename Overview
passes/opt/opt_argmax.cc New 775-line pass implementing fingerprint-based detection and balanced-tree rewrite of masked argmax loops; contains a dead struct member cell, unconditional allocation of a dangling wire in disconnect_old_output, and missing comments on non-obvious limits (value_width > 62, is_sequential exclusion list).
passes/opt/Makefile.inc Adds opt_argmax.o to the build and reorders opt_addcin.o to be alphabetically consistent with neighbouring entries.
tests/silimate/opt_argmax.sv Comprehensive set of 14 SystemVerilog test modules covering basic, scaled, flat-bus, multi-region, shared-consumer, and negative (non-power-of-two, bad index width, tie, non-zero default, min-select) cases for the new pass.
tests/silimate/opt_argmax.ys Yosys script with 15 test cases including SAT-based equivalence proofs and structural cell-count assertions; all tests rely on Verific (verific -import / verific -cfg), so the suite cannot run without a Verific-enabled build.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OptArgmaxPass::execute] --> B[For each selected module]
    B --> C[OptArgmaxWorker: build_indexes\nbuild bit_to_driver map\ncollect input_port_bits]
    C --> D[run: enumerate output wires]
    D --> E{out_width >= 2?}
    E -- No --> D
    E -- Yes --> F[get_cone: BFS from output\nup to max_cone_cells / max_leaf_bits]
    F --> G{cone has $bmux?}
    G -- No --> D
    G -- Yes --> H[For each valid input wire\nwidth == 2^out_width]
    H --> I[Collect index_buses / values_buses\nincl. split buses]
    I --> J[check_candidate]
    J --> K{width in range?\npower-of-two?\nindex_width matches?\nvalue_width in range?}
    K -- No --> H
    K -- Yes --> L{cone_has_required_shape?\nleaves_are_candidate_inputs?\nfind_anchor_driver?}
    L -- No --> H
    L -- Yes --> M[fingerprint via ConstEval\nover covering test vectors]
    M --> N{Behavioral match?}
    N -- No --> H
    N -- Yes --> O[Record Candidate\nclaimed_outputs.insert out]
    O --> D
    D --> P[For each accepted Candidate]
    P --> Q[emit_argmax:\nbmux leaves + balanced tree\nof not/lt/and/or/mux]
    Q --> R[disconnect_old_output]
    R --> S[module->connect out_wire to new output]
    S --> T[run_pass: clean -purge]
Loading

Reviews (1): Last reviewed commit: "opt_argmax pass" | Re-trigger Greptile

Comment thread passes/opt/opt_argmax.cc
Comment thread passes/opt/opt_argmax.cc
Comment thread passes/opt/opt_argmax.cc
Comment thread passes/opt/opt_argmax.cc
@akashlevy akashlevy merged commit 6a5aecf into main Jun 2, 2026
6 checks passed
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