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 1 commit
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
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
6 changes: 6 additions & 0 deletions passes/pmgen/peepopt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,12 @@ struct PeepoptPass : public Pass {
log("\n");
log("This pass employs the following rules by default:\n");
log("\n");
log(" * muxadd - Replace S?(A+B):A with A+(S?B:0)\n");
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 Down Expand Up @@ -105,6 +109,8 @@ struct PeepoptPass : public Pass {
pm.run_shiftmul_right();
pm.run_shiftmul_left();
pm.run_muldiv();
pm.run_muldiv_c();
pm.run_muxadd();
}
}
}
Expand Down
76 changes: 76 additions & 0 deletions passes/pmgen/peepopt_muldiv_c.pmg
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
pattern muldiv_c
//
// Authored by Akash Levy 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)
for (auto bit : mul_y)
if (nusers(bit) != 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()
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
int b_const_width = mul->getParam(ID::B_WIDTH).as_int();
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


// 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(\B, Const(b_const_int_shifted / c_const_int, b_const_width));
Copy link
Member

Choose a reason for hiding this comment

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

Because b_const_int_shifted is shifted up, there's no guarantee it fits into b_const_width

Also this might need special care wrt signedness. B_SIGNED on the multiplier may be false but b_const_int_shifted / c_const_int can be negative due to the divider. I wouldn't be opposed to requiring matching signedness on the divider and multiplier to make the transformation a little easier to reason about

Choose a reason for hiding this comment

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

@povik should I change to: mul->setPort(\B, Const(b_const_int_shifted / c_const_int, b_const_width + offset));

Copy link
Member

Choose a reason for hiding this comment

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

That would work but I suggest constructing the Const with 32 bits (the constructor default) and then calling compress(/signedness/) to fit to the constant. The reason for this is: unless wreduce is sequenced after peepopt I am not sure any pass would shrink the operand before we bitblast the multiplier

Choose a reason for hiding this comment

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

Implemented here: akashlevy@8b08f81
and
akashlevy@8687e5f

Choose a reason for hiding this comment

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

@povik is this fix acceptable?

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
57 changes: 57 additions & 0 deletions passes/pmgen/peepopt_muxadd.pmg
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
pattern muxadd
//
// Authored by Akash Levy of Silimate, Inc. under ISC license.
// Transforms add->mux into mux->add:
// y = s ? (a + b) : a ===> y = a + (s ? b : 0)
//

state <SigSpec> add_a add_b add_y

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

code add_y add_a add_b
// Get adder signals
add_a = port(add, \A);
add_b = port(add, \B);
add_y = port(add, \Y);

// Fanout of each adder Y bit should be 1 (no bit-split)
for (auto bit : add_y)
if (nusers(bit) != 2)
reject;
akashlevy marked this conversation as resolved.
Show resolved Hide resolved

// A and B can be interchanged
branch;
std::swap(add_a, add_b);
povik marked this conversation as resolved.
Show resolved Hide resolved
endcode

match mux
akashlevy marked this conversation as resolved.
Show resolved Hide resolved
// Select mux of form s ? (a + b) : a, allow leading 0s when A_WIDTH != Y_WIDTH
select mux->type == $mux
index <SigSpec> port(mux, \A) === SigSpec({Const(State::S0, GetSize(add_y)-GetSize(add_a)), add_a})
povik marked this conversation as resolved.
Show resolved Hide resolved
index <SigSpec> port(mux, \B) === add_y
endmatch

code
// Get mux signal
SigSpec mux_y = port(mux, \Y);

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

// Rewire
mux->setPort(\A, Const(State::S0, GetSize(add_b)));
mux->setPort(\B, add_b);
mux->setPort(\Y, mid);
add->setPort(\B, mid);
povik marked this conversation as resolved.
Show resolved Hide resolved
add->setPort(\Y, mux_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();
accept;
endcode