From 10383ab2a4b9a14eec5c4d33da21c1f2ea23c368 Mon Sep 17 00:00:00 2001 From: Gus Smith Date: Mon, 23 Mar 2026 15:04:47 -0700 Subject: [PATCH 1/5] Tests for both issues --- tests/verilog/issue4402.ys | 20 ++++++++++++++++++++ tests/verilog/issue5745.ys | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/verilog/issue4402.ys create mode 100644 tests/verilog/issue5745.ys diff --git a/tests/verilog/issue4402.ys b/tests/verilog/issue4402.ys new file mode 100644 index 000000000..82133f60e --- /dev/null +++ b/tests/verilog/issue4402.ys @@ -0,0 +1,20 @@ +# Issue #4402: read_verilog doesn't respect signed keyword +# +# write_verilog drops the signed keyword from input port declarations, +# even though the internal comparison logic is correctly preserved via +# explicit $signed() casts. The port declaration should retain signed +# so that the interface contract is correct for downstream tools. + +! mkdir -p temp + +read_verilog < 11), giving k = 0. + +read_verilog < Date: Mon, 23 Mar 2026 15:20:42 -0700 Subject: [PATCH 2/5] Add `signed` keyword in write_verilog --- backends/verilog/verilog_backend.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/backends/verilog/verilog_backend.cc b/backends/verilog/verilog_backend.cc index 73ffcbf3e..d5f83aefc 100644 --- a/backends/verilog/verilog_backend.cc +++ b/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 } From 8a282b3c1753c28833dfe15ca034f125fa78f678 Mon Sep 17 00:00:00 2001 From: Gus Smith Date: Mon, 23 Mar 2026 15:21:48 -0700 Subject: [PATCH 3/5] Update test --- tests/verilog/issue4402.ys | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/verilog/issue4402.ys b/tests/verilog/issue4402.ys index 82133f60e..f08c43229 100644 --- a/tests/verilog/issue4402.ys +++ b/tests/verilog/issue4402.ys @@ -1,9 +1,6 @@ # Issue #4402: read_verilog doesn't respect signed keyword # -# write_verilog drops the signed keyword from input port declarations, -# even though the internal comparison logic is correctly preserved via -# explicit $signed() casts. The port declaration should retain signed -# so that the interface contract is correct for downstream tools. +# write_verilog was not emitting the signed keyword for port declarations. ! mkdir -p temp @@ -16,5 +13,4 @@ hierarchy -top mod write_verilog temp/issue4402_roundtrip.v # The output port declaration must include the signed keyword. -# Bug: write_verilog emits `input [5:0] wire0` instead of `input signed [5:0] wire0`. ! grep -q "input signed" temp/issue4402_roundtrip.v From 91fca0a0eceb551ae1988a31f6406ede307335ca Mon Sep 17 00:00:00 2001 From: Gus Smith Date: Mon, 23 Mar 2026 16:26:07 -0700 Subject: [PATCH 4/5] Make tests closer to original issue --- tests/verilog/issue4402.ys | 28 +++++++++++++---- tests/verilog/issue4402_sim.sh | 55 ++++++++++++++++++++++++++++++++++ tests/verilog/issue4402_tb.v | 20 +++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100755 tests/verilog/issue4402_sim.sh create mode 100644 tests/verilog/issue4402_tb.v diff --git a/tests/verilog/issue4402.ys b/tests/verilog/issue4402.ys index f08c43229..f5ddca7d2 100644 --- a/tests/verilog/issue4402.ys +++ b/tests/verilog/issue4402.ys @@ -1,16 +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 < /dev/null 2>&1; then + echo "SKIP: iverilog not found" + exit 0 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +YOSYS="${YOSYS:-$SCRIPT_DIR/../../yosys}" +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +cat > "$TMPDIR/top.v" << 'EOF' +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 +EOF + +# Synthesize +"$YOSYS" -q -p " +read_verilog $TMPDIR/top.v +hierarchy -top top +proc +write_verilog $TMPDIR/top_syn.v +" + +# Simulate original +iverilog -o "$TMPDIR/sim_orig" "$SCRIPT_DIR/issue4402_tb.v" "$TMPDIR/top.v" 2>/dev/null +# Simulate synthesized +iverilog -o "$TMPDIR/sim_syn" "$SCRIPT_DIR/issue4402_tb.v" "$TMPDIR/top_syn.v" 2>/dev/null + +ORIG=$(vvp "$TMPDIR/sim_orig" 2>/dev/null | grep "y =") +SYN=$(vvp "$TMPDIR/sim_syn" 2>/dev/null | grep "y =") + +echo "Original: $ORIG" +echo "Synthesized: $SYN" + +if [ "$ORIG" != "$SYN" ]; then + echo "FAIL: pre/post-synthesis outputs differ" + exit 1 +fi +echo "PASS" diff --git a/tests/verilog/issue4402_tb.v b/tests/verilog/issue4402_tb.v new file mode 100644 index 000000000..4faee54b9 --- /dev/null +++ b/tests/verilog/issue4402_tb.v @@ -0,0 +1,20 @@ +`timescale 1ns / 1ps +module testbench; + reg clk; + reg signed [5:0] wire0; + wire y; + + top uut (.y(y), .clk(clk), .wire0(wire0)); + + initial begin + clk = 0; + wire0 = 6'b111101; + forever #5 clk = ~clk; + end + + initial begin + #100; + $display("y = %d", y); + $finish; + end +endmodule From b0871185ef35220ea1bcf2d5a0634d462acf17e8 Mon Sep 17 00:00:00 2001 From: Gus Smith Date: Mon, 20 Apr 2026 18:56:04 -0700 Subject: [PATCH 5/5] Make sure to apply correct signedness to loop vars --- frontends/ast/simplify.cc | 28 ++++++++----- tests/verilog/for_loop_signed_index.ys | 56 ++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 11 deletions(-) create mode 100644 tests/verilog/for_loop_signed_index.ys diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index f314ff3d5..ffe8f859c 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -2591,21 +2591,26 @@ 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 &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(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(); @@ -2680,6 +2685,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); } diff --git a/tests/verilog/for_loop_signed_index.ys b/tests/verilog/for_loop_signed_index.ys new file mode 100644 index 000000000..87e6b14cb --- /dev/null +++ b/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 <