Fix/write vhdl shift pos operand types

src/vhdl_backend.cc

@@ -754,15 +754,15 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 		if (a_width == y_width) {
 			dump_sigspec(f, cell->getPort(ID::A));
 		} else {
-			// Use arith_open/close so y_width==1 produces std_logic (not
-			// std_logic_vector(0 downto 0) which would be a type mismatch).
+			// Use arith_open/close so y_width==1 produces std_logic, not
+			// std_logic_vector(0 downto 0).  Use dump_sigspec_unsigned/signed
+			// so 1-bit and constant A are wrapped correctly.
 			f << arith_open(y_width) << "resize(";
 			if (a_signed)
-				f << "signed(";
+				dump_sigspec_signed(f, cell->getPort(ID::A));
 			else
-				f << "unsigned(";
-			dump_sigspec(f, cell->getPort(ID::A));
-			f << "), " << y_width << ")" << arith_close(y_width);
+				dump_sigspec_unsigned(f, cell->getPort(ID::A));
+			f << ", " << y_width << ")" << arith_close(y_width);
 		}
 		f << ";\n";
 		return true;
@@ -1006,18 +1006,13 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 			f << "shift_right(";
 		}
 
-		if (a_signed && cell->type.in(ID($sshl), ID($sshr))) {
-			f << "signed(";
-			dump_sigspec(f, cell->getPort(ID::A));
-			f << ")";
-		} else {
-			f << "unsigned(";
-			dump_sigspec(f, cell->getPort(ID::A));
-			f << ")";
-		}
-		f << ", to_integer(unsigned(";
-		dump_sigspec(f, cell->getPort(ID::B));
-		f << ")))";
+		if (a_signed && cell->type.in(ID($sshl), ID($sshr)))
+			dump_sigspec_signed(f, cell->getPort(ID::A));
+		else
+			dump_sigspec_unsigned(f, cell->getPort(ID::A));
+		f << ", to_integer(";
+		dump_sigspec_unsigned(f, cell->getPort(ID::B));
+		f << "))";
 
 		f << ", " << y_width << "))";
 		if (y_width == 1)
@@ -1037,17 +1032,17 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 		f << " <= std_logic_vector(resize(";
 		if (b_signed) {
 			// When B is negative, shift left; when positive, shift right
-			f << "shift_right(unsigned(";
-			dump_sigspec(f, cell->getPort(ID::A));
-			f << "), to_integer(signed(";
-			dump_sigspec(f, cell->getPort(ID::B));
-			f << ")))";
+			f << "shift_right(";
+			dump_sigspec_unsigned(f, cell->getPort(ID::A));
+			f << ", to_integer(";
+			dump_sigspec_signed(f, cell->getPort(ID::B));
+			f << "))";
 		} else {
-			f << "shift_right(unsigned(";
-			dump_sigspec(f, cell->getPort(ID::A));
-			f << "), to_integer(unsigned(";
-			dump_sigspec(f, cell->getPort(ID::B));
-			f << ")))";
+			f << "shift_right(";
+			dump_sigspec_unsigned(f, cell->getPort(ID::A));
+			f << ", to_integer(";
+			dump_sigspec_unsigned(f, cell->getPort(ID::B));
+			f << "))";
 		}
 		f << ", " << y_width << "))";
 		if (y_width == 1)
@@ -1063,11 +1058,11 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 		f << indent << "-- $shiftx\n";
 		f << indent;
 		dump_sigspec(f, cell->getPort(ID::Y));
-		f << " <= std_logic_vector(resize(shift_right(unsigned(";
-		dump_sigspec(f, cell->getPort(ID::A));
-		f << "), to_integer(unsigned(";
-		dump_sigspec(f, cell->getPort(ID::B));
-		f << "))), " << y_width << "))";
+		f << " <= std_logic_vector(resize(shift_right(";
+		dump_sigspec_unsigned(f, cell->getPort(ID::A));
+		f << ", to_integer(";
+		dump_sigspec_unsigned(f, cell->getPort(ID::B));
+		f << ")), " << y_width << "))";
 		if (y_width == 1)
 			f << "(0)";
 		f << ";\n";

testsuite/write_vhdl/jcore/.gitignore

@@ -0,0 +1 @@
+jcore-src/

testsuite/write_vhdl/jcore/testsuite.sh

@@ -0,0 +1,103 @@
+#!/bin/sh
+# Test: write_vhdl J-Core CPU synthesis
+#
+# Synthesises the J-Core SH2-compatible CPU from VHDL source via the GHDL
+# plugin and verifies that write_vhdl produces VHDL-93 that GHDL accepts.
+#
+# This is a real-world integration test: the J-Core CPU is a substantial
+# VHDL design (~14 source files, ~15k lines of synthesised VHDL output)
+# that exercises record ports, nested entities, FFs, memories, and the
+# full range of cell types the backend must handle.
+#
+# Source: testsuite/write_vhdl/jcore/jcore-src/   (not checked in)
+#
+# To run this test, populate jcore-src/ with the J-Core RTL source tree.
+# The expected layout mirrors jcore-calc-ghdl/:
+#   jcore-src/
+#     tools/v2p          perl macro preprocessor
+#     cpu2j0_pkg.vhd     and the remaining RTL .vhd/.vhm files
+#
+# The .vhm files are VHDL with macro-preprocessing; the test runs
+# perl tools/v2p to expand them to plain VHDL before analysis.
+#
+# Skipped if jcore-src/ is not present.
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+. "$SCRIPT_DIR/../../rename/common.sh"
+check_tools
+
+JCORE="$SCRIPT_DIR/jcore-src"
+
+if [ ! -d "$JCORE" ]; then
+    echo "SKIP: J-Core source not found (populate $SCRIPT_DIR/jcore-src/)"
+    exit 0
+fi
+
+if [ ! -f "$JCORE/tools/v2p" ]; then
+    echo "SKIP: J-Core preprocessor not found at $JCORE/tools/v2p"
+    exit 0
+fi
+
+# Preprocess .vhm -> .vhd
+for vhm in "$JCORE"/*.vhm; do
+    base=$(basename "${vhm%.vhm}")
+    perl "$JCORE/tools/v2p" < "$vhm" > "$WORK/${base}.vhd"
+done
+
+# Copy plain .vhd RTL files
+cp "$JCORE"/cpu2j0_pkg.vhd \
+   "$JCORE"/components_pkg.vhd \
+   "$JCORE"/mult_pkg.vhd \
+   "$JCORE"/decode_pkg.vhd \
+   "$JCORE"/decode_body.vhd \
+   "$JCORE"/datapath_pkg.vhd \
+   "$JCORE"/cpu.vhd \
+   "$JCORE"/decode.vhd \
+   "$JCORE"/decode_table.vhd \
+   "$JCORE"/register_file_sync.vhd \
+   "$JCORE"/decode_table_reverse.vhd \
+   "$WORK/" 2>/dev/null || {
+    echo "FAIL jcore: could not copy RTL source files"
+    exit 1
+}
+
+# Analyse in dependency order (--std=08 for VHDL-2008 compatibility)
+CORE_FILES="cpu2j0_pkg.vhd components_pkg.vhd mult_pkg.vhd decode_pkg.vhd \
+decode_body.vhd datapath_pkg.vhd cpu.vhd decode.vhd decode_core.vhd \
+decode_table.vhd datapath.vhd register_file_sync.vhd mult.vhd decode_table_reverse.vhd"
+
+(cd "$WORK" && for f in $CORE_FILES; do
+    "$GHDL" -a --std=08 "$f" 2>/dev/null || {
+        echo "FAIL jcore: ghdl -a failed on $f"
+        exit 1
+    }
+done) || exit 1
+
+# Synthesise the top-level cpu entity and emit VHDL-93
+(cd "$WORK" && yosys -m "$PLUGIN" \
+    -p "ghdl --std=08 $CORE_FILES -e cpu; write_vhdl cpu_synth.vhd") \
+    >"$WORK/yosys.log" 2>&1 || {
+    echo "FAIL jcore: yosys synthesis failed"
+    cat "$WORK/yosys.log" >&2
+    exit 1
+}
+
+if [ ! -f "$WORK/cpu_synth.vhd" ]; then
+    echo "FAIL jcore: write_vhdl produced no output"
+    exit 1
+fi
+
+lines=$(wc -l < "$WORK/cpu_synth.vhd")
+echo "  cpu_synth.vhd: $lines lines"
+
+# Syntax check the output under both VHDL standards
+"$GHDL" -s --std=93 "$WORK/cpu_synth.vhd" || {
+    echo "FAIL jcore: GHDL -s --std=93 rejected cpu_synth.vhd"
+    exit 1
+}
+"$GHDL" -s --std=08 "$WORK/cpu_synth.vhd" || {
+    echo "FAIL jcore: GHDL -s --std=08 rejected cpu_synth.vhd"
+    exit 1
+}
+
+echo "PASS: jcore"

testsuite/write_vhdl/shift_pos/t_pos_1bit.v

@@ -0,0 +1,18 @@
+// t_pos_1bit.v -- $pos extending a 1-bit signal to a wider type (issue #232)
+//
+// When a 1-bit wire feeds a wider mux (e.g. via ternary), Yosys inserts a
+// $pos cell (A_WIDTH=1, Y_WIDTH=N).  write_vhdl was emitting:
+//   resize(unsigned(data), N)
+// unsigned() requires std_logic_vector, not std_logic.
+// Fix: use dump_sigspec_unsigned for A in $pos, which emits
+// unsigned'("" & data) for 1-bit signals.
+module t_pos_1bit (
+    input  wire       sel,
+    input  wire       data,        // 1-bit: becomes std_logic
+    input  wire [3:0] bus,         // 4-bit: control, must not regress
+    output wire [3:0] o_extend,    // sel ? data : 4'b0  ($pos A_WIDTH=1 Y_WIDTH=4)
+    output wire [3:0] o_ctrl       // sel ? bus  : 4'b0  (no $pos: control)
+);
+    assign o_extend = sel ? data  : 4'b0;  // $pos data 1->4, then $mux
+    assign o_ctrl   = sel ? bus   : 4'b0;  // no $pos needed: widths match
+endmodule

testsuite/write_vhdl/shift_pos/t_shift_1bit.v

@@ -0,0 +1,25 @@
+// t_shift_1bit.v -- shift with 1-bit operands (issues #231 and related)
+//
+// Two sub-cases, same root cause: raw unsigned()/signed() on std_logic fails.
+//
+// 1-bit B (issue #231): to_integer(unsigned(shamt)) where shamt is std_logic.
+//   Fix: use dump_sigspec_unsigned(B) -> unsigned'("" & shamt).
+//
+// 1-bit A: unsigned(flag) where flag is std_logic.
+//   Fix: use dump_sigspec_unsigned(A) -> unsigned'("" & flag).
+module t_shift_1bit (
+    input  wire [4:0] data,     // 5-bit data
+    input  wire       flag,     // 1-bit: becomes std_logic
+    input  wire       shamt,    // 1-bit shift amount
+    output wire [4:0] o_shl_b,  // data << shamt   (1-bit B)
+    output wire [4:0] o_shr_b,  // data >> shamt   (1-bit B)
+    output wire [4:0] o_shl_a,  // flag << 2       (1-bit A)
+    output wire [4:0] o_shr_a,  // flag >> 1       (1-bit A, different const)
+    output wire [4:0] o_ctrl    // data >> 2       (control: normal widths)
+);
+    assign o_shl_b = data << shamt;  // $shl B_WIDTH=1
+    assign o_shr_b = data >> shamt;  // $shr B_WIDTH=1
+    assign o_shl_a = flag << 2;      // $shl A_WIDTH=1
+    assign o_shr_a = flag >> 1;      // $shr A_WIDTH=1
+    assign o_ctrl  = data >> 2;      // control: no 1-bit operands
+endmodule

testsuite/write_vhdl/shift_pos/t_shift_const.v

@@ -0,0 +1,19 @@
+// t_shift_const.v -- shift by a constant amount (issue #230)
+//
+// Yosys represents the constant shift amount as a 32-bit RTLIL constant.
+// write_vhdl was emitting:
+//   to_integer(unsigned("00000000000000000000000000000001"))
+// A string literal is not a valid operand for unsigned() in VHDL-93.
+// Fix: use dump_sigspec_unsigned for B so constants get unsigned'("...").
+module t_shift_const (
+    input  wire [3:0] a,
+    output wire [3:0] o_shl,   // a << 1  ($shl, B is 32-bit constant 1)
+    output wire [3:0] o_shr,   // a >> 2  ($shr, B is 32-bit constant 2)
+    output wire [3:0] o_sshl,  // a <<< 1 ($sshl, signed left shift)
+    output wire [3:0] o_sshr   // a >>> 1 ($sshr, signed right shift)
+);
+    assign o_shl  = a << 1;
+    assign o_shr  = a >> 2;
+    assign o_sshl = $signed(a) <<< 1;
+    assign o_sshr = $signed(a) >>> 1;
+endmodule

testsuite/write_vhdl/shift_pos/testsuite.sh

@@ -0,0 +1,67 @@
+#!/bin/sh
+# Test: write_vhdl shift and $pos operand type errors (#230, #231, #232)
+#
+# Three distinct bugs in operand wrapping, all with the same root cause:
+# the shift handlers and $pos used dump_sigspec() directly on operands
+# that require dump_sigspec_unsigned/signed to handle 1-bit signals and
+# constants correctly.
+#
+# #230: shift by constant -- to_integer(unsigned("0000...1")) is illegal;
+#       string literals cannot be type conversion operands in VHDL-93.
+# #231: shift by 1-bit signal -- to_integer(unsigned(sig)) where sig is
+#       std_logic; unsigned() requires std_logic_vector.
+# #232: $pos extending a 1-bit signal -- resize(unsigned(data), N) where
+#       data is std_logic; same type mismatch as #231.
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+. "$SCRIPT_DIR/../../rename/common.sh"
+check_tools
+
+run_test() {
+    label="$1"
+    src="$2"
+    out="$3"
+
+    cp "$SCRIPT_DIR/$src" "$WORK/$src"
+    (cd "$WORK" && yosys -m "$PLUGIN" \
+        -p "read_verilog $src; write_vhdl $out") \
+        >"$WORK/${label}.log" 2>&1 || {
+        echo "FAIL $label: yosys write_vhdl failed"
+        cat "$WORK/${label}.log" >&2
+        exit 1
+    }
+
+    if command -v "$GHDL" >/dev/null 2>&1; then
+        "$GHDL" -s --std=93 "$WORK/$out" || {
+            echo "FAIL $label: GHDL -s --std=93 rejected $out"
+            exit 1
+        }
+        "$GHDL" -s --std=08 "$WORK/$out" || {
+            echo "FAIL $label: GHDL -s --std=08 rejected $out"
+            exit 1
+        }
+    fi
+
+    echo "PASS: $label"
+}
+
+# ---------------------------------------------------------------------------
+# t_shift_const: shift by constant -- issue #230
+#   $shl, $shr, $sshl, $sshr with B a 32-bit RTLIL constant
+# ---------------------------------------------------------------------------
+run_test t_shift_const t_shift_const.v t_shift_const.vhd
+
+# ---------------------------------------------------------------------------
+# t_shift_1bit: shift with 1-bit operands -- issues #231 and related
+#   $shl, $shr with B_WIDTH=1 (std_logic shift amount, issue #231)
+#   $shl, $shr with A_WIDTH=1 (std_logic value being shifted)
+# ---------------------------------------------------------------------------
+run_test t_shift_1bit t_shift_1bit.v t_shift_1bit.vhd
+
+# ---------------------------------------------------------------------------
+# t_pos_1bit: $pos extending 1-bit signal to wider type -- issue #232
+#   $pos A_WIDTH=1, Y_WIDTH=4 triggered by ternary with mixed widths
+# ---------------------------------------------------------------------------
+run_test t_pos_1bit t_pos_1bit.v t_pos_1bit.vhd
+
+echo "PASS: shift_pos"