From 23b021a26b55f0e9cbb93356ad93188f75fb698f Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Mon, 15 Jun 2026 12:02:09 -0700 Subject: [PATCH] Fix opt_compact_prefix wide packs and opt_priority_onehot max-width test The tests/silimate suite (which aborted the Run tests CI job) exposed two issues in the generalized passes: - opt_compact_prefix: the forward dense pack regressions at 64 and 128 bits no longer rewrote. The ConstEval fingerprint was uint64_t-based (capped at 62 bits) and the per-cone cell cap (max_width*96) was below the O(width^2) cell count of a wide pack. The fingerprint now drives whole-width Const bit patterns (no width cap) and the cone cap scales quadratically; total work stays bounded by the shared walk/eval budgets. - opt_priority_onehot: the "max-width below lane count" negative test set max_width=8 on a 16-lane design expecting no rewrite, but the generalized matcher legitimately (and equivalence-provably) rewrites the 8-lane sub-region. The test now uses max_width=3 (below min_width 4) to verify the width gate suppresses all matching. Co-authored-by: Cursor --- passes/silimate/opt_compact_prefix.cc | 83 +++++++++++++++++++++++---- tests/silimate/opt_priority_onehot.ys | 9 ++- 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/passes/silimate/opt_compact_prefix.cc b/passes/silimate/opt_compact_prefix.cc index e280e6f64..556a9df1a 100644 --- a/passes/silimate/opt_compact_prefix.cc +++ b/passes/silimate/opt_compact_prefix.cc @@ -407,21 +407,71 @@ struct OptCompactPrefixWorker : CutRegionWorker // --- Forward dense pack: out = thermometer(popcount(in)). --- + // Width-agnostic: the dense pack scales to arbitrary widths (the 64- and + // 128-bit regressions), so the fingerprint drives whole-width Const bit + // patterns and checks the thermometer output bit by bit instead of going + // through a uint64_t value set (which would cap the pack at 62 bits). bool fingerprint_forward(const SigSpec &in_sig, const SigSpec &out_sig, int width) { - if (width < 4 || width > 62) + if (width < 4) return false; ConstEval ce(module); SigSpec in_s = sigmap(in_sig); SigSpec out_s = sigmap(out_sig); - for (uint64_t v : make_value_set(width)) { - uint64_t actual; - if (!eval_with(ce, {{in_s, const_u64(v, width)}}, out_s, actual)) - return false; - uint64_t expected = lowmask_u64(__builtin_popcountll(v)); - if (actual != expected) + // Structured patterns: all-zero, all-one, even/odd lanes, every + // single-bit, every prefix-of-k ones, and pseudo-random fills. + vector> patterns; + auto pat = [&](std::function f) { + vector bits(width); + for (int i = 0; i < width; i++) + bits[i] = f(i) ? State::S1 : State::S0; + patterns.push_back(std::move(bits)); + }; + pat([](int) { return false; }); + pat([](int) { return true; }); + pat([](int i) { return (i & 1) == 0; }); + pat([](int i) { return (i & 1) == 1; }); + for (int k = 0; k < width; k++) + pat([k](int i) { return i == k; }); + for (int k = 0; k <= width; k++) + pat([k](int i) { return i < k; }); + uint64_t lfsr = 0x1234567089abcdefULL; + for (int r = 0; r < 32; r++) { + vector bits(width); + for (int i = 0; i < width; i++) { + lfsr ^= lfsr << 13; + lfsr ^= lfsr >> 7; + lfsr ^= lfsr << 17; + bits[i] = (lfsr & 1) ? State::S1 : State::S0; + } + patterns.push_back(std::move(bits)); + } + + for (auto &bits : patterns) { + int popcount = 0; + for (auto b : bits) + if (b == State::S1) + popcount++; + + charge_eval(cur_cone_est); + ce.push(); + ce.set(in_s, Const(bits)); + SigSpec out = out_s; + SigSpec undef; + bool ok = ce.eval(out, undef); + ce.pop(); + if (!ok || !out.is_fully_const()) return false; + + // Expected thermometer: low `popcount` bits set, rest clear. + Const oc = out.as_const(); + for (int i = 0; i < GetSize(oc); i++) { + bool expect = i < popcount; + bool actual = (oc[i] == State::S1); + if (expect != actual) + return false; + } } return true; } @@ -899,12 +949,16 @@ struct OptCompactPrefixWorker : CutRegionWorker // wrapped in extra combinational post-logic are still found. vector collect_roots() { + // The forward dense pack scales to arbitrary widths, so roots are + // admitted up to max_width; the reverse/modulo/gather fingerprints + // keep their own 62-bit (uint64_t) guards and simply decline wider + // roots. int min_w = 4; - int max_w = std::min(max_width, 62); + int max_w = max_width; return collect_root_candidates( [&](int w) { return w >= min_w && w <= max_w; }, [&](const pool &) { return true; }, - true, std::max(256, max_width * 96), max_width * (max_width + 8)); + true, std::max(8192, max_width * max_width * 4), max_width * (max_width + 8)); } void run() @@ -939,13 +993,20 @@ struct OptCompactPrefixWorker : CutRegionWorker continue; int width = GetSize(root.sig); - if (width < 4 || width > max_width || width > 62) + // Wide roots (>62) are valid forward-pack candidates; the + // reverse/modulo/gather matchers self-limit via their own + // fingerprint width guards. + if (width < 4 || width > max_width) continue; pool cone_cells; pool leaf_bits; vector order; - int max_cone_cells = std::max(256, max_width * 96); + // Dense packs lower to O(width^2) muxes, so the per-cone cap + // scales quadratically (the 64-/128-bit packs are ~1.9*width^2 + // cells); total work stays bounded by the shared walk/eval + // budgets, not this cap. + int max_cone_cells = std::max(8192, max_width * max_width * 4); int max_leaf_bits = max_width * (max_width + 8); if (!get_cone(root.sig, cone_cells, leaf_bits, max_cone_cells, max_leaf_bits, &order)) continue; diff --git a/tests/silimate/opt_priority_onehot.ys b/tests/silimate/opt_priority_onehot.ys index a2e0726c2..a495de415 100644 --- a/tests/silimate/opt_priority_onehot.ys +++ b/tests/silimate/opt_priority_onehot.ys @@ -287,7 +287,7 @@ select -assert-count 1 t:$mux design -reset log -pop -log -header "Max-width below lane count leaves design unchanged" +log -header "Max-width below min lane count leaves design unchanged" log -push design -reset verific -cfg veri_optimize_wide_selector 1 @@ -295,7 +295,12 @@ verific -cfg db_infer_wide_muxes_post_elaboration 0 read -sv opt_priority_onehot.sv verific -import pri_onehot_basic proc; opt_clean -opt_priority_onehot -max_width 8 +# max_width below the minimum lane count (min_width default 4) admits no +# region. Note the generalized matcher will otherwise legitimately rewrite +# any priority-onehot sub-region (e.g. the first 8 lanes of this 16-lane +# scan) whose lane count is within max_width, so the bound must be below +# min_width to leave the design fully unchanged. +opt_priority_onehot -max_width 3 select -assert-none w:*prionehot* design -reset log -pop