From 676ac184edf5386f6f9c7d7dfbc9d363179b3142 Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Mon, 6 Jul 2026 21:35:51 -0700 Subject: [PATCH 1/2] opt_prienc: guard ConstEval fingerprint inputs against constant/aliased bits The round-robin detector fed sigmap(wire) signals straight into ConstEval::set() while sweeping test vectors. On real designs a candidate bus can have bits tied to constants, repeated bits, or a req/start pair that alias to the same net after sigmap. ConstEval::set() asserts (current_val[i].wire != NULL || current_val[i] == value[i]) when asked to re-pin such a bit to a conflicting value, crashing the pass (consteval.h:83) on designs like veer/picorv32/murax/raygentop under formal synthesis. Add clean_set_signals() and reject any fingerprint candidate whose set-signals contain constant bits, repeated bits, or overlap each other, in both the priority-encoder and round-robin paths. Skipping an unclean candidate only forgoes a possible rewrite; it never produces an incorrect one. Clean candidates (the intended patterns) are unaffected. Co-authored-by: Cursor --- passes/opt/opt_prienc.cc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/passes/opt/opt_prienc.cc b/passes/opt/opt_prienc.cc index 7231db45e..07089283c 100644 --- a/passes/opt/opt_prienc.cc +++ b/passes/opt/opt_prienc.cc @@ -243,6 +243,23 @@ struct OptPriEncWorker { return vs; } + // ConstEval::set() requires every (sigmap-canonical) bit it pins to be a + // distinct free wire bit. Real designs can tie parts of a bus to constants + // or alias nets together, so guard the fingerprint inputs: reject signals + // containing constant or repeated bits, and (across the whole set) any + // overlap between them. This prevents a ConstEval assertion; skipping an + // unclean candidate only forgoes a possible rewrite, never yields a wrong + // one. + static bool clean_set_signals(std::initializer_list sigs) { + pool seen; + for (const SigSpec* sp : sigs) + for (auto bit : *sp) { + if (bit.wire == nullptr) return false; + if (!seen.insert(bit).second) return false; + } + return true; + } + // Run all candidate test vectors through ConstEval and try to match each of // the four PE variants against the recorded outputs. Returns the matched // variant, or NONE. @@ -257,6 +274,9 @@ struct OptPriEncWorker { if (!clz_full_ok && !ctz_full_ok && !clz_short_ok && !ctz_short_ok) return PEVariant::NONE; + if (!clean_set_signals({&T_sig})) + return PEVariant::NONE; + auto vs = gen_test_vectors(N); for (auto& v : vs) { ce.push(); @@ -440,6 +460,8 @@ struct OptPriEncWorker { int fingerprint_rr(SigSpec req_sig, SigSpec start_sig, SigSpec S_sig, int N, int W) { ConstEval ce(module); + if (!clean_set_signals({&req_sig, &start_sig})) + return -1; bool ok0 = true, ok1 = true; auto deck = gen_test_vectors(N); int checks = 0; From 945e4a403ba50928dfb5837e346c4df8af9b4d68 Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Mon, 6 Jul 2026 22:05:50 -0700 Subject: [PATCH 2/2] tests/opt: add generalization coverage for the QoR pattern passes Prove the three pattern detectors work on unseen inputs, not just the RTL they were derived from. Because detection is functional (ConstEval fingerprinting over the reachable input space), correctness is established per case with equiv_opt -assert (full) or a SAT miter clamped to the reachable range (non-power-of-two), and detection is confirmed with a w:*tag* probe. opt_priokey: D1-D3 spelling variants (explicit shift-or set, compound derived guard, accumulator also exported) -- all fire and prove equivalent. E1-E2 parameter sweep P=2..8, S=4..32. E3 non-power-of-two S=12 reachable-range equivalence via SAT miter. E4 same shape under -strict declines to rewrite (formal-flow safety). F1-F2 near-miss negatives (clear accumulator, multi-hot set) -> no rewrite. opt_prienc (round-robin): RR4-RR5 DEPTH sweep 8/32, full sequential equivalence. RR6 non-power-of-two DEPTH=7 reachable-range equivalence (SAT miter). RR7 an entirely different spelling (upward wrap-scan, first-hit) of the same arbiter -- fires and proves equivalent. RR8 fixed-priority (no rotating pointer) negative. opt_first_fit_alloc (coalesce): H1 inline same-category compare (no precomputed matrix) spelling. H2 different slot/field shape (N=8, NB=8, W=3). All new cases pass locally; they avoid brittle exact cell-count asserts so they are robust to upstream optimization drift. Co-authored-by: Cursor --- tests/opt/opt_first_fit_alloc.ys | 100 +++++++++ tests/opt/opt_prienc.ys | 182 ++++++++++++++++ tests/opt/opt_priokey.ys | 343 +++++++++++++++++++++++++++++++ 3 files changed, 625 insertions(+) diff --git a/tests/opt/opt_first_fit_alloc.ys b/tests/opt/opt_first_fit_alloc.ys index a4bec64b0..4ffe36684 100644 --- a/tests/opt/opt_first_fit_alloc.ys +++ b/tests/opt/opt_first_fit_alloc.ys @@ -757,3 +757,103 @@ opt_first_fit_alloc select -assert-count 0 w:*ffa_* design -reset log -pop + +# ============================================================================ +# Group H: coalesce generalization (spelling + shape variants) +# ============================================================================ +# +# The coalesce variant is detected by functional fingerprinting of the dsel +# cone, so it should not depend on the exact way the same-category forwarding +# is written or on the specific N/NB/C shape. These cases vary both. + +# H1: coalesce with the same-category forwarding written INLINE (no precomputed +# same_cat[i][k] matrix) -- the leader compares categories directly in its +# forward loop. Functionally identical to G1's matrix form; must still detect +# the enable-independent coalescing variant and prove equivalent. +log -header "H1: coalesce inline-compare spelling, N=8 (equiv + fires)" +log -push +design -reset +read_verilog -sv <= N) ? (si - N) : si; + rr_dut #(N,W) u1(.req(req),.s(s),.grant(g1),.idx_next(n1)); + rr_ref #(N,W) u2(.req(req),.s(s),.grant(g2),.idx_next(n2)); + always_comb begin + assert (g1 == g2); + assert (n1 == n2); + end +endmodule +EOF +hierarchy -top tb +flatten +chformal -lower +opt -full +sat -verify -prove-asserts -show-ports tb +design -reset +log -pop + +# RR7: DIFFERENT RTL SPELLING of the same rotated-priority function. Instead of +# the customer's downward idx-- last-write-wins loop, scan UPWARD from s+1 with +# wraparound and keep the FIRST hit. This is a functionally identical arbiter +# written in an unrelated style; detection is functional so it must still fire +# and prove equivalent. (Combinational, pointer `s` an input -> full equiv at +# power-of-2 N over all pointer values.) +log -header "RR7: upward-scan spelling variant, N=16 (equiv + fires)" +log -push +design -reset +read_verilog -sv < no round-robin rewrite" +log -push +design -reset +read_verilog -sv <= SW'(S)) ? (k - SW'(S)) : k; // into [0,S) + end + logic [P-1:0] w1, w2; + pk_dut #(P,S,SW) u1(.act(act), .sel_flat(sel_c), .win(w1)); + pk_ref #(P,S,SW) u2(.act(act), .sel_flat(sel_c), .win(w2)); + always_comb assert (w1 == w2); +endmodule +EOF +hierarchy -top tb +flatten +chformal -lower +opt -full +sat -verify -prove-asserts -show-ports tb +design -reset +log -pop + +# E4: SAME non-pow2 S=12 under -strict. Strict validation sweeps the FULL key +# range and rejects rewrites that only hold via out-of-range don't-cares, so +# the pass must decline to rewrite. This is the safety mode used by formal +# synthesis flows: no reliance on out-of-range freedom. +log -header "E4: non-pow2 S=12 -strict -> no rewrite (safety)" +log -push +design -reset +read_verilog -sv < no rewrite" +log -push +design -reset +read_verilog -sv < no rewrite" +log -push +design -reset +read_verilog -sv <