From b4e94d9f136c8ea81296e134b66479f0701a8750 Mon Sep 17 00:00:00 2001 From: Akash Levy Date: Wed, 20 May 2026 01:25:28 -0700 Subject: [PATCH] modshr onehot pass --- passes/opt/Makefile.inc | 1 + passes/opt/peepopt.cc | 18 +- passes/opt/peepopt_modshr_onehot.pmg | 123 ++++++++++ tests/peepopt/modshr_onehot.ys | 338 +++++++++++++++++++++++++++ 4 files changed, 473 insertions(+), 7 deletions(-) create mode 100644 passes/opt/peepopt_modshr_onehot.pmg create mode 100644 tests/peepopt/modshr_onehot.ys diff --git a/passes/opt/Makefile.inc b/passes/opt/Makefile.inc index 53c123b79..461412422 100644 --- a/passes/opt/Makefile.inc +++ b/passes/opt/Makefile.inc @@ -34,6 +34,7 @@ PEEPOPT_PATTERN += passes/opt/peepopt_shiftmul_left.pmg PEEPOPT_PATTERN += passes/opt/peepopt_shiftadd.pmg PEEPOPT_PATTERN += passes/opt/peepopt_muldiv.pmg PEEPOPT_PATTERN += passes/opt/peepopt_muldiv_c.pmg +PEEPOPT_PATTERN += passes/opt/peepopt_modshr_onehot.pmg PEEPOPT_PATTERN += passes/opt/peepopt_addsub_c.pmg PEEPOPT_PATTERN += passes/opt/peepopt_muxorder.pmg PEEPOPT_PATTERN += passes/opt/peepopt_formal_clockgateff.pmg diff --git a/passes/opt/peepopt.cc b/passes/opt/peepopt.cc index 1ef160191..6d2df1287 100644 --- a/passes/opt/peepopt.cc +++ b/passes/opt/peepopt.cc @@ -57,6 +57,9 @@ struct PeepoptPass : public Pass { log("\n"); log(" * muldiv_c - Replace (A*B)/C with A*(B/C) when C is a const divisible by B.\n"); log("\n"); + log(" * modshr_onehot - Replace a%%(mbase>>shiftamt) with a&((mbase-1)>>shiftamt)\n"); + log(" when mbase is a constant one-hot signal.\n"); + log("\n"); log(" * shiftmul - Replace A>>(B*C) with A'>>(B<> shiftamt) ===> y = a & ((mbase - 1) >> shiftamt) +// +// where `mbase` is a fully-constant one-hot signal. This eliminates a +// variable divider, replacing it with a single constant-A right shift +// and a bitwise AND. +// +// Correctness: +// Because `mbase = 1 << B` for some constant bit index B, +// `mbase >> shiftamt` is always either a power of two or zero. +// For unsigned operands, `a % 2^k = a & (2^k - 1)`, and +// `(mbase - 1) >> shiftamt` is exactly the lower-k-bits mask. +// When `shiftamt > B` the original `a % 0` is undefined (Yosys +// `const_mod` returns Sx); the rewrite yields `a & 0 = 0`, which is +// a legal refinement of Sx. +// +// We match $mod first and then look for a $shr/$shiftx whose Y's +// lower bits drive mod->B. This is critical because Verilog often +// infers the shifter as 32 bits wide (signed integer parameters) and +// only the lower bits actually feed the modulo. +// + +state mod_b +state mod_b_width +state mbase_const +state onehot_pos + +match mod + select mod->type.in($mod, $modfloor) + // Signed modulo has different semantics for negative dividends; + // the bitmask identity holds only for unsigned a. + filter !param(mod, \A_SIGNED).as_bool() +endmatch + +code mod_b mod_b_width + mod_b = port(mod, \B); + mod_b_width = GetSize(mod_b); +endcode + +match shift + // $shr / $shiftx perform logical operations regardless of A_SIGNED, + // so we accept any A_SIGNED. $sshr is excluded because with + // A_SIGNED=1 and a one-hot at the MSB it would sign-extend with + // 1s, breaking the power-of-two assumption. + select shift->type.in($shr, $shiftx) + // B_SIGNED must be false so shiftamt is non-negative; for $shiftx + // in particular a negative B would shift left, invalidating the + // rewrite. + filter !param(shift, \B_SIGNED).as_bool() + // A must be a constant one-hot signal. + filter port(shift, \A).is_fully_const() + filter port(shift, \A).is_onehot() + // shift->Y must be at least as wide as mod->B (we look at its low + // bits only), and the lower `mod_b_width` bits of Y must exactly + // equal mod->B. The shifter output is allowed to fan out to many + // $mod cells (each one is rewritten independently); we do not + // constrain nusers here. + filter GetSize(port(shift, \Y)) >= mod_b_width + filter port(shift, \Y).extract(0, mod_b_width) == mod_b +endmatch + +code mbase_const onehot_pos + mbase_const = port(shift, \A).as_const(); + mbase_const.is_onehot(&onehot_pos); +endcode + +code +{ + // If mod sees B as signed, the divisor bit pattern must encode a + // non-negative value; reject when the one-hot bit sits at the MSB + // of mbase (which would make it negative). + if (param(mod, \B_SIGNED).as_bool() && onehot_pos == GetSize(mbase_const) - 1) + reject; + + // Build mask_const = mbase - 1: bits [onehot_pos-1:0] = 1, rest = 0. + Const mask_const(State::S0, GetSize(mbase_const)); + for (int i = 0; i < onehot_pos; i++) + mask_const.set(i, State::S1); + + // `cell` is read by NEW_ID2_SUFFIX (kernel/yosys_common.h:321); + // use the soon-to-be-converted $mod as the naming parent so the + // new mask wire and shifter inherit a stable, debuggable name. + Cell *cell = mod; + + // New shifter producing shifted_mask = mask_const >> shiftamt, + // of the same type as the original shift but width-trimmed to + // match mod->B. + Wire *shifted_mask = module->addWire(NEW_ID2_SUFFIX("mask"), mod_b_width); + Cell *new_shift = module->addCell(NEW_ID2_SUFFIX("maskshift"), shift->type); + new_shift->setPort(\A, mask_const); + new_shift->setPort(\B, port(shift, \B)); + new_shift->setPort(\Y, shifted_mask); + new_shift->setParam(\A_SIGNED, false); + new_shift->setParam(\B_SIGNED, false); + new_shift->fixup_parameters(); + new_shift->set_src_attribute(mod->get_src_attribute()); + + // Convert $mod -> $and in place: keep A and Y, retarget B to the + // new shifted_mask wire, and refresh widths. + mod->type = $and; + mod->setPort(\B, shifted_mask); + mod->setParam(\B_SIGNED, false); + mod->fixup_parameters(); + module->rename(mod, NEW_ID2_SUFFIX("modmask")); + + // Do NOT autoremove(shift): a single shifter often fans out to + // many $mod cells (e.g. inside a generate loop), and autoremove + // blacklists the cell, preventing further matches for siblings. + // Leave shift in place -- opt_clean will prune it later if all + // of its Y bits have become dead. + + log("modshr_onehot pattern in %s: shift=%s, mod=%s\n", + log_id(module), log_id(shift), log_id(mod)); + did_something = true; + accept; +} +endcode diff --git a/tests/peepopt/modshr_onehot.ys b/tests/peepopt/modshr_onehot.ys new file mode 100644 index 000000000..513cb04c6 --- /dev/null +++ b/tests/peepopt/modshr_onehot.ys @@ -0,0 +1,338 @@ +log -header "Positive: mbase MSB one-hot (8'b1000_0000), shiftamt 3 bits" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: mbase non-MSB one-hot (8'b0010_0000)" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: shiftamt clog2(N) bits, divisor never zero (mbase = 1<<(N-1))" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: low-position one-hot, shiftamt bounded so divisor != 0" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: wider mbase than a, shiftamt sized so divisor != 0" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: \$shr A_SIGNED=1 (signed parameter), Y wider than mod->B" +log -push +design -reset +read_verilog -sv <> shiftamt) in {32,16,8,4} -- never zero. + parameter FIFO_DEPTH = 32; + wire [5:0] divisor = FIFO_DEPTH >> shiftamt; + assign y = a % divisor; +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Positive: shared shifter Y fans out to many \$mod cells (generate loop)" +log -push +design -reset +read_verilog -sv <> shiftamt; + assign y0 = a0 % divisor; + assign y1 = a1 % divisor; + assign y2 = a2 % divisor; + assign y3 = a3 % divisor; +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 4 t:$and +# 4 mask shifters + 1 original (still drives the shared divisor wire +# whose remaining bits may be dead). opt_clean keeps the original +# while any of its Y bits has a name attached; check >= 4 shifters. +select -assert-min 4 t:$shr +design -reset +log -pop + + +log -header "Positive: rewrite still applied even when divisor can be 0; assert via peepopt only" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +peepopt +opt_clean +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 1 t:$shr +design -reset +log -pop + + +log -header "Negative: mbase is NOT one-hot (8'b0011_0000)" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +select -assert-count 1 t:$mod +select -assert-count 0 t:$and +design -reset +log -pop + + +log -header "Negative: mbase is a wire (not constant) -- pattern must not match" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +select -assert-count 1 t:$mod +design -reset +log -pop + + +log -header "Negative: signed dividend (A_SIGNED=1 on $mod)" +log -push +design -reset +read_verilog <> shiftamt); +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +select -assert-count 1 t:$mod +design -reset +log -pop + + +log -header "Negative: shifter is $shl (left shift) -- not the right pattern" +log -push +design -reset +read_verilog <> shiftamt; + assign y = a % divisor; + assign probe = divisor; +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +opt_clean +# The original $shr must be preserved because it drives \probe; the +# rewrite only retargets the $mod, replacing it with $and + a new mask +# shifter. +select -assert-count 0 t:$mod +select -assert-count 1 t:$and +select -assert-count 2 t:$shr +design -reset +log -pop + + +log -header "Negative: B_SIGNED=1 on $mod with one-hot at MSB of mbase" +log -push +design -reset +read_verilog <>> shiftamt; + assign y = a % divisor; +endmodule +EOT +check -assert +equiv_opt -assert peepopt +design -load postopt +select -assert-count 1 t:$mod +design -reset +log -pop