From c952ab417f1ac0a5e21aa0ce49e552bb70c1c3fe Mon Sep 17 00:00:00 2001 From: George Rennie Date: Sat, 26 Apr 2025 12:03:50 +0200 Subject: [PATCH 1/3] opt_expr: only sign extend shift arguments for arithmetic right shift --- passes/opt/opt_expr.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/passes/opt/opt_expr.cc b/passes/opt/opt_expr.cc index df969daf0..74f5b386a 100644 --- a/passes/opt/opt_expr.cc +++ b/passes/opt/opt_expr.cc @@ -1315,13 +1315,14 @@ skip_fine_alu: RTLIL::SigSpec sig_a = assign_map(cell->getPort(ID::A)); RTLIL::SigSpec sig_y(cell->type == ID($shiftx) ? RTLIL::State::Sx : RTLIL::State::S0, cell->getParam(ID::Y_WIDTH).as_int()); - // Limit indexing to the size of a, which is behaviourally identical (result is all 0) - // and avoids integer overflow of i + shift_bits when e.g. ID::B == INT_MAX - shift_bits = min(shift_bits, GetSize(sig_a)); - if (cell->type != ID($shiftx) && GetSize(sig_a) < GetSize(sig_y)) sig_a.extend_u0(GetSize(sig_y), cell->getParam(ID::A_SIGNED).as_bool()); + // Limit indexing to the size of a, which is behaviourally identical (result is all 0) + // and avoids integer overflow of i + shift_bits when e.g. ID::B == INT_MAX. + // We do this after sign-extending a so this accounts for the output size + shift_bits = min(shift_bits, GetSize(sig_a)); + for (int i = 0; i < GetSize(sig_y); i++) { int idx = i + shift_bits; if (0 <= idx && idx < GetSize(sig_a)) From 70a44f035c4152c99f4c8408e562095c58b22676 Mon Sep 17 00:00:00 2001 From: George Rennie Date: Sat, 26 Apr 2025 12:10:53 +0200 Subject: [PATCH 2/3] tests: test opt_expr constant shift edge cases --- tests/opt/opt_expr_shift.ys | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tests/opt/opt_expr_shift.ys diff --git a/tests/opt/opt_expr_shift.ys b/tests/opt/opt_expr_shift.ys new file mode 100644 index 000000000..aac2e6f62 --- /dev/null +++ b/tests/opt/opt_expr_shift.ys @@ -0,0 +1,50 @@ +# Testing edge cases where ports are signed/have different widths/shift amounts +# greater than the size + +read_verilog <> 20; + assign shr_us = in_u >> 20; + assign shr_su = in_s >> 20; + assign shr_ss = in_s >> 20; + assign sshl_uu = in_u <<< 20; + assign sshl_us = in_u <<< 20; + assign sshl_su = in_s <<< 20; + assign sshl_ss = in_s <<< 20; + assign sshr_uu = in_u >>> 20; + assign sshr_us = in_u >>> 20; + assign sshr_su = in_s >>> 20; + assign sshr_ss = in_s >>> 20; +endmodule +EOT + +equiv_opt opt_expr + +design -load postopt +select -assert-none t:$shl +select -assert-none t:$shr +select -assert-none t:$sshl +select -assert-none t:$sshr From 4fbb2bc1f397846fefbf4e7fa25c96e38b0eb184 Mon Sep 17 00:00:00 2001 From: George Rennie Date: Sat, 26 Apr 2025 18:34:21 +0200 Subject: [PATCH 3/3] celledges: use capped shift width --- kernel/celledges.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/celledges.cc b/kernel/celledges.cc index 8129e6b1b..8e52d0380 100644 --- a/kernel/celledges.cc +++ b/kernel/celledges.cc @@ -247,7 +247,7 @@ void shift_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell) db->add_edge(cell, ID::A, a_width - 1, ID::Y, i, -1); } - for (int k = 0; k < b_width; k++) { + for (int k = 0; k < b_width_capped; k++) { // left shifts if (cell->type.in(ID($shl), ID($sshl))) { if (a_width == 1 && is_signed) { @@ -268,7 +268,7 @@ void shift_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell) bool shift_in_bulk = i < a_width - 1; // can we jump into the zero-padding by toggling B[k]? bool zpad_jump = (((y_width - i) & ((1 << (k + 1)) - 1)) != 0 \ - && (((y_width - i) & ~(1 << k)) < (1 << b_width))); + && (((y_width - i) & ~(1 << k)) < (1 << b_width_capped))); if (shift_in_bulk || (cell->type.in(ID($shr), ID($shift), ID($shiftx)) && zpad_jump)) db->add_edge(cell, ID::B, k, ID::Y, i, -1); @@ -279,7 +279,7 @@ void shift_op(AbstractCellEdgesDatabase *db, RTLIL::Cell *cell) // bidirectional shifts (positive B shifts right, negative left) } else if (cell->type.in(ID($shift), ID($shiftx)) && is_b_signed) { if (is_signed) { - if (k != b_width - 1) { + if (k != b_width_capped - 1) { bool r_shift_in_bulk = i < a_width - 1; // assuming B is positive, can we jump into the upper zero-padding by toggling B[k]? bool r_zpad_jump = (((y_width - i) & ((1 << (k + 1)) - 1)) != 0 \