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

Add muldiv_c and muxadd peepopts #4740

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
9943b18
Add `muldiv_c` and `muxadd` peepopts
akashlevy Nov 13, 2024
9f84a92
Update passes/pmgen/peepopt_muxadd.pmg
akashlevy Nov 17, 2024
a3cc228
Update passes/pmgen/peepopt_muxadd.pmg
akashlevy Nov 17, 2024
ee18e1f
Preliminary fixes, not done
akashlevy Nov 17, 2024
0ddb3d7
Merge branch 'YosysHQ:main' into new_peepopts
akashlevy Dec 17, 2024
2375879
Update peepopt_muldiv_c.pmg
akashlevy Dec 17, 2024
0e15edd
Add muxadd peepopt tests
povik Dec 17, 2024
3b73888
Compiles and transforms correctly, fails equiv
Dec 17, 2024
8a6c100
Clean after opt
Dec 17, 2024
cfadf28
Merge pull request #1 from alainmarcel/new_peepopts
akashlevy Dec 17, 2024
b4fa7dc
Update peepopt_muxadd.pmg
akashlevy Dec 17, 2024
f9ae66d
Update peepopt_muldiv_c.pmg
akashlevy Dec 17, 2024
2979232
Fix code review issue
Dec 18, 2024
6c5d7cc
Merge pull request #2 from alainmarcel/new_peepopts
akashlevy Dec 18, 2024
5212ad7
Passing equiv for simplest muxadd case, prevent multiple match/rewiri…
Dec 18, 2024
8e78720
peepopt_muldiv_c: add test
widlarizer Dec 18, 2024
d81bda8
Merge pull request #3 from alainmarcel/new_peepopts
akashlevy Dec 18, 2024
ff5237f
Switch formal proof to use miter/sat
Dec 18, 2024
029c255
Merge pull request #4 from alainmarcel/new_peepopts
akashlevy Dec 18, 2024
7b70ba4
Convert to miter/sat
Dec 18, 2024
ceb5d2a
Merge pull request #5 from alainmarcel/new_peepopts
alaindargelas Dec 18, 2024
0f8356d
Code review fix, bail out on integers above 64 bits
Dec 18, 2024
50c93f7
Merge pull request #6 from alainmarcel/new_peepopts
alaindargelas Dec 18, 2024
53cf38c
Leave comment about signage assumption
Dec 18, 2024
d98b224
Merge pull request #7 from alainmarcel/new_peepopts
alaindargelas Dec 18, 2024
1c774f4
peepopt_muldiv_c: remove write_verilog from test
widlarizer Dec 18, 2024
6db406f
Back to equiv_opt for multdiv tests
Dec 19, 2024
24c012a
Back to equiv_opt for multdiv tests
Dec 19, 2024
32406ea
Merge pull request #8 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
cd9e45d
Check for overflow, remove obsolete code, fix test
Dec 19, 2024
1ad1a0a
Merge pull request #9 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
fc47616
Remove redundant code
Dec 19, 2024
eb4e147
Merge branch 'new_peepopts' of github.com:alainmarcel/yosys_akash int…
Dec 19, 2024
7270cd3
Merge pull request #10 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
45500f4
A or B width extend
Dec 19, 2024
bb49acd
Merge pull request #11 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
325b0e3
peepopt_multdiv_c: add forgotten -assert
widlarizer Dec 19, 2024
e8e806f
Consolidate tests
Dec 19, 2024
b63ef81
Merge pull request #12 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
b525a02
Move helper code to peepopt.cc, check offset
Dec 19, 2024
5b97274
Merge pull request #13 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
8b08f81
compress constant ratio
Dec 19, 2024
8687e5f
compress constant ratio
Dec 19, 2024
43f4181
Merge pull request #14 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
72aef58
muxadd pass basic equiv_opt
Dec 19, 2024
a1db286
Merge pull request #15 from alainmarcel/new_peepopts
alaindargelas Dec 19, 2024
456773e
log test header for debug
Dec 20, 2024
87b8419
Merge pull request #16 from alainmarcel/new_peepopts
alaindargelas Dec 20, 2024
4d984d5
Fix symmetry test
Dec 20, 2024
a2bc9b2
Merge pull request #17 from alainmarcel/new_peepopts
alaindargelas Dec 20, 2024
4a28e0d
Pass all muldiv tests
Dec 20, 2024
a7fcda9
Merge branch 'new_peepopts' of github.com:alainmarcel/yosys_akash int…
Dec 20, 2024
48a971c
Merge pull request #18 from alainmarcel/new_peepopts
alaindargelas Dec 20, 2024
b6d079b
Overflow test fix
Dec 20, 2024
b1930e9
Overflow test fix
Dec 20, 2024
a497be5
Merge pull request #19 from alainmarcel/new_peepopts
alaindargelas Dec 20, 2024
7ce0460
All tests pass
Dec 20, 2024
095bf92
All tests pass
Dec 20, 2024
cd503d4
All tests pass
Dec 20, 2024
a731570
Merge pull request #20 from alainmarcel/new_peepopts
alaindargelas Dec 20, 2024
e0534a2
Merge remote-tracking branch 'upstream/main' into new_peepopts
akashlevy Dec 20, 2024
39ff442
Small muxadd comments
akashlevy Dec 20, 2024
832c877
Smallfixes
akashlevy Dec 20, 2024
b9c3666
GetSize, fix int sizing thing, add .gitignore
akashlevy Dec 20, 2024
11e4446
Fix nanoxplore meminit test
Dec 21, 2024
9450dc1
Merge pull request #21 from alainmarcel/new_peepopts
alaindargelas Dec 21, 2024
41a7c72
muxadd breaks Quicklogic dsp inference, make it optional
Dec 21, 2024
0b32e09
Merge pull request #22 from alainmarcel/new_peepopts
alaindargelas Dec 21, 2024
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
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,7 @@ SH_TEST_DIRS += tests/bram
SH_TEST_DIRS += tests/svinterfaces
SH_TEST_DIRS += tests/xprop
SH_TEST_DIRS += tests/select
SH_TEST_DIRS += tests/peepopt
SH_TEST_DIRS += tests/proc
SH_TEST_DIRS += tests/blif
SH_TEST_DIRS += tests/arch
Expand Down
2 changes: 2 additions & 0 deletions passes/pmgen/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ PEEPOPT_PATTERN = passes/pmgen/peepopt_shiftmul_right.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_shiftmul_left.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_shiftadd.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_muldiv.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_muldiv_c.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_muxadd.pmg
PEEPOPT_PATTERN += passes/pmgen/peepopt_formal_clockgateff.pmg

passes/pmgen/peepopt_pm.h: passes/pmgen/pmgen.py $(PEEPOPT_PATTERN)
Expand Down
27 changes: 26 additions & 1 deletion passes/pmgen/peepopt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ bool did_something;
// scratchpad configurations for pmgen
int shiftadd_max_ratio;

// Helper function, removes LSB 0s
SigSpec remove_bottom_padding(SigSpec sig);

#include "passes/pmgen/peepopt_pm.h"

struct PeepoptPass : public Pass {
Expand All @@ -44,6 +47,8 @@ struct PeepoptPass : public Pass {
log("\n");
log(" * muldiv - Replace (A*B)/B with A\n");
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(" * 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");
Expand All @@ -63,20 +68,28 @@ struct PeepoptPass : public Pass {
log(" based pattern to prevent combinational paths from the\n");
log(" output to the enable input after running clk2fflogic.\n");
log("\n");
log("If -withmuxadd is specified it adds the following rule:\n");
log("\n");
log(" * muxadd - Replace S?(A+B):A with A+(S?B:0)\n");
log("\n");
}
void execute(std::vector<std::string> args, RTLIL::Design *design) override
{
log_header(design, "Executing PEEPOPT pass (run peephole optimizers).\n");

bool formalclk = false;

bool withmuxadd = false;
size_t argidx;
for (argidx = 1; argidx < args.size(); argidx++)
{
if (args[argidx] == "-formalclk") {
formalclk = true;
continue;
}
if (args[argidx] == "-withmuxadd") {
withmuxadd = true;
continue;
}
break;
}
extra_args(args, argidx, design);
Expand Down Expand Up @@ -105,10 +118,22 @@ struct PeepoptPass : public Pass {
pm.run_shiftmul_right();
pm.run_shiftmul_left();
pm.run_muldiv();
pm.run_muldiv_c();
if (withmuxadd)
pm.run_muxadd();
}
}
}
}
} PeepoptPass;


SigSpec remove_bottom_padding(SigSpec sig)
{
int i = 0;
for (; i < sig.size() - 1 && sig[i] == State::S0; i++) {
}
return sig.extract(i, sig.size() - i);
}

PRIVATE_NAMESPACE_END
125 changes: 125 additions & 0 deletions passes/pmgen/peepopt_muldiv_c.pmg
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
pattern muldiv_c
//
// Authored by Akash Levy and Alain Dargelas of Silimate, Inc. under ISC license.
// Transforms mul->div into const->mul when b and c are divisible constants:
// y = (a * b_const) / c_const ===> a * eval(b_const / c_const)
//

state <SigSpec> a b_const mul_y

match mul
// Select multiplier
select mul->type == $mul
endmatch

code a b_const mul_y
// Get multiplier signals
a = port(mul, \A);
b_const = port(mul, \B);
mul_y = port(mul, \Y);

// Fanout of each multiplier Y bit should be 1 (no bit-split)
if (nusers(mul_y) != 2)
reject;

// A and B can be interchanged
branch;
std::swap(a, b_const);
endcode

match div
povik marked this conversation as resolved.
Show resolved Hide resolved
// Select div of form (a * b_const) / c_const
select div->type == $div

// Check that b_const and c_const is constant
filter b_const.is_fully_const()
filter port(div, \B).is_fully_const()
index <SigSpec> remove_bottom_padding(port(div, \A)) === mul_y
endmatch

code
// Get div signals
SigSpec div_a = port(div, \A);
SigSpec c_const = port(div, \B);
SigSpec div_y = port(div, \Y);

// Get offset of multiplier result chunk in divider
int offset = GetSize(div_a) - GetSize(mul_y);

// Get properties and values of b_const and c_const
// b_const may be coming from the A port
// But it is an RTLIL invariant that A_SIGNED equals B_SIGNED
bool b_const_signed = mul->getParam(ID::B_SIGNED).as_bool();
povik marked this conversation as resolved.
Show resolved Hide resolved
bool c_const_signed = div->getParam(ID::B_SIGNED).as_bool();
int b_const_int = b_const.as_int(b_const_signed);
int c_const_int = c_const.as_int(c_const_signed);
int b_const_int_shifted = b_const_int << offset;
Copy link
Member

Choose a reason for hiding this comment

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

The as_int() conversion may overflow if the operands are too wide. We should catch this case and bail

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

I think b_const_int << offset can still overflow

Choose a reason for hiding this comment

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

Not sure how to proceed, a test specifically designed for this condition will help.

Copy link
Member

Choose a reason for hiding this comment

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

As long as you test b_const_int.size() + offset <= 31 you should be fine

Copy link
Member

Choose a reason for hiding this comment

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

Err, b_const.size() + offset


// Helper lambdas for two's complement math
auto sign2sComplement = [](auto value, int numBits) {
if (value & (1 << (numBits - 1))) {
return -1;
} else {
return 1;
}
};
auto twosComplement = [](auto value, int numBits) {
if (value & (1 << (numBits - 1))) {
return (~value) + 1; // invert bits before adding 1
} else {
return value;
}
};

// Two's complement conversion
if (b_const_signed)
b_const_int = sign2sComplement(b_const_int, GetSize(b_const)) * twosComplement(b_const_int, GetSize(b_const));
if (c_const_signed)
c_const_int = sign2sComplement(c_const_int, GetSize(c_const)) * twosComplement(c_const_int, GetSize(c_const));
// Calculate the constant and compress the width to fit the value
Const const_ratio;
Const b_const_actual;
// Avoid division by zero
if (c_const_int == 0)
reject;
b_const_actual = b_const_int_shifted;
b_const_actual.compress(b_const_signed);

const_ratio = b_const_int_shifted / c_const_int;
const_ratio.compress(b_const_signed | c_const_signed);

// Integer values should be lesser than 32 bits
// This is because we are using C++ types, and int is 32 bits
// FIXME: use long long or BigInteger to make pass work with >32 bits
if (GetSize(mul->getParam(ID::B_WIDTH)) > 32)
reject;
if (GetSize(b_const) > 32)
reject;
Copy link
Member

Choose a reason for hiding this comment

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

I keep forgetting this but I think the typical size of int (and the standard mandated minimum) is 32 bits, also let's check b_const.size() + offset

Choose a reason for hiding this comment

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

Done here: akashlevy#13

if (GetSize(c_const) + offset > 32)
reject;

// Check for potential multiplier overflow
if (GetSize(b_const_actual) + GetSize(a) > GetSize(mul_y))
reject;

// Check that there are only zeros before offset
if (offset < 0 || !div_a.extract(0, offset).is_fully_zero())
reject;

// Check that b is divisible by c
if (b_const_int_shifted % c_const_int != 0)
reject;

// Rewire to only keep multiplier
povik marked this conversation as resolved.
Show resolved Hide resolved
mul->setPort(\A, a);
mul->setPort(\B, const_ratio);
mul->setPort(\Y, div_y);

// Remove divider
autoremove(div);

// Log, fixup, accept
log("muldiv_const pattern in %s: mul=%s, div=%s\n", log_id(module), log_id(mul), log_id(div));
mul->fixup_parameters();
accept;
endcode
123 changes: 123 additions & 0 deletions passes/pmgen/peepopt_muxadd.pmg
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
pattern muxadd
//
// Authored by Akash Levy and Alain Dargelas of Silimate, Inc. under ISC license.
// Transforms add->mux into mux->add:
// y = s ? (a + b) : a ===> y = a + (s ? b : 0)
// or
// y = s ? a : (a + b) ===> y = a + (s ? 0 : b)

state <SigSpec> add_a add_b add_y add_a_ext mux_a mux_b mux_y
state <Const> add_a_signed
state <IdString> add_a_id add_b_id mux_a_id mux_b_id

match add
// Select adder
select add->type == $add
povik marked this conversation as resolved.
Show resolved Hide resolved

// Set ports, allowing A and B to be swapped
choice <IdString> A {\A, \B}
define <IdString> B (A == \A ? \B : \A)
set add_a port(add, A)
set add_b port(add, B)
set add_y port(add, \Y)

// Get signedness
set add_a_signed param(add, (A == \A) ? \A_SIGNED : \B_SIGNED)

// Choice ids
set add_a_id A
set add_b_id B
endmatch

code add_y add_a add_b add_a_ext
// Get adder signals
add_a_ext = SigSpec(port(add, add_a_id));
add_a_ext.extend_u0(GetSize(add_y), add_a_signed.as_bool());

// Fanout of each adder Y bit should be 1 (no bit-split)
if (nusers(add_y) != 2)
reject;
endcode

match mux
// Select mux of form: s ? (a + b) : a
// Allow leading 0s when A_WIDTH != Y_WIDTH or s ? a : (a + b)
select mux->type == $mux
choice <IdString> AB {\A, \B}
define <IdString> BA (AB == \A ? \B : \A)
set mux_y port(mux, \Y)
set mux_a port(mux, AB)
set mux_b port(mux, BA)
set mux_a_id AB
set mux_b_id BA
index <SigSpec> port(mux, AB) === add_a_ext
index <SigSpec> port(mux, BA) === add_y
endmatch

code add_y add_a add_b add_a_ext add_a_id add_b_id mux_y mux_a mux_b mux_a_id mux_b_id
// Get mux signal
SigSpec mid;
std::string adder_y_name;
if (add_y.is_wire())
adder_y_name = add_y.as_wire()->name.c_str();
else
adder_y_name = add_y.as_string();

// Start by renaming the LHS of an eventual assign statement
// where the RHS is the adder output (that is getting rewired).
// Renaming the signal allows equiv_opt to function as it would
// otherwise try to match the functionality which would fail
// as the LHS signal has indeed changed function.

// Adder output could be assigned
for (auto it = module->connections().begin(); it != module->connections().end(); ++it) {
RTLIL::SigSpec rhs = it->second;
if (rhs.is_wire()) {
const std::string& rhs_name = rhs.as_wire()->name.c_str();
if (rhs_name == adder_y_name) {
RTLIL::SigSpec lhs = it->first;
if (lhs.is_wire()) {
const std::string& lhs_name = lhs.as_wire()->name.c_str();
module->rename(lhs_name, module->uniquify("$" + lhs_name));
break;
}
}
}
}
// Alternatively, the port name could be a wire name
if (add_y.is_wire()) {
if (GetSize(adder_y_name)) {
if (adder_y_name[0] != '$') {
module->rename(adder_y_name, module->uniquify("$" + adder_y_name));
}
}
} else {
for (auto chunk : add_y.chunks()) {
if (chunk.is_wire()) {
const std::string& name = chunk.wire->name.c_str();
if (name[0] != '$') {
module->rename(name, module->uniquify("$" + name));
}
}
}
}

// Create new mid wire
mid = module->addWire(NEW_ID, GetSize(add_b));

// Connect ports
add->setPort(add_b_id, mid);
add->setPort(add_a_id, add_a);
add->setPort(\Y, add_y);
mux->setPort(mux_a_id, Const(State::S0, GetSize(add_b)));
mux->setPort(mux_b_id, add_b);
mux->setPort(\Y, mid);
module->connect(mux_y, add_y);

// Log, fixup, accept
log("muxadd pattern in %s: mux=%s, add=%s\n", log_id(module), log_id(mux), log_id(add));
add->fixup_parameters();
mux->fixup_parameters();
did_something = true;
accept;
endcode
1 change: 1 addition & 0 deletions tests/peepopt/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/*.log
Loading
Loading