An array parameter that cannot be bound is passed by value - #2745
Merged
Conversation
…y value
W458 binds an array parameter to ONE module-level array name and lets the body
reference that name directly, which is why such parameters vanish from the
emitted port list. Three situations had no single name to bind, and all three
REFUSED the whole function:
* no call site at all
* call sites that disagree -- `neuronN(acts, w0, ..)` and `neuronN(acts, w1, ..)`
* an argument that is not a plain identifier
Refusing is what produced the contradiction in specs/ternary/bitnet_layer.t27,
where the emitter printed
// ERROR: function on_comb has array parameter(s) but no call site
and four lines below wrote
assign result = on_comb(acts, w0, w1, nchunks, threshold);
-- the call it had just said did not exist.
None of the three is a defect. An array parameter that cannot be bound is simply
passed BY VALUE: it stays a real `input [W-1:0]` and the body indexes it by
element slice, which #1745 already implements and which every one of these
specs' testbenches was already written against. So the fix deletes the three
refusals rather than adding machinery.
MEASURED, whole test suite with --no-fail-fast:
2419 passed / 5 failed -> 2424 passed / 0 failed
The five were bitnet_layer, bitnet_mlp, bitnet_mlp3, bitnet_neuron_nchunk and
verilog_array_param_index -- four of them generate Verilog, compile it with
iverilog, RUN it and check numbers against a reference model. No test file is
touched by this commit.
Blast radius, generating Verilog for all 650 corpus specs with the before and
after binaries and diffing per file: 10 specs changed, every one in the same
direction -- 19 refusals removed, 19 functions gained, nothing lost. Corpus
acceptance is unchanged at Zig 217, cc 157, Zig-AND-Verilog 194.
A correction to the record: earlier reports in this loop of "1786 passed / 1
failed" came from `cargo test` runs that stop at the first failing binary. With
--no-fail-fast the repository baseline was 2419/5, and four of those five had
never been printed.
Closes #2743
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tals Closes #2743 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Contributor
PR DashboardGenerated at: 2026-08-28 00:20:59 UTC
Summary
Seal Status
|
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.
Closes #2743
W458 binds an array parameter to one module-level array name and lets the body reference that name directly — which is why such parameters vanish from the emitted port list. Three situations have no single name to bind, and all three refused the whole function:
neuronN(acts, w0, …)andneuronN(acts, w1, …)That refusal is what produced the contradiction in
specs/ternary/bitnet_layer.t27, where the emitter printed// ERROR: function on_comb has array parameter(s) but no call siteand four lines below wrote
— the call it had just said did not exist.
None of the three is a defect
An array parameter that cannot be bound is simply passed by value: it stays a real
input [W-1:0]and the body indexes it by element slice, which #1745 already implements and which every one of these specs' testbenches was already written against.So the fix deletes the three refusals rather than adding machinery: 18 insertions, 22 deletions, one file.
I first built the elaborate version — monomorphising the conflicting function into
neuronN__acts_w0/neuronN__acts_w1, binding the entry point's parameters to their own ports, and teaching the index path that a bound name can be packed. It worked, and it was wrong: it fixed one of the five failures and left the by-value path — which the testbenches assume — still refused. The small answer subsumes it.Measured
Whole test suite,
--no-fail-fast:The five were
bitnet_layer,bitnet_mlp,bitnet_mlp3,bitnet_neuron_nchunkandverilog_array_param_index. Four of them generate Verilog, compile it with iverilog, run it, and check numbers against a reference model — a two-layer MLP, a three-layer MLP, a two-neuron layer, an n-chunk neuron. No test file is touched by this commit.Blast radius, generating Verilog for all 650 corpus specs with the before binary and the after binary and diffing per file:
Corpus acceptance unchanged: Zig 217, cc 157, Zig-AND-Verilog 194.
A correction to the record
Earlier reports in this loop of "1786 passed / 1 failed" came from
cargo testruns that stop at the first failing binary. With--no-fail-fastthe repository baseline was 2419 / 5, and four of those five failures had never been printed by anything I ran. The numbers above are the whole suite.🤖 Generated with Claude Code