fix(verilog): declare the enums a spec imports (Closes #2316) - #2317
Merged
Conversation
`gen_verilog_expr` lowers `Enum.variant` to the flat identifier `Enum_variant`
whichever spec declared the enum; `gen_verilog` declared the matching
`localparam` only for enums a spec declares itself. An enum arriving through
`use` therefore produced a reference with nothing declaring it, which is not
valid Verilog under any standard:
gen/mac.v:100: error: Unable to bind wire/reg/memory `Trit_neg'
in `ZeroDSP_MAC.extract_trit.extract_trit_body'
`use_resolve::imported_enums` returns `(enum, [(variant, value)])` for the specs
a file imports -- enums only, no splicing. `resolve()` pulls whole declarations
the Verilog backend cannot lower, and 492 of the 650 specs carry a `use` line;
an enum is the one case where the backend already does everything except emit
the declaration. The backend emits them through the same `gen_verilog_enum` a
same-spec enum goes through.
Two filters. Referenced only: an enum the module never names emits nothing, so
`specs/fpga/uart.t27` -- which imports both modules that declare `Trit` and
never mentions it -- is byte-identical to before. Never shadow: a name the
module already declares is dropped, since a redeclaration is a compile error and
this pass may only ever add a declaration nothing else provides.
Measured with the prebuilt `t27c` at `fb88da234`, which regenerates `mac.v`
byte-identical to the `fpga-verilog` artifact of run 32358697899:
`iverilog -g2012 -DSIMULATION gen/mac.v` goes from 28 errors to 22, none of them
mentioning `Trit_`. Across all 650 specs, exactly six import an enum they name
and do not declare, and all six want `Trit`.
`fpga-conformance` stays red at 4/32. The other 22 errors in `mac.v`, and 26 of
the 28 failing modules, are a different defect: a struct-typed function
parameter is declared as one scalar and then referenced through
`<param>_<field>` names nothing declares (`word_raw`, `mac_units_status`,
`cfg_addr_width` -- 202 distinct identifiers). Choosing one struct
representation is a design decision, not a repair, and it is not made here.
Closes #2316
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Contributor
gHashTag
enabled auto-merge (squash)
August 20, 2026 12:35
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
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 #2316
What was wrong
gen_verilog_exprlowersEnum.variantto the flat identifierEnum_variantwhichever spec declared the enum.
gen_verilogdeclared the matchinglocalparamonly for enums a spec declares itself, so an enum arrivingthrough
useproduced a reference with nothing declaring it:specs/fpga/mac.t27doesuse base::ops;, andbase/ops.t27declaresTrit = enum(i8) { neg = -1, zero = 0, pos = 1 }.Enums only, not
use_resolve::resolveresolve()splices whole declarations -- functions, structs, constants -- andthe Verilog backend cannot lower most of them, while 492 of the 650 specs carry
a
useline. An enum is the one case where the backend already does everythingexcept emit the declaration, so
use_resolve::imported_enumsreturns(enum, [(variant, value)])and nothing else, and the backend emits it throughthe same
gen_verilog_enuma same-spec enum goes through.Two filters keep the blast radius where it belongs:
specs/fpga/uart.t27imports both modules that declareTritand nevermentions it; its output is byte-identical to before. There is a test for
exactly this.
redeclaration is a compile error, so this pass may only ever ADD a declaration
that nothing else provides.
Measured
The prebuilt
t27catfb88da234regeneratesmac.vbyte-identical to thefpga-verilogartifact of run 32358697899, so it is a faithful stand-in for theemitter. With the three
localparamlines at the exact position the enumsection occupies and nothing else changed:
iverilog -g2012 -DSIMULATION gen/mac.vTrit_Across all 650 specs, the ones that carry a
use, referenceEnum.variant, anddo not declare that enum locally are exactly six, and all six want
Trit:fpga/mac.t27,nn/attention.t27,nn/hslm.t27,vsa/ops.t27, and the twodemos/jones_topology_*-- the last two import onlybase::types, which isNOPARSE, so they are unchanged by this.What this does NOT do
fpga-conformancestays red at 4/32.mac.vstill fails on 22 errors, and26 of the 28 failing modules fail on a defect this PR does not touch: a
struct-typed function parameter is declared as one scalar and then referenced
through
<param>_<field>names nothing declares --word_raw,mac_units_status,cfg_addr_width, 202 distinct identifiers. The emitter isinternally inconsistent there (packed bit-slices in some places, per-field regs
in others,
<param>_<field>matching neither); picking one representation is adesign decision, not a repair.
bridge.v(cross-spec calls to functions definedinside other modules -- illegal in Verilog whatever file set you compile) and
zerodsp_top.v(a structural wrapper the loop compiles without its fivedependencies) are the remaining two.
gen-verilog-for-simulation/icarus-simulateare deliberately left on thesource-only path, so no
.trinity/icarus-baselines/entry moves.Verification, and its limits
iverilogbefore/after on the real artifact, above. This is the load-bearingevidence.
bootstrap/tests/verilog_imported_enum.rsasserts the property directly --every
Enum_variantidentifier the output USES is one it DECLARES -- onmac.t27, onuart.t27as the control, and on a spec tree the test owns. Runagainst the current artifact the first and third fail (3 undeclared each) and
the control passes, which is what a test for this defect should do.
cargo test -p t27cwas removedfrom
corpus-ratchetin fix(corpus-ratchet): remove the t27c test step; it was measured on the wrong tree #2293 because master has 13 failing tests; per thenote left there it should return as a ratchet, not a plain gate. Nothing in CI
currently compiles
bootstrap/tests/**.rustc --edition 2021 --crate-type lib --emit=metadataovercompiler.rs+use_resolve.rs: identical error andwarning set to master (one unresolved
serdeimport, 8 warnings), and adeliberately planted type error in the new code was caught, so the check is
not vacuous.