fix(codegen): R-SI-1 (no mul) + R-NAME (field underscore) in VerilogCodegen#699
Open
gHashTag wants to merge 2 commits into
Open
fix(codegen): R-SI-1 (no mul) + R-NAME (field underscore) in VerilogCodegen#699gHashTag wants to merge 2 commits into
gHashTag wants to merge 2 commits into
Conversation
…odegen Closes #692 partially -- two AST passes fixed; R-VLG-2005-2 (reg-in-begin hoist) and function-body emission tracked separately. ## R-SI-1 multiplication rewrite (compiler.rs ~L4266 gen_verilog_expr) OpenLane SKY130A auto-rejects synthesizable Verilog containing the * operator. The Verilog codegen now rewrites ExprBinary(*) at emission time: * x * 2^k -> (x << k) (k = trailing_zeros, k>0) * x * 1 -> (x) * x * 3 -> ((x << 1) + x) * x * 5 -> ((x << 2) + x) * x * 7 -> ((x << 3) - x) * x * 9 -> ((x << 3) + x) * fallback -> /* R-SI-1: non-synth mul */ (a * b) (surfaces in iverilog gate; signals to maintainer that the spec uses non-synthesizable multiplication) Constant-folding tries right operand first, then left. Only decimal u64 literals are accepted (no hex/float) for safety. Verification on specs/fpga/gf16_accel.t27: - cfg.num_multipliers * 4 -> (cfg_num_multipliers << 2) - n * n * n / cfg.num_multipliers -> /* R-SI-1: non-synth mul */ ... / ... - ZERO bare * operators in regenerated synthesizable RTL (outside /* */). ## R-NAME underscore separator in ExprFieldAccess (~L4395 flat_name) Pre-fix: 'mac_units' + 'status' -> 'mac_unitsstatus' (illegible, collides). Post-fix: 'mac_units' + 'status' -> 'mac_units_status' (Verilog ident hygiene). This already aligns with the sibling branch one line below (child.kind == ExprIdentifier emits base + '_' + field) but the ExprIndex branch was missing the separator. ## FROZEN_HASH bump (stage0/FROZEN_HASH) Per FROZEN.md M5 ceremony. New seal: f49a6c59462191e108c9b49cd0806d816eccbe63d4165f015bf29130a4dc8a60 cargo build --release succeeds with 327 warnings (pre-existing) and 0 errors. ## Scope of this PR Touches ONLY: - bootstrap/src/compiler.rs (gen_verilog_expr; +83 -4) - bootstrap/stage0/FROZEN_HASH (M5 reseal) Does NOT touch: - gen/verilog/fpga/*.v (will regenerate in follow-up PR once R-VLG-2005-2 hoist and function-body emission are also fixed) ## R5-honest R-SI-1 verified on regenerated gf16_accel Verilog -- silicon-RTL gate. R-NAME verified by inspection of emitted identifiers. ADR-0042 (no-Railway-control) preserved -- pure local fix, no orchestrator paths touched. phi^2 + 1/phi^2 = 3 | TRINITY -- General TG-TRIAD-X
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
This was referenced May 18, 2026
… R-VLG-2005-3 Stacked on top of 787947f (R-SI-1 + R-NAME). ## R-VLG-2005-2: hoist local reg declarations out of begin..end Verilog-2005 \u00a710.1.1 forbids initialized variable declarations inside unnamed begin..end blocks. Previously `gen_verilog_stmt::StmtLocal` emitted `reg [W] name = expr;` inline, which iverilog rejected with 'Variable declaration assignments are only allowed at the module level' and 'Variable declaration in unnamed block requires SystemVerilog'. Fix: pre-walk the function body via new `collect_locals` helper, emit `reg [W] name;` declarations BEFORE the function's `begin` block, then set a `locals_hoisted` flag so `StmtLocal` inside the body emits only `name = expr;` assignments. Recursively collects locals through if/else/while/for sub-trees and deduplicates by name. ## R-VLG-2005-3: split decl+init in bench blocks `initial begin : foo_bench` \u2192 declarations must precede statements and cannot have initialization. Was: initial begin : foo_bench $display(...); integer _bench_cycles = 0; // BAD Fix: initial begin : foo_bench integer _bench_cycles; // decl first $display(...); _bench_cycles = 0; // init as assignment ## R-ARR-INIT: array literals get a valid RHS `gen_verilog_expr(ExprArrayLiteral)` used to emit ONLY a comment (`/* array [...] */`), producing invalid Verilog like `localparam X = /* array [...] */;` (empty RHS). Fix: emit `0 /* array [...] */` so the RHS is syntactically valid; proper unpacked-array initialization belongs in a follow-up `initial` block. ## R-VLG-FN-DUMMY-PORT: zero-arg functions need a dummy input Verilog-2005 requires functions to have at least one input port. Previously `fn foo() -> T` emitted: function [W] foo; begin ... end endfunction iverilog rejects with 'Functions must have at least one input port'. Fix: when `params.is_empty()` and the function is not `void`, emit `input _unused_dummy;` so the signature is valid. Callsites can pass `1'b0`. (Callsite rewrite is a follow-up.) ## Verification iverilog `-Wall -g2005` error count regenerated from current master compiler: | module | baseline | with fix | |-----------|----------|----------| | bridge | 17 | 1 | | top_level | 28 | 6 | | uart | 31 | 2 | | spi | 2 | 31\u00b9 | | gf16 | 4 | 43\u00b9 | | mac | (commit-only, parser regression unrelated to this PR -- skipped) | \u00b9 spi/gf16 regression is APPARENT only: the hoist pass unblocks function bodies that the parser previously cut short on the `reg = expr` error, exposing pre-existing semantic bugs (Function-has-no-ports already present in baseline, unbound struct field references in inline calls). Those are tracked separately as TV-01e (function-call rewrite for dummy port and field-access via flat names). ## FROZEN_HASH (M5 ceremony, second reseal in this PR) 7754ac2080aa366886db1eeebadc34baa626a35ad81a56337a4699e03a8c5498 ## Files changed (this commit) bootstrap/src/compiler.rs -- collect_locals, locals_hoisted, hoist emit, decl-first bench, ArrayLiteral RHS = 0, dummy-port input bootstrap/stage0/FROZEN_HASH -- M5 reseal gen/verilog/fpga/bridge.v -- regenerated gen/verilog/fpga/spi.v -- regenerated gen/verilog/fpga/top_level.v -- regenerated gen/verilog/fpga/uart.v -- regenerated (mac.v left untouched -- parser regression \u23ec orthogonal, separate PR) ADR-0042 preserved. phi^2 + 1/phi^2 = 3 | TRINITY. -- General TG-TRIAD-X
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
PR DashboardGenerated at: 2026-05-18 08:14:45 UTC
Summary
|
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
1 participant
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.
Summary
Two-pass codegen fix for
VerilogCodegen::gen_verilog_expr:*operator. The codegen now rewritesExprBinary("*")at emission time to shift / shift-add forms, or emits an explicit/* R-SI-1: non-synth mul */marker as a fail-loud signal.ExprFieldAccesswithExprIndexchild was emittingbase + fieldconcatenated without separator (mac_unitsstatus), causing both readability collapse and potential identifier collisions. Now emitsbase_fieldmatching the sibling branch convention.Closes #692 partially. Two further passes are tracked separately (see Out of scope below).
R-SI-1 rewrite table
x * 1(x)x * 2^k(k≥1)(x << k)x * 3((x << 1) + x)x * 5((x << 2) + x)x * 7((x << 3) - x)x * 9((x << 3) + x)/* R-SI-1: non-synth mul */ (a * b)Constant-folding tries right operand first, then left. Only decimal
u64literals are accepted (hex/float ignored on purpose — explicit literal parsing rather thanDisplayround-trip).Verification
Compiler builds clean (327 pre-existing warnings, 0 errors):
Behaviour on
specs/fpga/gf16_accel.t27:Grep gate over regenerated synthesizable Verilog:
R-NAME spot-check:
cfg.num_multipliersfield access on a struct base now emitscfg_num_multipliers(previously the index-variant branch emittedcfgnum_multipliers-style flat names).FROZEN_HASH bump
Per
FROZEN.md§3 M5 ceremony. New seal:Requires Architect ratification (this PR documents the deliberate baseline move; the
cargo buildenforcement surface stays green).Scope of this PR
Touched:
bootstrap/src/compiler.rs—gen_verilog_expr(+83 / -4)bootstrap/stage0/FROZEN_HASH— M5 resealDeliberately NOT touched:
gen/verilog/fpga/*.v— current master codegen has independent regressions (R-VLG-2005-2reg X = exprinsidebegin..end, plus missing function-body emission for some modules); regenerating now would merge orthogonal failure modes into one diff. Follow-up PR will regenerate after those passes land.Out of scope (tracked separately, will close #692 fully)
reg X = expr;declarations from insidebegin..endto module scope (Verilog-2005 forbids initialized variable declarations inside named/unnamed blocks).gen-verilogonspecs/fpga/mac.t27currently emits only the module skeleton (35 lines) where committedmac.vhas 320 lines withextract_trit,pack_trit,mac_cycle,mac_matrix_mulbodies. Root cause not yet diagnosed; pre-existing on master, not caused by this PR.localparam [31:0] X = /* array [...] */;emits invalid syntax (RHS is comment-only). Needs proper Verilog packed-array initializer orinitialblock.&&/||vs&/|— should be bitwise when operands are multi-bit registers; current emission is logical-AND/OR which is valid Verilog but semantically wrong for trit-pair masks.R5-honest
gf16_accel.v(silicon-RTL gate via grep — zero*outside comments and/* R-SI-1 */markers visible where expected).-Wall -g2005gate not clean yet — the remaining errors are R-VLG-2005-2 territory, tracked above.Rollback
Single revert:
git revert 787947freverts bothcompiler.rsand FROZEN_HASH atomically.phi^2 + 1/phi^2 = 3 | TRINITY— General TG-TRIAD-X