Fix: read_verilog doesn't respect signed keyword

backends/verilog/verilog_backend.cc

@@ -456,21 +456,22 @@ void dump_wire(std::ostream &f, std::string indent, RTLIL::Wire *wire)
 		if (wire->attributes.count(ID::single_bit_vector))
 			range = stringf(" [%d:%d]", wire->start_offset, wire->start_offset);
 	}
+	std::string sign = wire->is_signed ? " signed" : "";
 	if (wire->port_input && !wire->port_output)
-		f << stringf("%s" "input%s %s;\n", indent, range, id(wire->name));
+		f << stringf("%s" "input%s%s %s;\n", indent, sign, range, id(wire->name));
 	if (!wire->port_input && wire->port_output)
-		f << stringf("%s" "output%s %s;\n", indent, range, id(wire->name));
+		f << stringf("%s" "output%s%s %s;\n", indent, sign, range, id(wire->name));
 	if (wire->port_input && wire->port_output)
-		f << stringf("%s" "inout%s %s;\n", indent, range, id(wire->name));
+		f << stringf("%s" "inout%s%s %s;\n", indent, sign, range, id(wire->name));
 	if (reg_wires.count(wire->name)) {
-		f << stringf("%s" "reg%s %s", indent, range, id(wire->name));
+		f << stringf("%s" "reg%s%s %s", indent, sign, range, id(wire->name));
 		if (wire->attributes.count(ID::init)) {
 			f << stringf(" = ");
 			dump_const(f, wire->attributes.at(ID::init));
 		}
 		f << stringf(";\n");
 	} else
-		f << stringf("%s" "wire%s %s;\n", indent, range, id(wire->name));
+		f << stringf("%s" "wire%s%s %s;\n", indent, sign, range, id(wire->name));
 #endif
 }
 

frontends/ast/simplify.cc

@@ -2619,21 +2619,27 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin
 			input_error("Right hand side of 1st expression of %s for-loop is not constant!\n", loop_type_str);
 
 		auto resolved = current_scope.at(init_ast->children[0]->str);
-		if (resolved->range_valid) {
-			int const_size = varbuf->range_left - varbuf->range_right;
-			int resolved_size = resolved->range_left - resolved->range_right;
-			if (const_size < resolved_size) {
-				for (int i = const_size; i < resolved_size; i++)
-					varbuf->bits.push_back(resolved->is_signed ? varbuf->bits.back() : State::S0);
-				varbuf->range_left = resolved->range_left;
-				varbuf->range_right = resolved->range_right;
-				varbuf->range_swapped = resolved->range_swapped;
-				varbuf->range_valid = resolved->range_valid;
+		auto apply_loop_var_type = [&resolved](std::unique_ptr<AstNode> &value) {
+			if (resolved->range_valid) {
+				int const_size = value->range_left - value->range_right;
+				int resolved_size = resolved->range_left - resolved->range_right;
+				if (const_size < resolved_size) {
+					for (int i = const_size; i < resolved_size; i++)
+						value->bits.push_back(resolved->is_signed ? value->bits.back() : State::S0);
+					value->range_left = resolved->range_left;
+					value->range_right = resolved->range_right;
+					value->range_swapped = resolved->range_swapped;
+					value->range_valid = resolved->range_valid;
+				}
 			}
-		}
+			value->is_signed = resolved->is_signed;
+		};
+
+		apply_loop_var_type(varbuf);
 
 		varbuf = std::make_unique<AstNode>(location, AST_LOCALPARAM, std::move(varbuf));
 		varbuf->str = init_ast->children[0]->str;
+		varbuf->is_signed = resolved->is_signed;
 
 		AstNode *backup_scope_varbuf = current_scope[varbuf->str];
 		current_scope[varbuf->str] = varbuf.get();
@@ -2708,6 +2714,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin
 			if (buf->type != AST_CONSTANT)
 				input_error("Right hand side of 3rd expression of %s for-loop is not constant (%s)!\n", loop_type_str, type2str(buf->type));
 
+			apply_loop_var_type(buf);
 			varbuf->children[0] = std::move(buf);
 		}
 

tests/verilog/for_loop_signed_index.ys

@@ -0,0 +1,56 @@
+# Regression test: when procedural for-loops are unrolled, the constant
+# replacement for the loop variable must keep the variable's declared
+# signedness.
+
+read_verilog <<EOT
+module signed_index(input signed a, output reg y);
+  reg signed i;
+
+  always @*
+    for (i = 1'h0; i < 1'h1; i = i + 1'h1)
+      y = a <= i;
+endmodule
+
+EOT
+
+hierarchy -top signed_index
+proc
+sat -set a 1 -prove y 1 -verify
+
+design -reset
+
+# This loop runs more than once. The final assignment to y happens after the
+# step expression has recomputed i, so it fails unless the stepped value is
+# retagged with the loop variable's signedness too.
+read_verilog <<EOT
+module signed_index_after_step(input signed a, output reg y);
+  reg signed [1:0] i;
+
+  always @* begin
+    y = 1'b0;
+    for (i = -2'sd1; i < 2'sd1; i = i + 1'h1)
+      y = a <= i;
+  end
+endmodule
+EOT
+
+hierarchy -top signed_index_after_step
+proc
+sat -set a 1 -prove y 1 -verify
+
+design -reset
+
+# Control: `a` signed but `i` unsigned, so comparison should be unsigned.
+read_verilog <<EOT
+module unsigned_index(input signed a, output reg y);
+  reg i;
+
+  always @*
+    for (i = 1'h0; i < 1'h1; i = i + 1'h1)
+      y = a <= i;
+endmodule
+EOT
+
+hierarchy -top unsigned_index
+proc
+sat -set a 1 -prove y 0 -verify

tests/verilog/issue4402.ys

@@ -0,0 +1,32 @@
+# Issue #4402: read_verilog doesn't respect signed keyword
+#
+# write_verilog was not emitting the signed keyword for port declarations.
+# Uses the original reproduction module from the issue (var2/var3 given
+# initial values of 0, which were uninitialized/assumed-zero in the report).
+#
+# Pre-synthesis simulation:  wire0=1'b1 (signed -1), -1<=0 true -> y=0
+# Post-synthesis (unfixed):  wire0 loses signed, 1<=0 false -> y=1  (BUG)
+# Post-synthesis (fixed):    wire0 retains signed, -1<=0 true -> y=0
+
+! mkdir -p temp
+
+read_verilog <<EOT
+module top (y, clk, wire0);
+  output wire y;
+  input wire clk;
+  input wire signed wire0;
+  reg reg1;
+  reg var2 = 0;
+  reg var3 = 0;
+  assign y = reg1;
+  always @(posedge clk) begin
+    reg1 = ($signed(wire0 <= 0) ? $unsigned(-var3) : (^~$signed(var2)));
+  end
+endmodule
+EOT
+hierarchy -top top
+proc
+write_verilog temp/issue4402_syn.v
+
+# Port declaration must include the signed keyword.
+! grep -q "input signed wire0" temp/issue4402_syn.v