Skip to content

Fix SpMV correctness and align RTL schedule - #332

Draft
n0thingNoob wants to merge 111 commits into
zeonica-bicg-validationfrom
spmv_debug
Draft

Fix SpMV correctness and align RTL schedule#332
n0thingNoob wants to merge 111 commits into
zeonica-bicg-validationfrom
spmv_debug

Conversation

@n0thingNoob

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes the VectorCGRA SpMV RTL validation so that the numerical result matches the CPU model and the steady-state schedule matches the compiler/Zeonica timing.

SpMV validation and memory layout

  • Give the logical arrays distinct addresses: val=0, col=8, row=16, feature=24, and output=28.
  • Patch the generated GEP base constants locally in the pytest without changing spmv.yaml.
  • Replicate the input image across four physical memory banks and assign one bank to each unrolled lane.
  • Configure every tile before sending launch packets, then order launches by controller-ring distance.
  • Add a CPU golden model, logical-memory reconstruction, output-store monitoring, and an assertion that the measured effective II equals the compiled II.

Generator fixes

  • Order constants by their first real consumption time, including invalid_iterations * II, instead of YAML/control-address order.
  • Apply prologue handling to register-to-routing-crossbar paths as well as directional tile inputs.

RTL token and handshake fixes

  • Gate stores with address/data predicates and keep an outstanding load from accepting a later false token.
  • Preserve PHI input predicates and update PHI first-use state only on a completed handshake.
  • Track GRANT_ONCE state per control slot.
  • Defer constant-queue pointer advancement until the associated control step retires.
  • Drain one final II after CMD_COMPLETE so post-return stores can finish.
  • Preserve read-before-write data while a control step is stalled and drain unused register-routing inputs during prologue.
  • Expose control-step completion to register clusters in all tile variants.

Trace support

  • Record raw address interfaces and the current crossbar prologue state so load/store timing can be compared directly with Zeonica.

Root cause

The incorrect result had multiple causes: all GEP bases aliased the same memory region, constants were queued in a different order from their post-prologue consumption order, and several false/stalled tokens were retired incorrectly by the RTL.

After the numerical fixes, the measured RTL II was still 19 instead of the compiled II of 15. The four unrolled lanes were contending for the same single-port memory bank even though the compiler schedule assumed parallel accesses. Assigning one bank per lane removes that serialization.

Results

  • CPU and CGRA memory contents match.
  • SpMV output: [1, 4, 9, 0, 5, 12, 21, 0].
  • Compiled II: 15.
  • Measured RTL effective II: 15.
  • Main dataflow window: Zeonica 28 cycles, RTL 28 cycles.
  • First valid operation through final store: Zeonica 42 cycles, RTL 43 cycles.
  • The remaining one-cycle difference is startup ring/launch skew, not a steady-state stall.

Validation

  • Full SpMV RTL simulation with CPU-memory and effective-II assertions: passed.
  • SpMV lane-to-bank static test: passed.
  • MemUnit, PHI, Grant, constant queue, control memory, register bank, and register cluster regressions: 20 passed.
  • Python compilation and git diff --check: passed.

Branch note

This is intentionally a draft. The tested local validation baseline is ahead of the current remote zeonica-bicg-validation branch, so the PR history should be cleaned up or the base branch synchronized before final review.

tancheng and others added 30 commits April 15, 2026 10:30
Fixes two-cycles bug of NAH operation.
Fixes the bug of unexpected stall cycles during prologue.
…dividual CGRAs and add method to count valid tiles in ParamCGRA.
…ns for heterogeneous multi-CGRA architectures. Update CtrlMemDynamicRTL to correctly handle tile IDs in packet types.
…rite ports for tiles, enhancing support for heterogeneous multi-CGRA configurations.
[Feature] Support heterogeneous multi-CGRAs.
[Doc] Unify coordinate system of multi-cgra
…ks in single-CGRA mode, preventing MultiWriterError.
[Fix] Enhance CgraTemplateRTL to manage grounded ports for memory links in single-CGRA mode, preventing MultiWriterError.
[CleanUp][NFC] Standardize line endings to LF
tancheng and others added 29 commits July 14, 2026 08:43
Captures the non-obvious knowledge needed to work productively in this
repo: the custom pymtl3 fork setup, how to actually exercise Verilog
translation in tests (--test-verilog), RTLIR translator constraints,
the ctrl-step protocol and val/rdy conventions (including FUs that
snoop operands without handshakes), register-cluster token semantics,
test-suite timing expectations, and how to distinguish local toolchain
issues from real failures.
Per review: the content moves to AGENTS.md (the convention Codex and
other agents read) with a tool-neutral header; CLAUDE.md becomes an
@AGENTS.md import so Claude Code loads the same single source of truth.
Also documents the naming conventions observed in the codebase:
CamelCase classes/type names, snake_case functions and variables,
UPPER_SNAKE_CASE constants, and the kCamelCase attribute keys.
Add CLAUDE.md with development guidance for AI coding agents
…tion

# Conflicts:
#	fu/single/CompRTL.py
#	fu/single/test/CompRTL_test.py
#	lib/opt_type.py
#	noc/CrossbarRTL.py
Fixes #321. RegisterBankRTL previously had no notion of whether a
register held an unconsumed token: a valid input directly overwrote the
selected register, a configured read always asserted val (emitting
garbage for never-written entries), and nothing stopped a later
iteration from clobbering an earlier token whose consumer was delayed.

Each register entry now has a token-valid bit:
- Set when a token is written.
- A read asserts val only while the selected entry holds a token that
  the corresponding destination path has not yet accepted.
- Cleared only after every configured destination path completes a
  val/rdy handshake; for read_reg_towards=BOTH, per-path sticky
  taken bits let the FU and routing-crossbar paths accept in
  different cycles, and each path accepts the token at most once.
- A write to an entry that still holds a token is rejected, and the
  bank exposes outport_wr_rdy so the cluster backpressures the selected
  write source (unselected sources keep their always-ready behavior,
  preserving register-free configurations).

The write gate depends only on registered state (never on same-cycle
release), avoiding a combinational cycle through the FU, whose
recv_in.rdy depends on its send_out.rdy.

The bank's read path towards the routing crossbar is now a real val/rdy
handshake (send_data_to_xbar) instead of an unconditionally-asserted
val, so the direct reg->routing_crossbar path participates in token
release.

Existing tests are updated for the new semantics (no leading garbage
reads; each token delivered exactly once) and two new tests cover the
overwrite-protection and skewed BOTH-path-acceptance behaviors.
The initial token implementation gated every configured register read
on token presence, which starved existing kernels that intentionally
read never-written registers as an always-valid default-token source
(e.g. CgraRTL_test's INC kernels, documented as consuming data from
their own register cluster without anything writing it). Those kernels
deadlocked waiting for operands that never became valid, hanging CI.

Each register now tracks an "armed" bit, set on its first write and
kept until reset:
- A never-written register keeps the exact legacy behavior: a
  configured read always asserts val (default-token source), reads are
  repeatable, and nothing is tracked.
- Once armed, the full issue #321 token discipline applies: val only
  while a token is present, consume-once per destination path,
  release after all configured paths accept, and write backpressure
  while a token is unconsumed.

Legacy (unarmed) read acceptances do not update the per-path taken
bits, so they cannot poison the token bookkeeping of the first real
token.

The previously-failing kernels pass again (CgraRTL_test homogeneous
2x2 / ctrl_count_2 / king_mesh, CgraRTL_fir_test 4x4 terminate), and
the register-cluster tests keep their original leading default-value
reads, which are exactly the unarmed legacy phase.
test_multi_CGRA_fir_scalar_dynamic_migration hung (max_cycles timeout)
because the token/armed/taken bits survived a task switch: after a task
is terminated/migrated, the state its registers accumulated leaked into
the next task on that tile, whose reads then starved on armed-but-empty
registers instead of getting the legacy always-valid behavior a freshly
launched task expects.

The register bank now exposes a `clear` input, mirrored on the cluster,
which resets all token bookkeeping (register data itself is preserved).
TileWithContextSwitchRTL drives it from the same `clear` signal it
already feeds to the FUs, crossbars, and const mem on CMD_TERMINATE;
the plain and streaming tile variants tie it to 0 like their other
clear ports.
The third CI round exposed two FU usage patterns that break
handshake-based token release:
- VectorAllReduceRTL snoops its register-sourced base operand
  (recv_in[1]) without ever asserting rdy for ADD_BASE_GLOBAL, and
  needs it again cycles later when the global-reduce response arrives.
  With release tied to a val/rdy handshake that never happens, the
  token was never consumed and the next iteration's write deadlocked
  (test_multi_CGRA_fir_vector_global_reduce hung at max_cycles).
- Vector-factor replays fire the FU multiple times within one ctrl
  step, legitimately reading the same operand more than once.

Within a ctrl step, register reads are therefore level signals, as they
were before token tracking: consumers may accept or merely snoop them
any number of times. The token is consumed when the ctrl step that
reads the register completes, signaled by the new inport_ctrl_proceed
(the same per-step signal the const queue already advances on, wired
identically in all three tile variants). For read_reg_towards=BOTH,
the tile's existing done-tracking only lets the step complete after
both the FU and routing-crossbar paths have been served, which is
exactly the release-after-all-destinations condition; the per-path
sticky taken bits are no longer needed and are removed.

Armed/unarmed semantics, write backpressure while a token is
unconsumed, and clear-on-task-switch are unchanged.

Also ties off the register cluster's send_data_to_routing_crossbar in
TileWithStreamingLoadRTL, which has no direct register-to-routing-
crossbar path; the interface's rdy is genuinely read now, so leaving
it undriven failed elaboration (CgraWithStreamingLoadRTL tests).

The cluster unit tests emulate the tile's per-step done-tracking to
drive inport_ctrl_proceed where token consumption matters; harnesses
without ctrl stepping tie it low (tokens held, level reads).
The comment on the FU-path priority mux still described the removed
per-path taken-bit behavior; it now explains that the bank's send
interfaces are direction-gated internally, which is why the cluster
needs no additional read_reg_towards check. The kReadTowards* local
aliases lost their last use when that check moved into the bank.
Symmetric with the send_data_to_xbar read interface introduced for the
routing-crossbar path, and consistent with the cluster's own
send_data_to_fu port naming.
Under token discipline, an operation that reads and writes the same
register within one ctrl step (an accumulator, or #281's
`NOT $0 -> $0, SOUTH` under backpressure) deadlocked: the write was
rejected while the operand token was unconsumed, and the step could
not complete until the write was delivered.

Each bank now has a one-entry write skid buffer (one write port ->
at most one blocked write). A write is accepted whenever the skid is
free (outport_wr_rdy = ~skid_valid, pure registered state, so the
producer's rdy never combinationally depends on any consumer's
readiness and never collides with a commit on the single write port):
- It lands directly if that cannot disturb an in-flight read (target
  token-free and not read by the current step), or at the boundary of
  the reading step (the write's token atomically replaces the consumed
  one; set wins over clear in the token update).
- Otherwise it parks in the skid and commits once its target is no
  longer read by an open step: at that step's completion pulse, or
  immediately if the target is not being read and holds no token.

The stability invariant — a register being read holds its value until
the step completes — also fixes the partial-multicast corruption from
issue #281 (a speculative register write landing while another fan-out
leg is backpressured, followed by re-execution on the changed value),
which the previous direct-write-if-token-free behavior still allowed.

Tests:
- test_reg_cluster_write_skid_buffer: three back-to-back writes with a
  stalled consumer flow through park/commit in order.
- test_tile_same_register_accumulate: single-ctrl-word INC accumulator
  through one register with a stalled tile outport (the #281 shape);
  deadlocks without the skid buffer (verified), completes with it.
- The cluster test harnesses now always emulate the tile's per-step
  done-tracking to drive inport_ctrl_proceed, since commit timing is
  tied to step completion.
@n0thingNoob

Copy link
Copy Markdown
Collaborator Author

There are too many modifications in RTL files. We can review it later after the paper submission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants