Skip to content

fix(multibit): concat broadcast into parametric sub-circuit ports - #72

Merged
jeffersonmourak merged 1 commit into
mainfrom
fix-concat-broadcast-papercut
May 25, 2026
Merged

fix(multibit): concat broadcast into parametric sub-circuit ports#72
jeffersonmourak merged 1 commit into
mainfrom
fix-concat-broadcast-papercut

Conversation

@jeffersonmourak

Copy link
Copy Markdown
Owner

Summary

Fixes the first of three multi-bit-language papercuts documented in #71's
fixtures: xor xn[4](a={c, c, c, c}, b=...) is now accepted, removing the
need for the wire-buffer broadcast workaround in those fixtures.

Root cause

Two bugs converged to reject the pattern:

  1. Validatorport_validation.zig checked Component.width on the
    destination directly. For a sub_circuit_ref that field reflects only
    the type-position [N] annotation; the instance-position [N]
    (xor xn[4]) parses into width_args and is consumed by the project
    resolver during specialization. Result: Component.width stayed at
    the default 1, and the concat-width check spuriously errored with
    "concat width 4 ... does not match destination width 1".

  2. Topology builder{serializer,full_serializer}.zig walked
    module.components in a single declaration-order pass. Recursion
    into a sub_circuit_ref tried to resolve sibling sources (e.g. a
    concat declared after the instance) that weren't yet in
    local_to_global, hitting error.InternalError.

Fix

  • port_validation.zig's concat-width check now skips destinations that
    are sub_circuit_ref. The cross-boundary E014 check in
    sub_circuit_validation.zig has the project context to look up the
    specialized target's input port width; endpointWidth is extended so
    the concat/slice .from side returns its Component.width,
    completing the path.

  • Topology Pass 1 is split into Pass 1a (emit primitives, slices,
    concats; register in local_to_global) and Pass 1b (recurse into
    sub-circuit instances). Every sibling is resolvable at recursion time.

The split shifts global IDs (in-module primitives precede sub-circuit
children in emit order). Truth-table simulation is unaffected — the
runtime's nodes.items[id] lookup still matches emit-order ID
assignment.

Test plan

  • zig build test passes (one over-specified topology test was
    rewritten to find components by name instead of by position).
  • zig build bench reports "golden matches".
  • Existing scalar alu_4bit.truth.golden semantics preserved.
  • Parametric and inline 4-bit ALU goldens from test(fixtures): multi-bit Hack ALU fixtures (parametric + single-file) #71 match
    bit-for-bit against the regenerated outputs.

Goldens regenerated

  • 5 preview-layout/render goldens — ID shifts only; visual layout,
    coordinates, and wiring identical.
  • tests/fixtures/bench/engine.bench.golden — ~35 sub-circuit
    fixtures show topology_hash shifts. Counters, vector counts,
    and component counts byte-identical.

Follow-up

The wire-buffer broadcast workarounds in #71's fixtures can now be
inlined to direct {c, c, ...} concats. Leaving that cleanup for a
separate PR to keep this review focused on the engine change.

`xor xn[4](a={c, c, c, c}, b=...)` and similar broadcast-via-concat
patterns into parametric macro/sub-circuit instance ports are now
accepted. Two papercuts converged to reject this pattern:

1. Validator (port_validation.zig): the concat width-sum check read
   `Component.width` on the destination directly. For a sub_circuit_ref,
   that field only reflects the type-position `[N]` annotation; the
   instance-position `[N]` parses into `width_args` and is consumed by
   the project resolver during specialization, leaving `Component.width`
   at its default of 1. The local check now skips sub_circuit_ref
   destinations and lets sub_circuit_validation.zig handle the cross-
   boundary width check (which has the project context to read the
   specialized target's input port width). `endpointWidth` is extended
   to return `component.width` for slice/concat on the `.from` side so
   that path correctly emits E014 for genuine mismatches.

2. Topology builder (serializer.zig, full_serializer.zig): the single
   Pass 1 over `module.components` processed each in declaration order,
   so recursion into a sub_circuit_ref tried to resolve a sibling concat
   or slice declared later (and not yet in `local_to_global`), failing
   with `error.InternalError`. Pass 1 now splits:
     Pass 1a emits primitives/slices/concats.
     Pass 1b recurses into sub_circuit_refs with every sibling already
              registered.
   This shifts global IDs: in-module primitives now precede sub-circuit
   children in emit order. Truth-table simulation is unaffected; the
   runtime's `nodes.items[id]` lookup still works because emit order
   continues to match sequential ID assignment.

The one over-specified test, `full_walk_one_subcircuit_records_origin`,
was rewritten to find components by name rather than asserting their
positions in the emit array.

Goldens regenerated where ID ordering surfaces:
  - 5 preview-layout/render goldens (ID shifts only; visual layout,
    coordinates, and wiring stay identical).
  - tests/fixtures/bench/engine.bench.golden: ~35 sub-circuit fixtures
    show topology_hash shifts. Counters, vector counts, and component
    counts are byte-identical, so the regression gate's intent (no
    extra algorithmic work) holds.
@jeffersonmourak jeffersonmourak added the perf This contains performance work label May 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Engine bench: deltas vs main

35 fixture(s) changed

Fixture Counter Before After Δ Δ %

Produced by zig build bench -- --output=json against main's engine.bench.golden, rendered by tools/bench/format-delta-comment.py. Wall-clock (drv_ms) is host-dependent and not asserted, so it isn't shown; see DOCS/benchmark.md for the bench's measurement model.

@jeffersonmourak
jeffersonmourak merged commit 057e96a into main May 25, 2026
4 of 5 checks passed
@jeffersonmourak
jeffersonmourak deleted the fix-concat-broadcast-papercut branch May 25, 2026 22:15
github-actions Bot added a commit that referenced this pull request May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf This contains performance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant