Fix/write vhdl conditional in aggregate - #228
Merged
tgingold merged 3 commits intoJun 21, 2026
Merged
Conversation
…n-aggregate
write_vhdl emits illegal VHDL-93 when boolean/comparison cells have
output width > 1:
n0 <= (0 => '1' when COND else '0', others => '0');
VHDL-93 s7.3.2 forbids conditional expressions inside aggregates.
GHDL rejects this with "')' is expected instead of 'when'".
Five test modules, all failing before the fix:
t_exact: verbatim reporter case ($logic_and y_width=3)
t_logic: $logic_and/or/not at y_width=4 (broken) and y_width=1 (control)
t_reduce: $reduce_and/or/xor/xnor at y_width=4 and y_width=1;
$reduce_xnor has zero prior test coverage at any width
t_compare: $eq/$ne/$lt/$eqx at y_width=4 and y_width=1;
$eqx has zero prior test coverage at any width
t_combined: three independent boolean cells ($logic_and/or/not) feeding
one 4-bit output mixed with a plain signal and a constant;
asserts exactly 3 std_logic intermediates post-fix -- one per
cell, proving the fix is per-cell not a merged expression
Each module includes y_width=1 control outputs exercising the same cell
types on the already-correct path, confirming the fix does not regress it.
Ref: ghdl#227
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Two related fixes for illegal VHDL-93 output: 1. Conditional-in-aggregate (issue ghdl#227): $logic_and, $logic_or, $logic_not, $reduce_and/or/xor/xnor, and comparison cells ($lt/le/eq/ne/ge/gt/eqx/nex) with output width > 1 were emitting: n0 <= (0 => '1' when COND else '0', others => '0'); VHDL-93 s7.3.2 forbids conditional expressions inside aggregates. GHDL rejects this with "')' is expected instead of 'when'". Fix: introduce a std_logic intermediate signal via aux_signal_decls and emit two legal concurrent statements: n_bool <= '1' when COND else '0'; n0 <= (0 => n_bool, others => '0'); The intermediate is allocated only when Y_WIDTH > 1; the common 1-bit path is unchanged. 2. $pos Y_WIDTH==1 type mismatch: When $pos extends/truncates to width 1, the backend was emitting: Y <= std_logic_vector(resize(unsigned(A), 1)); which has type std_logic_vector(0 downto 0), not std_logic. Fix: use arith_open/arith_close as every other arithmetic cell does, producing resize(unsigned(A), 1)(0) -- a std_logic value. Implementation: a new dump_bool_assign() helper encapsulates the Y_WIDTH dispatch and the intermediate-signal pattern, eliminating repetition across all five affected cell handlers. Also rename ff_signal_decls -> aux_signal_decls to reflect its broader use: FF internals, split-Y temporaries, and now bool-to-vector intermediates. Fixes: ghdl#227 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Add t_vhdl.vhd exercising three paths used extensively in production:
- Signed/unsigned arithmetic: GHDL normalizes signed/unsigned to
std_logic_vector before emitting RTLIL; write_vhdl must emit valid
VHDL-93 for the resulting $add cells.
- Single-bit sub-range slices: bus8(2) and bus8(5) must produce
std_logic output ports, not std_logic_vector(0 downto 0).
- Boolean flag OR'd into vector: the issue ghdl#227 pattern exercised
end-to-end via the GHDL import path.
Note: fixed_pkg is intentionally not used. fixed_pkg injects overflow
$assert cells whose 'if '1' = '1'' condition is ambiguous under both
--std=93 and --std=08 when std_logic_1164 is in scope. That is a
pre-existing limitation of the $assert handler, separate from this fix.
Only VHDL-2008 syntax is checked (ghdl -s --std=08); simulation-level
correctness is deferred to a future test.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #227
write_vhdlemitted a conditional expression inside an aggregate element association, which is illegal in VHDL. Affected cell types whenY_WIDTH > 1:$logic_and/or/not, $reduce_*,and all comparison cells.Fix: introduce an intermediate std_logic signal only when needed, reusing the existing
aux_signal_declsmechanism.Also fixes a
$postype mismatch forY_WIDTH = 1.Test cases cover all affected operators at both
Y_WIDTH > 1andY_WIDTH = 1(control), including a combined output to verify one intermediate per cell.