From: Xiretza <xiretza@xiretza.xyz>
Date: Fri, 3 Jul 2020 11:13:21 +0000 (+0200)
Subject: Respect \A_SIGNED for $shift
X-Git-Tag: working-ls180~320
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=928fd40c2ebc8b83b76c02d80d751d2531341d9d;p=yosys.git

Respect \A_SIGNED for $shift

This reflects the behaviour of $shr/$shl, which sign-extend their A
operands to the size of their output, then do a logical shift (shift in
0-bits).
---

diff --git a/backends/btor/btor.cc b/backends/btor/btor.cc
index e5da6c1e7..5abab8978 100644
--- a/backends/btor/btor.cc
+++ b/backends/btor/btor.cc
@@ -205,9 +205,8 @@ struct BtorWorker
 			if (cell->type.in(ID($xnor), ID($_XNOR_))) btor_op = "xnor";
 			log_assert(!btor_op.empty());
 
-			int width = GetSize(cell->getPort(ID::Y));
-			width = std::max(width, GetSize(cell->getPort(ID::A)));
-			width = std::max(width, GetSize(cell->getPort(ID::B)));
+			int width_ay = std::max(GetSize(cell->getPort(ID::A)), GetSize(cell->getPort(ID::Y)));
+			int width = std::max(width_ay, GetSize(cell->getPort(ID::B)));
 
 			bool a_signed = cell->hasParam(ID::A_SIGNED) ? cell->getParam(ID::A_SIGNED).as_bool() : false;
 			bool b_signed = cell->hasParam(ID::B_SIGNED) ? cell->getParam(ID::B_SIGNED).as_bool() : false;
@@ -224,11 +223,23 @@ struct BtorWorker
 			int sid = get_bv_sid(width);
 			int nid;
 
+			int nid_a;
+			if (cell->type.in(ID($shl), ID($shr), ID($shift), ID($shiftx)) && a_signed && width_ay < width) {
+				// sign-extend A up to the width of Y
+				int nid_a_padded = get_sig_nid(cell->getPort(ID::A), width_ay, a_signed);
+
+				// zero-extend the rest
+				int zeroes = get_sig_nid(Const(0, width-width_ay));
+				nid_a = next_nid++;
+				btorf("%d concat %d %d %d\n", nid_a, sid, zeroes, nid_a_padded);
+			} else {
+				nid_a = get_sig_nid(cell->getPort(ID::A), width, a_signed);
+			}
+
+			int nid_b = get_sig_nid(cell->getPort(ID::B), width, b_signed);
+
 			if (btor_op == "shift")
 			{
-				int nid_a = get_sig_nid(cell->getPort(ID::A), width, false);
-				int nid_b = get_sig_nid(cell->getPort(ID::B), width, b_signed);
-
 				int nid_r = next_nid++;
 				btorf("%d srl %d %d %d\n", nid_r, sid, nid_a, nid_b);
 
@@ -248,9 +259,6 @@ struct BtorWorker
 			}
 			else
 			{
-				int nid_a = get_sig_nid(cell->getPort(ID::A), width, a_signed);
-				int nid_b = get_sig_nid(cell->getPort(ID::B), width, b_signed);
-
 				nid = next_nid++;
 				btorf("%d %s %d %d %d%s\n", nid, btor_op.c_str(), sid, nid_a, nid_b, getinfo(cell).c_str());
 			}
diff --git a/backends/btor/test_cells.sh b/backends/btor/test_cells.sh
old mode 100644
new mode 100755
index 3f077201a..0a011932d
--- a/backends/btor/test_cells.sh
+++ b/backends/btor/test_cells.sh
@@ -6,7 +6,7 @@ rm -rf test_cells.tmp
 mkdir -p test_cells.tmp
 cd test_cells.tmp
 
-../../../yosys -p 'test_cell -n 5 -w test all /$alu /$fa /$lcu /$lut /$sop /$macc /$mul /$div /$mod /$divfloor /$modfloor'
+../../../yosys -p 'test_cell -n 5 -w test all /$alu /$fa /$lcu /$lut /$sop /$macc /$mul /$div /$mod /$divfloor /$modfloor /$shiftx'
 
 for fn in test_*.il; do
 	../../../yosys -p "
@@ -19,7 +19,7 @@ for fn in test_*.il; do
 		hierarchy -top main
 		write_btor ${fn%.il}.btor
 	"
-	boolectormc -kmax 1 --trace-gen --stop-first -v ${fn%.il}.btor > ${fn%.il}.out
+	btormc -kmax 1 --trace-gen --stop-first -v ${fn%.il}.btor > ${fn%.il}.out
 	if grep " SATISFIABLE" ${fn%.il}.out; then
 		echo "Check failed for ${fn%.il}."
 		exit 1
diff --git a/backends/verilog/verilog_backend.cc b/backends/verilog/verilog_backend.cc
index a0e677d13..372f68ea5 100644
--- a/backends/verilog/verilog_backend.cc
+++ b/backends/verilog/verilog_backend.cc
@@ -750,21 +750,19 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 		f << stringf(" = ");
 		if (cell->getParam(ID::B_SIGNED).as_bool())
 		{
-			f << stringf("$signed(");
-			dump_sigspec(f, cell->getPort(ID::B));
-			f << stringf(")");
+			dump_cell_expr_port(f, cell, "B", true);
 			f << stringf(" < 0 ? ");
-			dump_sigspec(f, cell->getPort(ID::A));
+			dump_cell_expr_port(f, cell, "A", true);
 			f << stringf(" << - ");
 			dump_sigspec(f, cell->getPort(ID::B));
 			f << stringf(" : ");
-			dump_sigspec(f, cell->getPort(ID::A));
+			dump_cell_expr_port(f, cell, "A", true);
 			f << stringf(" >> ");
 			dump_sigspec(f, cell->getPort(ID::B));
 		}
 		else
 		{
-			dump_sigspec(f, cell->getPort(ID::A));
+			dump_cell_expr_port(f, cell, "A", true);
 			f << stringf(" >> ");
 			dump_sigspec(f, cell->getPort(ID::B));
 		}
diff --git a/kernel/calc.cc b/kernel/calc.cc
index ae18809d3..d54ccbc10 100644
--- a/kernel/calc.cc
+++ b/kernel/calc.cc
@@ -275,10 +275,15 @@ RTLIL::Const RTLIL::const_logic_or(const RTLIL::Const &arg1, const RTLIL::Const
 	return result;
 }
 
-static RTLIL::Const const_shift_worker(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool sign_ext, int direction, int result_len)
+// Shift `arg1` by `arg2` bits.
+// If `direction` is +1, `arg1` is shifted right by `arg2` bits; if `direction` is -1, `arg1` is shifted left by `arg2` bits.
+// If `signed2` is true, `arg2` is interpreted as a signed integer; a negative `arg2` will cause a shift in the opposite direction.
+// Any required bits outside the bounds of `arg1` are padded with `vacant_bits` unless `sign_ext` is true, in which case any bits outside the left
+// bounds are filled with the leftmost bit of `arg1` (arithmetic shift).
+static RTLIL::Const const_shift_worker(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool sign_ext, bool signed2, int direction, int result_len, RTLIL::State vacant_bits = RTLIL::State::S0)
 {
 	int undef_bit_pos = -1;
-	BigInteger offset = const2big(arg2, false, undef_bit_pos) * direction;
+	BigInteger offset = const2big(arg2, signed2, undef_bit_pos) * direction;
 
 	if (result_len < 0)
 		result_len = arg1.bits.size();
@@ -290,9 +295,9 @@ static RTLIL::Const const_shift_worker(const RTLIL::Const &arg1, const RTLIL::Co
 	for (int i = 0; i < result_len; i++) {
 		BigInteger pos = BigInteger(i) + offset;
 		if (pos < 0)
-			result.bits[i] = RTLIL::State::S0;
+			result.bits[i] = vacant_bits;
 		else if (pos >= BigInteger(int(arg1.bits.size())))
-			result.bits[i] = sign_ext ? arg1.bits.back() : RTLIL::State::S0;
+			result.bits[i] = sign_ext ? arg1.bits.back() : vacant_bits;
 		else
 			result.bits[i] = arg1.bits[pos.toInt()];
 	}
@@ -304,61 +309,36 @@ RTLIL::Const RTLIL::const_shl(const RTLIL::Const &arg1, const RTLIL::Const &arg2
 {
 	RTLIL::Const arg1_ext = arg1;
 	extend_u0(arg1_ext, result_len, signed1);
-	return const_shift_worker(arg1_ext, arg2, false, -1, result_len);
+	return const_shift_worker(arg1_ext, arg2, false, false, -1, result_len);
 }
 
 RTLIL::Const RTLIL::const_shr(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool, int result_len)
 {
 	RTLIL::Const arg1_ext = arg1;
 	extend_u0(arg1_ext, max(result_len, GetSize(arg1)), signed1);
-	return const_shift_worker(arg1_ext, arg2, false, +1, result_len);
-}
-
-RTLIL::Const RTLIL::const_sshl(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool signed2, int result_len)
-{
-	if (!signed1)
-		return const_shl(arg1, arg2, signed1, signed2, result_len);
-	return const_shift_worker(arg1, arg2, true, -1, result_len);
+	return const_shift_worker(arg1_ext, arg2, false, false, +1, result_len);
 }
 
-RTLIL::Const RTLIL::const_sshr(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool signed2, int result_len)
+RTLIL::Const RTLIL::const_sshl(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool, int result_len)
 {
-	if (!signed1)
-		return const_shr(arg1, arg2, signed1, signed2, result_len);
-	return const_shift_worker(arg1, arg2, true, +1, result_len);
+	return const_shift_worker(arg1, arg2, signed1, false, -1, result_len);
 }
 
-static RTLIL::Const const_shift_shiftx(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool, bool signed2, int result_len, RTLIL::State other_bits)
+RTLIL::Const RTLIL::const_sshr(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool, int result_len)
 {
-	int undef_bit_pos = -1;
-	BigInteger offset = const2big(arg2, signed2, undef_bit_pos);
-
-	if (result_len < 0)
-		result_len = arg1.bits.size();
-
-	RTLIL::Const result(RTLIL::State::Sx, result_len);
-	if (undef_bit_pos >= 0)
-		return result;
-
-	for (int i = 0; i < result_len; i++) {
-		BigInteger pos = BigInteger(i) + offset;
-		if (pos < 0 || pos >= BigInteger(int(arg1.bits.size())))
-			result.bits[i] = other_bits;
-		else
-			result.bits[i] = arg1.bits[pos.toInt()];
-	}
-
-	return result;
+	return const_shift_worker(arg1, arg2, signed1, false, +1, result_len);
 }
 
 RTLIL::Const RTLIL::const_shift(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool signed2, int result_len)
 {
-	return const_shift_shiftx(arg1, arg2, signed1, signed2, result_len, RTLIL::State::S0);
+	RTLIL::Const arg1_ext = arg1;
+	extend_u0(arg1_ext, max(result_len, GetSize(arg1)), signed1);
+	return const_shift_worker(arg1_ext, arg2, false, signed2, +1, result_len);
 }
 
-RTLIL::Const RTLIL::const_shiftx(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool signed2, int result_len)
+RTLIL::Const RTLIL::const_shiftx(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool, bool signed2, int result_len)
 {
-	return const_shift_shiftx(arg1, arg2, signed1, signed2, result_len, RTLIL::State::Sx);
+	return const_shift_worker(arg1, arg2, false, signed2, +1, result_len, RTLIL::State::Sx);
 }
 
 RTLIL::Const RTLIL::const_lt(const RTLIL::Const &arg1, const RTLIL::Const &arg2, bool signed1, bool signed2, int result_len)
diff --git a/kernel/satgen.cc b/kernel/satgen.cc
index 73839d37a..2a54e78ec 100644
--- a/kernel/satgen.cc
+++ b/kernel/satgen.cc
@@ -522,7 +522,7 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep)
 
 		int extend_bit = ez->CONST_FALSE;
 
-		if (!cell->type.in(ID($shift), ID($shiftx)) && cell->parameters[ID::A_SIGNED].as_bool())
+		if (cell->parameters[ID::A_SIGNED].as_bool())
 			extend_bit = a.back();
 
 		while (y.size() < a.size())
@@ -555,7 +555,7 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep)
 			std::vector<int> undef_a_shifted;
 
 			extend_bit = cell->type == ID($shiftx) ? ez->CONST_TRUE : ez->CONST_FALSE;
-			if (!cell->type.in(ID($shift), ID($shiftx)) && cell->parameters[ID::A_SIGNED].as_bool())
+			if (cell->parameters[ID::A_SIGNED].as_bool())
 				extend_bit = undef_a.back();
 
 			while (undef_y.size() < undef_a.size())
diff --git a/techlibs/common/simlib.v b/techlibs/common/simlib.v
index 2660e6f15..e94884025 100644
--- a/techlibs/common/simlib.v
+++ b/techlibs/common/simlib.v
@@ -480,10 +480,18 @@ input [B_WIDTH-1:0] B;
 output [Y_WIDTH-1:0] Y;
 
 generate
-	if (B_SIGNED) begin:BLOCK1
-		assign Y = $signed(B) < 0 ? A << -B : A >> B;
-	end else begin:BLOCK2
-		assign Y = A >> B;
+	if (A_SIGNED) begin:BLOCK1
+		if (B_SIGNED) begin:BLOCK2
+			assign Y = $signed(B) < 0 ? $signed(A) << -B : $signed(A) >> B;
+		end else begin:BLOCK3
+			assign Y = $signed(A) >> B;
+		end
+	end else begin:BLOCK4
+		if (B_SIGNED) begin:BLOCK5
+			assign Y = $signed(B) < 0 ? A << -B : A >> B;
+		end else begin:BLOCK6
+			assign Y = A >> B;
+		end
 	end
 endgenerate
 
diff --git a/techlibs/common/techmap.v b/techlibs/common/techmap.v
index 9607302b7..03c27d49d 100644
--- a/techlibs/common/techmap.v
+++ b/techlibs/common/techmap.v
@@ -141,6 +141,7 @@ module _90_shift_shiftx (A, B, Y);
 	parameter [B_WIDTH-1:0] _TECHMAP_CONSTVAL_B_ = 0;
 
 	localparam extbit = _TECHMAP_CELLTYPE_ == "$shift" ? 1'b0 : 1'bx;
+	wire a_padding = _TECHMAP_CELLTYPE_ == "$shiftx" ? extbit : (A_SIGNED ? A[A_WIDTH-1] : 1'b0);
 
 	generate
 `ifndef NO_LSB_FIRST_SHIFT_SHIFTX
@@ -160,7 +161,7 @@ module _90_shift_shiftx (A, B, Y);
 			localparam entries = (A_WIDTH+Y_WIDTH-1)/Y_WIDTH2;
 			localparam len = Y_WIDTH2 * ((entries+1)/2);
 			wire [len-1:0] AA;
-			wire [(A_WIDTH+Y_WIDTH2+Y_WIDTH-1)-1:0] Apad = {{(Y_WIDTH2+Y_WIDTH-1){extbit}}, A};
+			wire [(A_WIDTH+Y_WIDTH2+Y_WIDTH-1)-1:0] Apad = {{(Y_WIDTH2+Y_WIDTH-1){a_padding}}, A};
 			genvar i;
 			for (i = 0; i < A_WIDTH; i=i+Y_WIDTH2*2)
 				assign AA[i/2 +: Y_WIDTH2] = B[CLOG2_Y_WIDTH] ? Apad[i+Y_WIDTH2 +: Y_WIDTH2] : Apad[i +: Y_WIDTH2];
@@ -187,7 +188,8 @@ module _90_shift_shiftx (A, B, Y);
 			always @* begin
 				overflow = 0;
 				buffer = {WIDTH{extbit}};
-				buffer[`MAX(A_WIDTH, Y_WIDTH)-1:0] = A;
+				buffer[Y_WIDTH-1:0] = {Y_WIDTH{a_padding}};
+				buffer[A_WIDTH-1:0] = A;
 
 				if (B_WIDTH > BB_WIDTH) begin
 					if (B_SIGNED) begin