From a0c3be3aaeb2bbb87161432748aa349712e35dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Wed, 2 Aug 2023 13:52:40 +0200 Subject: [PATCH 1/8] peepopt: Drop unused 'initbits' code Drop code that was once used by the 'dffmux' pattern but now is unused after that pattern has been obsoleted by the 'opt_dff' pass. --- passes/pmgen/peepopt.cc | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/passes/pmgen/peepopt.cc b/passes/pmgen/peepopt.cc index 9a497c91441..1d27d77a0f2 100644 --- a/passes/pmgen/peepopt.cc +++ b/passes/pmgen/peepopt.cc @@ -24,8 +24,6 @@ USING_YOSYS_NAMESPACE PRIVATE_NAMESPACE_BEGIN bool did_something; -dict initbits; -pool rminitbits; #include "passes/pmgen/peepopt_pm.h" #include "generate.h" @@ -60,9 +58,6 @@ struct PeepoptPass : public Pass { if (!genmode.empty()) { - initbits.clear(); - rminitbits.clear(); - if (genmode == "shiftmul") GENERATE_PATTERN(peepopt_pm, shiftmul); else if (genmode == "muldiv") @@ -79,47 +74,13 @@ struct PeepoptPass : public Pass { while (did_something) { did_something = false; - initbits.clear(); - rminitbits.clear(); peepopt_pm pm(module); - for (auto w : module->wires()) { - auto it = w->attributes.find(ID::init); - if (it != w->attributes.end()) { - SigSpec sig = pm.sigmap(w); - Const val = it->second; - int len = std::min(GetSize(sig), GetSize(val)); - for (int i = 0; i < len; i++) { - if (sig[i].wire == nullptr) - continue; - if (val[i] != State::S0 && val[i] != State::S1) - continue; - initbits[sig[i]] = val[i]; - } - } - } - pm.setup(module->selected_cells()); pm.run_shiftmul(); pm.run_muldiv(); - - for (auto w : module->wires()) { - auto it = w->attributes.find(ID::init); - if (it != w->attributes.end()) { - SigSpec sig = pm.sigmap(w); - Const &val = it->second; - int len = std::min(GetSize(sig), GetSize(val)); - for (int i = 0; i < len; i++) { - if (rminitbits.count(sig[i])) - val[i] = State::Sx; - } - } - } - - initbits.clear(); - rminitbits.clear(); } } } From bd8a81a907f0307254387a2b74aa1d1ebc582505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Wed, 2 Aug 2023 15:44:41 +0200 Subject: [PATCH 2/8] peepopt: Clean up 'shiftmul' a bit No functional change intended. --- passes/pmgen/peepopt_shiftmul.pmg | 66 +++++++++++++++++-------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/passes/pmgen/peepopt_shiftmul.pmg b/passes/pmgen/peepopt_shiftmul.pmg index d71fbf74449..92e902d3ea6 100644 --- a/passes/pmgen/peepopt_shiftmul.pmg +++ b/passes/pmgen/peepopt_shiftmul.pmg @@ -3,61 +3,67 @@ pattern shiftmul // Optimize mul+shift pairs that result from expressions such as foo[s*W+:W] // -state shamt - match shift select shift->type.in($shift, $shiftx, $shr) + filter !port(shift, \B).empty() endmatch -code shamt - shamt = port(shift, \B); - if (shamt.empty()) - reject; - if (shamt[GetSize(shamt)-1] == State::S0) { - do { - shamt.remove(GetSize(shamt)-1); - if (shamt.empty()) - reject; - } while (shamt[GetSize(shamt)-1] == State::S0); - } else - if (shift->type.in($shift, $shiftx) && param(shift, \B_SIGNED).as_bool()) { +state shift_amount + +code shift_amount + shift_amount = port(shift, \B); + if (shift->type.in($shr) || !param(shift, \B_SIGNED).as_bool()) + shift_amount.append(State::S0); + + // at this point shift_amount is signed, make + // sure we can never go negative + if (shift_amount.bits().back() != State::S0) reject; + + while (shift_amount.bits().back() == State::S0) { + shift_amount.remove(GetSize(shift_amount) - 1); + if (shift_amount.empty()) reject; } - if (GetSize(shamt) > 20) + + if (GetSize(shift_amount) > 20) reject; endcode +state mul_din +state mul_const + match mul select mul->type.in($mul) - select port(mul, \A).is_fully_const() || port(mul, \B).is_fully_const() - index port(mul, \Y) === shamt + index port(mul, \Y) === shift_amount filter !param(mul, \A_SIGNED).as_bool() + + choice constport {\A, \B} + filter port(mul, constport).is_fully_const() + + define varport (constport == \A ? \B : \A) + set mul_const port(mul, constport).as_const() + set mul_din port(mul, varport) endmatch code { - IdString const_factor_port = port(mul, \A).is_fully_const() ? \A : \B; - Const const_factor_cnst = port(mul, const_factor_port).as_const(); - int const_factor = const_factor_cnst.as_int(); - - if (GetSize(const_factor_cnst) == 0) + if (mul_const.empty() || GetSize(mul_const) > 20) reject; - if (GetSize(const_factor_cnst) > 20) + // make sure there's no overlap in the signal + // selections by the shiftmul pattern + if (GetSize(port(shift, \Y)) > mul_const.as_int()) reject; - if (GetSize(port(shift, \Y)) > const_factor) - reject; - - int factor_bits = ceil_log2(const_factor); - SigSpec mul_din = port(mul, const_factor_port == \A ? \B : \A); - - if (GetSize(shamt) < factor_bits+GetSize(mul_din)) + int factor_bits = ceil_log2(mul_const.as_int()); + // make sure the multiplication never wraps around + if (GetSize(shift_amount) < factor_bits + GetSize(mul_din)) reject; did_something = true; log("shiftmul pattern in %s: shift=%s, mul=%s\n", log_id(module), log_id(shift), log_id(mul)); + int const_factor = mul_const.as_int(); int new_const_factor = 1 << factor_bits; SigSpec padding(State::Sx, new_const_factor-const_factor); SigSpec old_a = port(shift, \A), new_a; From dd1a8ae49a54b00407afef92f5dac1b36e7efefa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Wed, 2 Aug 2023 15:46:50 +0200 Subject: [PATCH 3/8] peepopt: Try to use original wires --- passes/pmgen/peepopt_shiftmul.pmg | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/passes/pmgen/peepopt_shiftmul.pmg b/passes/pmgen/peepopt_shiftmul.pmg index 92e902d3ea6..177d97371b4 100644 --- a/passes/pmgen/peepopt_shiftmul.pmg +++ b/passes/pmgen/peepopt_shiftmul.pmg @@ -42,7 +42,11 @@ match mul define varport (constport == \A ? \B : \A) set mul_const port(mul, constport).as_const() - set mul_din port(mul, varport) + // get mul_din unmapped (so no `port()` shorthand) + // because we will be using it to set the \A port + // on the shift cell, and we want to stay close + // to the original design + set mul_din mul->getPort(varport) endmatch code From 038a5e1ed4d31a1ad1fde63971939abd8885fe2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Wed, 2 Aug 2023 16:10:27 +0200 Subject: [PATCH 4/8] peepopt: Support shift amounts zero-padded from below The `opt_expr` pass running before `peepopt` can interfere with the detection of a shiftmul pattern due to some of the bottom bits of the shift amount being replaced with constant zero. Extend the detection to cover those situations as well. --- passes/pmgen/peepopt_shiftmul.pmg | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/passes/pmgen/peepopt_shiftmul.pmg b/passes/pmgen/peepopt_shiftmul.pmg index 177d97371b4..4808430b4d3 100644 --- a/passes/pmgen/peepopt_shiftmul.pmg +++ b/passes/pmgen/peepopt_shiftmul.pmg @@ -8,9 +8,13 @@ match shift filter !port(shift, \B).empty() endmatch +// the right shift amount state shift_amount +// log2 scale factor in interpreting of shift_amount +// due to zero padding on the shift cell's B port +state log2scale -code shift_amount +code shift_amount log2scale shift_amount = port(shift, \B); if (shift->type.in($shr) || !param(shift, \B_SIGNED).as_bool()) shift_amount.append(State::S0); @@ -25,6 +29,13 @@ code shift_amount if (shift_amount.empty()) reject; } + log2scale = 0; + while (shift_amount[0] == State::S0) { + shift_amount.remove(0); + if (shift_amount.empty()) reject; + log2scale++; + } + if (GetSize(shift_amount) > 20) reject; endcode @@ -41,7 +52,7 @@ match mul filter port(mul, constport).is_fully_const() define varport (constport == \A ? \B : \A) - set mul_const port(mul, constport).as_const() + set mul_const SigSpec({port(mul, constport), SigSpec(State::S0, log2scale)}).as_const() // get mul_din unmapped (so no `port()` shorthand) // because we will be using it to set the \A port // on the shift cell, and we want to stay close @@ -61,7 +72,7 @@ code int factor_bits = ceil_log2(mul_const.as_int()); // make sure the multiplication never wraps around - if (GetSize(shift_amount) < factor_bits + GetSize(mul_din)) + if (GetSize(shift_amount) + log2scale < factor_bits + GetSize(mul_din)) reject; did_something = true; From aa9b86aeec1b1b2ea2d3344098963300b4f624f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Wed, 2 Aug 2023 18:04:54 +0200 Subject: [PATCH 5/8] peepopt: Add left-shift 'shiftmul' variant Add a separate shiftmul pattern to match on left shifts which implement demuxing. This mirrors the right shift pattern matcher but is probably best kept separate instead of merging the two into a single matcher. In any case the diff of the two matchers should be easily readable. --- passes/pmgen/Makefile.inc | 3 +- passes/pmgen/peepopt.cc | 5 +- passes/pmgen/peepopt_shiftmul_left.pmg | 160 ++++++++++++++++++ ...hiftmul.pmg => peepopt_shiftmul_right.pmg} | 4 +- 4 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 passes/pmgen/peepopt_shiftmul_left.pmg rename passes/pmgen/{peepopt_shiftmul.pmg => peepopt_shiftmul_right.pmg} (95%) diff --git a/passes/pmgen/Makefile.inc b/passes/pmgen/Makefile.inc index a7ef642825c..c2257b72083 100644 --- a/passes/pmgen/Makefile.inc +++ b/passes/pmgen/Makefile.inc @@ -42,7 +42,8 @@ GENFILES += passes/pmgen/peepopt_pm.h passes/pmgen/peepopt.o: passes/pmgen/peepopt_pm.h $(eval $(call add_extra_objs,passes/pmgen/peepopt_pm.h)) -PEEPOPT_PATTERN = passes/pmgen/peepopt_shiftmul.pmg +PEEPOPT_PATTERN = passes/pmgen/peepopt_shiftmul_right.pmg +PEEPOPT_PATTERN += passes/pmgen/peepopt_shiftmul_left.pmg PEEPOPT_PATTERN += passes/pmgen/peepopt_muldiv.pmg passes/pmgen/peepopt_pm.h: passes/pmgen/pmgen.py $(PEEPOPT_PATTERN) diff --git a/passes/pmgen/peepopt.cc b/passes/pmgen/peepopt.cc index 1d27d77a0f2..2b225623dba 100644 --- a/passes/pmgen/peepopt.cc +++ b/passes/pmgen/peepopt.cc @@ -59,7 +59,7 @@ struct PeepoptPass : public Pass { if (!genmode.empty()) { if (genmode == "shiftmul") - GENERATE_PATTERN(peepopt_pm, shiftmul); + GENERATE_PATTERN(peepopt_pm, shiftmul_right); else if (genmode == "muldiv") GENERATE_PATTERN(peepopt_pm, muldiv); else @@ -79,7 +79,8 @@ struct PeepoptPass : public Pass { pm.setup(module->selected_cells()); - pm.run_shiftmul(); + pm.run_shiftmul_right(); + pm.run_shiftmul_left(); pm.run_muldiv(); } } diff --git a/passes/pmgen/peepopt_shiftmul_left.pmg b/passes/pmgen/peepopt_shiftmul_left.pmg new file mode 100644 index 00000000000..607f8368c53 --- /dev/null +++ b/passes/pmgen/peepopt_shiftmul_left.pmg @@ -0,0 +1,160 @@ +pattern shiftmul_left +// +// Optimize mul+shift pairs that result from expressions such as foo[s*W+:W] +// + +match shift + select shift->type.in($shift, $shiftx, $shl) + select shift->type.in($shl) || param(shift, \B_SIGNED).as_bool() + filter !port(shift, \B).empty() +endmatch + +match neg + if shift->type.in($shift, $shiftx) + select neg->type == $neg + index port(neg, \Y) === port(shift, \B) + filter !port(shift, \A).empty() +endmatch + +// the left shift amount +state shift_amount +// log2 scale factor in interpreting of shift_amount +// due to zero padding on the shift cell's B port +state log2scale + +code shift_amount log2scale + if (neg) { + // case of `$shift`, `$shiftx` + shift_amount = port(neg, \A); + if (!param(neg, \A_SIGNED).as_bool()) + shift_amount.append(State::S0); + } else { + // case of `$shl` + shift_amount = port(shift, \B); + if (!param(shift, \B_SIGNED).as_bool()) + shift_amount.append(State::S0); + } + + // at this point shift_amount is signed, make + // sure we can never go negative + if (shift_amount.bits().back() != State::S0) + reject; + + while (shift_amount.bits().back() == State::S0) { + shift_amount.remove(GetSize(shift_amount) - 1); + if (shift_amount.empty()) reject; + } + + log2scale = 0; + while (shift_amount[0] == State::S0) { + shift_amount.remove(0); + if (shift_amount.empty()) reject; + log2scale++; + } + + if (GetSize(shift_amount) > 20) + reject; +endcode + +state mul_din +state mul_const + +match mul + select mul->type.in($mul) + index port(mul, \Y) === shift_amount + filter !param(mul, \A_SIGNED).as_bool() + + choice constport {\A, \B} + filter port(mul, constport).is_fully_const() + + define varport (constport == \A ? \B : \A) + set mul_const SigSpec({port(mul, constport), SigSpec(State::S0, log2scale)}).as_const() + // get mul_din unmapped (so no `port()` shorthand) + // because we will be using it to set the \A port + // on the shift cell, and we want to stay close + // to the original design + set mul_din mul->getPort(varport) +endmatch + +code +{ + if (mul_const.empty() || GetSize(mul_const) > 20) + reject; + + // make sure there's no overlap in the signal + // selections by the shiftmul pattern + if (GetSize(port(shift, \A)) > mul_const.as_int()) + reject; + + int factor_bits = ceil_log2(mul_const.as_int()); + // make sure the multiplication never wraps around + if (GetSize(shift_amount) < factor_bits + GetSize(mul_din)) + reject; + + if (neg) { + // make sure the negation never wraps around + if (GetSize(port(shift, \B)) < factor_bits + GetSize(mul_din) + + log2scale + 1) + reject; + } + + did_something = true; + log("left shiftmul pattern in %s: shift=%s, mul=%s\n", log_id(module), log_id(shift), log_id(mul)); + + int const_factor = mul_const.as_int(); + int new_const_factor = 1 << factor_bits; + SigSpec padding(State::Sm, new_const_factor-const_factor); + SigSpec old_y = port(shift, \Y), new_y; + int trunc = 0; + + if (GetSize(old_y) % const_factor != 0) { + trunc = const_factor - GetSize(old_y) % const_factor; + old_y.append(SigSpec(State::Sm, trunc)); + } + + for (int i = 0; i*const_factor < GetSize(old_y); i++) { + SigSpec slice = old_y.extract(i*const_factor, const_factor); + new_y.append(slice); + new_y.append(padding); + } + + if (trunc > 0) + new_y.remove(GetSize(new_y)-trunc, trunc); + + { + // Now replace occurences of Sm in new_y with bits + // of a dummy wire + int padbits = 0; + for (auto bit : new_y) + if (bit == SigBit(State::Sm)) + padbits++; + + SigSpec padwire = module->addWire(NEW_ID, padbits); + + for (int i = new_y.size() - 1; i >= 0; i--) + if (new_y[i] == SigBit(State::Sm)) { + new_y[i] = padwire.bits().back(); + padwire.remove(padwire.size() - 1); + } + } + + SigSpec new_b = {mul_din, SigSpec(State::S0, factor_bits)}; + + shift->setPort(\Y, new_y); + shift->setParam(\Y_WIDTH, GetSize(new_y)); + if (shift->type == $shl) { + if (param(shift, \B_SIGNED).as_bool()) + new_b.append(State::S0); + shift->setPort(\B, new_b); + shift->setParam(\B_WIDTH, GetSize(new_b)); + } else { + SigSpec b_neg = module->addWire(NEW_ID, GetSize(new_b) + 1); + module->addNeg(NEW_ID, new_b, b_neg); + shift->setPort(\B, b_neg); + shift->setParam(\B_WIDTH, GetSize(b_neg)); + } + + blacklist(shift); + accept; +} +endcode diff --git a/passes/pmgen/peepopt_shiftmul.pmg b/passes/pmgen/peepopt_shiftmul_right.pmg similarity index 95% rename from passes/pmgen/peepopt_shiftmul.pmg rename to passes/pmgen/peepopt_shiftmul_right.pmg index 4808430b4d3..71e0980234a 100644 --- a/passes/pmgen/peepopt_shiftmul.pmg +++ b/passes/pmgen/peepopt_shiftmul_right.pmg @@ -1,4 +1,4 @@ -pattern shiftmul +pattern shiftmul_right // // Optimize mul+shift pairs that result from expressions such as foo[s*W+:W] // @@ -76,7 +76,7 @@ code reject; did_something = true; - log("shiftmul pattern in %s: shift=%s, mul=%s\n", log_id(module), log_id(shift), log_id(mul)); + log("right shiftmul pattern in %s: shift=%s, mul=%s\n", log_id(module), log_id(shift), log_id(mul)); int const_factor = mul_const.as_int(); int new_const_factor = 1 << factor_bits; From 5c0c8251c38f103aa63e5cf1e689c64803ba7e16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Mon, 16 Oct 2023 13:54:17 +0200 Subject: [PATCH 6/8] peepopt: Remove broken `-generate` option --- passes/pmgen/peepopt.cc | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/passes/pmgen/peepopt.cc b/passes/pmgen/peepopt.cc index 2b225623dba..f0349525472 100644 --- a/passes/pmgen/peepopt.cc +++ b/passes/pmgen/peepopt.cc @@ -26,7 +26,6 @@ PRIVATE_NAMESPACE_BEGIN bool did_something; #include "passes/pmgen/peepopt_pm.h" -#include "generate.h" struct PeepoptPass : public Pass { PeepoptPass() : Pass("peepopt", "collection of peephole optimizers") { } @@ -41,32 +40,15 @@ struct PeepoptPass : public Pass { } void execute(std::vector args, RTLIL::Design *design) override { - std::string genmode; - log_header(design, "Executing PEEPOPT pass (run peephole optimizers).\n"); size_t argidx; for (argidx = 1; argidx < args.size(); argidx++) { - if (args[argidx] == "-generate" && argidx+1 < args.size()) { - genmode = args[++argidx]; - continue; - } break; } extra_args(args, argidx, design); - if (!genmode.empty()) - { - if (genmode == "shiftmul") - GENERATE_PATTERN(peepopt_pm, shiftmul_right); - else if (genmode == "muldiv") - GENERATE_PATTERN(peepopt_pm, muldiv); - else - log_abort(); - return; - } - for (auto module : design->selected_modules()) { did_something = true; From 660be4a31e58f7d348fea9b3fe41553ac061f26a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Mon, 16 Oct 2023 14:08:42 +0200 Subject: [PATCH 7/8] peepopt: Describe rules in help message --- passes/pmgen/peepopt.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/passes/pmgen/peepopt.cc b/passes/pmgen/peepopt.cc index f0349525472..aef464d79c9 100644 --- a/passes/pmgen/peepopt.cc +++ b/passes/pmgen/peepopt.cc @@ -37,6 +37,17 @@ struct PeepoptPass : public Pass { log("\n"); log("This pass applies a collection of peephole optimizers to the current design.\n"); log("\n"); + log("This pass employs the following rules:\n"); + log("\n"); + log(" * muldiv - Replace (A*B)/B with A\n"); + log("\n"); + log(" * shiftmul - Replace A>>(B*C) with A'>>(B< args, RTLIL::Design *design) override { From d6d1cc705e524264361bfc16f7a2ef5a96b0b717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Mon, 16 Oct 2023 14:16:36 +0200 Subject: [PATCH 8/8] pmgen: Fix sample syntax --- passes/pmgen/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passes/pmgen/README.md b/passes/pmgen/README.md index 39560839f0f..3205be1b55e 100644 --- a/passes/pmgen/README.md +++ b/passes/pmgen/README.md @@ -212,7 +212,7 @@ second argument, and the matcher will iterate over those options: index port(eq, BA) === bar set eq_ab AB set eq_ba BA - generate + endmatch Notice how `define` can be used to define additional local variables similar to the loop variables defined by `slice` and `choice`.