gen-verilog: a GF16 value is sixteen bits, not thirty-two - #3444
Merged
Conversation
Closes #3442 Refs #3443 u16 -> input [15:0] x; GF16 -> input [31:0] x; NoSuchTypeXY -> input [31:0] x; The third line is the finding. 32 was not a decision about GF16; it was the unknown-type default, and this project's flagship type was indistinguishable from one the backend has never heard of. The same file already knew: `HwType::GF16.hw_width()` returns 16 and `HwType::GF16.verilog_range()` returns "[15:0]", both with passing tests. A function parameter reaches a different reader, `type_to_width`, which had no arm and fell to `_ => 32`. Two readers of one type, one of them green. The neighbouring arm carries the same story in a comment -- f64 "fell through to the 32-bit default and silently narrowed to half its width" -- so this is that defect again, in the other direction. Three spellings, three sizes of one bug: GF16 32 -> 16 gf16::GF16 32 -> 16 one type, two widths, depending on how a spec spelled it [4]GF16 32 -> 64 EIGHT bits per element, a quarter of the data Measured over 583 generated Verilog files, old binary against new: 119 sites in 3 files for the bare form, 278 in 8 files for the qualified one, and one struct (`SearchResult`) that stops being "UNSUPPORTED_ICARUS" and packs to 48 bits. Every added line contains [15:0] and every removed one [31:0], with a single matching pair of exceptions that is the same fact: a tuple return of three GF16 went [95:0] -> [47:0]. The widening of `is_primitive_scalar_type` was measured before being taken rather than reasoned about -- the comment on its neighbour records that widening a predicate here was once traced to the corpus' largest defect class. Blast radius: one file, six lines, and the change is the struct above. The load-bearing test is the negative one. A "fix" that widened the default to 16 passes every positive assertion in the file; only `an_unknown_type_still_takes_the_thirty_two_bit_default` kills that mutant. Four mutants, four dead. Full suite 2598 passed, 0 failed -- the same 2598 that passed while the width was wrong. Found by tools/backend_parity_table.py, added here: it prints what each of the four backends emits for one declaration form, side by side, because five previous repairs shared this shape and every one was found by accident. It is a reader, not a gate -- a row whose columns disagree is a question. Its first run also refuted one of its own candidates: `[]const u8` lowering to Rust's `&'static str` looks like a type change and is a deliberate, load-bearing choice, since the HashMap key lowering asks that mapping for its key type. Two further rows are filed as latent with a corpus population of ZERO in #3443: a nested array parameter emits `[3]u8* x` into a C header, and `tri` has no C spelling at all. The 11 files whose Verilog moved leave 21 stale seals, and they are resealed in this same commit rather than in the next one. That cycle -- every backend change invalidating seals with nothing in the merge path refreshing them -- is #3440, and I merged past a red `Spec Guards` twice before noticing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gHashTag
enabled auto-merge (squash)
September 8, 2026 01:04
Contributor
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
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 #3442 · Refs #3443
The third line is the finding. 32 was not a decision about GF16 — it was the
unknown-type default, and this project's flagship type was indistinguishable
from one the backend has never heard of.
The same file already said 16
Both assertions pass today. A function parameter reaches a different reader,
type_to_width, which had no arm and fell to_ => 32. Two readers of onetype, one of them green — and 2598 tests passed while the width was wrong.
The neighbouring arm carries the same story in a comment:
f64"fell throughto the 32-bit default and silently narrowed to half its width. Named explicitly
so the width is a decision rather than a fallthrough."
Three spellings, three sizes of one bug
GF16[31:0][15:0]gf16::GF16[31:0][15:0]— one type had two widths depending on how a spec spelled it[4]GF16[31:0][63:0]— eight bits per element, a quarter of the dataMeasured — 583 generated Verilog files, old binary against new
GF16narrowed 32 → 16gf16::GF16narrowedSearchResultstops beingUNSUPPORTED_ICARUS, packs to 48 bitsEvery added line contains
[15:0]and every removed line[31:0], with onematching pair of exceptions that is the same fact: a tuple return of three GF16
went
[95:0]→[47:0].Widening
is_primitive_scalar_typewas measured before being taken — thecomment on its neighbour records that widening a predicate here was once traced
to the corpus' largest defect class. Blast radius: one file, six lines, and the
change is the struct above.
The load-bearing test is the negative one
A "fix" that widened the default to 16 passes every positive assertion in the
file. Only
an_unknown_type_still_takes_the_thirty_two_bit_defaultkills it.Full suite: 2598 passed, 0 failed — the same 2598 that passed before.
How it was found
tools/backend_parity_table.py, added here. It prints what each of the fourbackends emits for one declaration form, side by side, because five previous
repairs shared this shape (
.len, the out-parameter,*T, the array size inC, the
Copyderive) and every one was found by accident.It is a reader, not a gate — a row whose columns disagree is a question. Its
first run refuted one of its own candidates:
[]const u8lowering to Rust's&'static strlooks like a type change and is a deliberate, load-bearingchoice, since the HashMap key lowering asks that mapping for its key type.
Two further rows are filed as latent, population zero (#3443): a nested
array parameter emits
[3]u8* xinto a C header, andtrihas no C spellingat all.
Seals
The 11 files whose Verilog moved leave 21 stale seals, resealed in this same
commit rather than the next one. That cycle is #3440 — I merged past a red
Spec Guardstwice before noticing it.