Skip to content

Commit

Permalink
peepopt: Fix padding for the peepopt_shiftmul_right pattern
Browse files Browse the repository at this point in the history
The previous version could easily generate a large amount of padding
when the constant factor was significantly larger than the width of the
shift data input. This could lead to huge amounts of logic being
generated before then being optimized away at a huge performance and
memory cost.

Additionally and more critically, when the input width was not a
multiple of the constant factor, the input data was padded with 'x bits
to such a multiple before interspersing the 'x padding needed to align
the selectable windows to power-of-two offsets.

Such a final padding would not be correct for shifts besides $shiftx,
and the previous version did attempt to remove that final padding at the
end so that the native zero/sign/x-extension behavior of the shift cell
would be used, but since the last selectable window also got
power-of-two padding appended after the padding the code is trying to
remove got added, it did not actually fully remove it in some cases.

I changed the code to only add 'x padding between selectable windows,
leaving the last selectable window unpadded. This omits the need to add
final padding to a multiple of the constant factor in the first place.
In turn, that means the only 'x bits added are actually impossible to
select. As a side effect no padding is added when the constant factor is
equal to or larger than the width of the shift data input, also solving
the reported performance bug.

This fixes #4056
  • Loading branch information
jix committed Dec 6, 2023
1 parent 45dd9ec commit 7b74caa
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
19 changes: 7 additions & 12 deletions passes/pmgen/peepopt_shiftmul_right.pmg
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,17 @@ code
int new_const_factor = 1 << factor_bits;
SigSpec padding(State::Sx, new_const_factor-const_factor);
SigSpec old_a = port(shift, \A), new_a;
int trunc = 0;

if (GetSize(old_a) % const_factor != 0) {
trunc = const_factor - GetSize(old_a) % const_factor;
old_a.append(SigSpec(State::Sx, trunc));
}

for (int i = 0; i*const_factor < GetSize(old_a); i++) {
SigSpec slice = old_a.extract(i*const_factor, const_factor);
new_a.append(slice);
new_a.append(padding);
if ((i+1)*const_factor < GetSize(old_a)) {
SigSpec slice = old_a.extract(i*const_factor, const_factor);
new_a.append(slice);
new_a.append(padding);
} else {
new_a.append(old_a.extract_end(i*const_factor));
}
}

if (trunc > 0)
new_a.remove(GetSize(new_a)-trunc, trunc);

SigSpec new_b = {mul_din, SigSpec(State::S0, factor_bits)};
if (param(shift, \B_SIGNED).as_bool())
new_b.append(State::S0);
Expand Down
26 changes: 25 additions & 1 deletion tests/various/peepopt.ys
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,31 @@ design -import gold -as gold peepopt_shiftmul_2
design -import gate -as gate peepopt_shiftmul_2

miter -equiv -make_assert -make_outputs -ignore_gold_x -flatten gold gate miter
sat -show-public -enable_undef -prove-asserts miter
sat -verify -show-public -enable_undef -prove-asserts miter
cd gate
select -assert-count 1 t:$shr
select -assert-count 1 t:$mul
select -assert-count 0 t:$shr t:$mul %% t:* %D

####################

design -reset
read_verilog <<EOT
module peepopt_shiftmul_3 (input [7:0] D, input [0:0] S, output [3:0] Y);
assign Y = D >> (S*5);
endmodule
EOT

prep
design -save gold
peepopt
design -stash gate

design -import gold -as gold peepopt_shiftmul_3
design -import gate -as gate peepopt_shiftmul_3

miter -equiv -make_assert -make_outputs -ignore_gold_x -flatten gold gate miter
sat -verify -show-public -enable_undef -prove-asserts miter
cd gate
select -assert-count 1 t:$shr
select -assert-count 1 t:$mul
Expand Down

0 comments on commit 7b74caa

Please sign in to comment.