Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend the peepopt shiftmul matcher #3873

Merged
merged 8 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion passes/pmgen/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion passes/pmgen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ second argument, and the matcher will iterate over those options:
index <SigSpec> 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`.
Expand Down
71 changes: 13 additions & 58 deletions passes/pmgen/peepopt.cc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this cleans up the removal of the dffmux pattern in a0e99a9
Originally added in b8774ae

The changes (removals) concerning init look fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, can add that to the commit message

Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@ USING_YOSYS_NAMESPACE
PRIVATE_NAMESPACE_BEGIN

bool did_something;
dict<SigBit, State> initbits;
pool<SigBit> rminitbits;

#include "passes/pmgen/peepopt_pm.h"
#include "generate.h"

struct PeepoptPass : public Pass {
PeepoptPass() : Pass("peepopt", "collection of peephole optimizers") { }
Expand All @@ -40,86 +37,44 @@ 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<<K) where C and K are constants\n");
log(" and A' is derived from A by appropriately inserting padding\n");
log(" into the signal. (right variant)\n");
log("\n");
log(" Analogously, replace A<<(B*C) with appropriate selection of\n");
log(" output bits from A<<(B<<K). (left variant)\n");
log("\n");
}
void execute(std::vector<std::string> 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())
{
initbits.clear();
rminitbits.clear();

if (genmode == "shiftmul")
GENERATE_PATTERN(peepopt_pm, shiftmul);
else if (genmode == "muldiv")
GENERATE_PATTERN(peepopt_pm, muldiv);
else
log_abort();
return;
}

for (auto module : design->selected_modules())
{
did_something = true;

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_shiftmul_right();
pm.run_shiftmul_left();
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();
}
}
}
Expand Down
92 changes: 0 additions & 92 deletions passes/pmgen/peepopt_shiftmul.pmg

This file was deleted.

160 changes: 160 additions & 0 deletions passes/pmgen/peepopt_shiftmul_left.pmg
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
match neg
match neg
select neg->type == $neg

Without this it tries to access port \Y on any cell, no matter the type, this gives an error as not all cells have a \Y port.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha! Of course

if shift->type.in($shift, $shiftx)
select neg->type == $neg
index <SigSpec> port(neg, \Y) === port(shift, \B)
filter !port(shift, \A).empty()
endmatch

// the left shift amount
state <SigSpec> shift_amount
// log2 scale factor in interpreting of shift_amount
// due to zero padding on the shift cell's B port
state <int> 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 <SigSpec> mul_din
state <Const> mul_const

match mul
select mul->type.in($mul)
index <SigSpec> port(mul, \Y) === shift_amount
filter !param(mul, \A_SIGNED).as_bool()

choice <IdString> constport {\A, \B}
filter port(mul, constport).is_fully_const()

define <IdString> 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
Loading
Loading