Skip to content

Fix simx flush arbiter - #391

Closed
ATWeatherly wants to merge 2 commits into
vortexgpgpu:masterfrom
ATWeatherly:fix/simx-flush-arbiter
Closed

Fix simx flush arbiter#391
ATWeatherly wants to merge 2 commits into
vortexgpgpu:masterfrom
ATWeatherly:fix/simx-flush-arbiter

Conversation

@ATWeatherly

Copy link
Copy Markdown
Contributor

some bug fixes that arose when testing VM work

ATWeatherly and others added 2 commits August 27, 2026 19:05
TFifo::empty() reports only entries that have emerged from the pipe's
latency window (SimChannel::queue_empty ignores pending_count_), so
processFlush()'s drain guard could not see a replayed store pushed into
pipe_req_ earlier in the same tick. The walk then swept the store's set
while its line was still clean and finished before the write applied,
losing the store on an end-of-kernel flush. Observed with vm_stress on
simx/XLEN=32 once the VM MMU model added translation latency, but the
window is generic to any replay that lands as the flush starts; size()
counts in-flight entries and closes it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TxArbiter and TxRxArbiter sized their per-output request group with
log2ceil(num_inputs / num_outputs); when the input count is not a multiple
of the output count that rounds down and the trailing inputs are never
polled (VX_stream_arb uses CDIV). Callers had to pad input counts up to a
power-of-two multiple to work around it. Also adds the CHANGELOG entry for
the cache flush-walk fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tinebp tinebp closed this in a3e7bf2 Aug 30, 2026
@tinebp

tinebp commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Reviewed and accepted — thank you, this is a real bug and the diagnosis was correct.

Verification

Reproduced on master before the fix. The shape that breaks is any request count that is not a
multiple of the output count with more than one output:

./ci/blackbox.sh --driver=simx --app=demo --cores=3 \
  --perf=0 CONFIGS="-DVX_CFG_SOCKET_SIZE=3 -DVX_CFG_NUM_ICACHES=2"

3 cores over 2 shared icaches → log2ceil(3/2) = log2ceil(1) = 0, so each output arbitrates
over a single input and the third core's fetch port is never polled. The run hangs at boot
(timeout, rc=124). With log2ceil(CDIV(3,2)) = 1 it passes. VX_stream_arb has always sized its
groups by ceiling division, so this was a straight SimX/RTL model divergence.

Why CI missed it

Every catalog case that shares an icache used a power-of-two core:cache ratio, so
inputs % outputs == 0 held everywhere and the floor/ceiling distinction never showed. I added
one case in the cache category that pins the non-power-of-two shape:

- id: demo-shared-icache-odd
  via: blackbox
  drivers: [simx]
  app: demo
  configs: -DVX_CFG_SOCKET_SIZE=3 -DVX_CFG_NUM_ICACHES=2
  shape: { cores: 3 }

Validated both ways: it hangs under floor grouping and passes under ceiling grouping.

What landed

a3e7bf222 on master, authored to you, with the arbiter fix, the catalog case, and a
CHANGELOG entry. The full tests/regression suite and the kernel conform suite stay green on
simx and rtlsim.

The other half of this PR — the cache-flush walk gating on size() rather than TFifo::empty()
had already landed independently via #394, which is why this branch conflicted with master and
could not be updated in place. Only the arbiter half was rebased forward. Closing here since the
work is merged; the commit carries your authorship.

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.

2 participants