Bug/gbar response phase - #403
Merged
Merged
Conversation
… advance
vx_barrier.h documents gbarrier::arrive() as returning "phase (current
generation number)" and gbarrier::wait(phase) as blocking "until generation >
phase". Both statements only hold if a completed global-barrier generation
actually advances the phase that arrive() reports. On RTL it never does.
- RTL hw/rtl/core/VX_bar_unit.sv:165-170 handles the cluster response with
phase_n = next_phase, and :186 store_write = req_valid ||
gbar_bus_if.rsp_valid commits it at the address latched by :238
store_waddr <= store_raddr. A response is only accepted while no
barrier op is in execute (:57 gbar_rsp_ready = ~req_valid), so the
previous cycle's read_addr was VX_wctl_unit.sv:177's fallback
txbar_bus_if.data.addr, which is tied 'x with DXA off
(VX_sfu_unit.sv:222-223). The flip therefore lands on an x-derived
slot instead of the barrier's own slot.
- SimX sim/simx/barrier_unit.cpp:152-166 global_resume() increments the
phase of the same entry get_phase() reads.
So the global barrier's own phase never moves on RTL. A SYNC global barrier
still works, because its release is an unlock (VX_bar_unit.sv:167-168) and
never consults the phase -- which is exactly why nothing else notices. But an
async global arrive/wait pair can never be released by its own barrier, and
any code treating the returned value as a generation number reads a constant.
gbar_phase samples the probe barrier's phase, runs a full generation on it,
rendezvouses the cores on a SECOND global barrier (a different id, hence a
different gbar row and a different per-core slot), then samples again, and
asserts the DOCUMENTED contract that the phase advanced. Sampling uses
arrive() rather than wait() deliberately: wait() would block forever, turning
a data mismatch into a timeout with no diagnostics. Results are grouped per
core, since the flip can land on one core's slot by coincidence while every
other core is stuck.
The existing coverage cannot see it: tests/regression/async_gbarrier
self-skips when num_cores < 2 (its main.cpp:80), VX_config.toml:5 defaults
VX_CFG_NUM_CORES to 1, and ci/testcases/regression.yaml carries no multi-core
shape for it, so the async-global path is never exercised at all.
Measured (2 cores, 4 warps, 4 threads):
./ci/blackbox.sh --cores=2 --driver=simx --app=gbar_phase PASSED 0 -> 1
./ci/blackbox.sh --cores=2 --driver=rtlsim --app=gbar_phase FAILED 0 -> 0
core phase@N phase@N+1 advanced (rtlsim)
0 0 0 NO
1 0 0 NO
The test's Makefile defaults CONFIGS to -DVX_CFG_NUM_CORES=2, because the
global barrier only exists when a cluster holds more than one core
(VX_bar_unit.sv:44, USE_GBAR = VX_CFG_NUM_CORES > 1).
This change is purely additive: no existing file is modified.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tinebp
pushed a commit
that referenced
this pull request
Sep 10, 2026
Two barrier phase hazards in RTL, and the SimX phase representation that hid one of them. The response path reused `store_waddr` and `store_phase_wdata` from the request pipeline while retiring an independent global response. Under overlap or backpressure that updated the wrong barrier slot, corrupted state, or lost a completed request behind the single request register. The unlock used `active_warps`, which could release scheduler-stalled warps that had never waited on that barrier. Track completion per barrier id instead: queue completed local arrivals in a request FIFO, apply each response to the id it names, retain the phase each pending generation observed, and wake only the warps recorded as waiting on it. Global responses no longer write the local barrier state RAM. Separately, the working phase register forwarded every phase write without checking that the write address matched the barrier being processed, so back-to-back arrivals on different slots consumed the previous slot's phase. Qualify the bypass by address. Dropping `mask_n = wait_mask` from the global wait path is part of the same correction: completion is decided by arrivals, and a wait could previously satisfy `wait_mask == active_warps` a generation early. On the SimX side the barrier phase was an unbounded counter, but `wctl_unit.cpp` masks it to one bit before handing it to software. So `BarrierUnit::wait()` compared an unmasked counter against a masked token, and from the third generation on `barrier.phase == phase` was never true — `wait()` stopped blocking entirely. Toggling one bit makes the internal state match both the architectural phase and the RTL. Verified on a clean worktree at bf1831c, rv32, 2 cores / 4 warps / 4 threads. `bar_slot_phase` over 256 rounds went from 512 errors to 0 on rtlsim, `gbar_phase` from a stuck 0 -> 0 to a correct advance, and `async_gbarrier` passes with its overlap window widened from 32 to 1024 iterations. Both reproducers already passed on simx before the change and still do. perf_gate was run because the local barrier phase path changed and sgemm-mc is multi-core: 49 passed, no cycle or retired-count movement. Both reproducers are registered in the regression catalog by this change, so they now run on every CI sweep rather than sitting dormant as they have since #402 and #403 landed. Two follow-ups are left open. `sw/kernel/include/vx_barrier.h` still documents `arrive()` as returning a generation number and `wait()` as blocking until "generation > phase", which the one-bit phase has never satisfied. And the new request FIFO has no matching SimX latency model; no model_parity case exercises multi-core global barriers, so the gate cannot observe the gap today. Fixes #400 Fixes #401 Co-Authored-By: symmetryyyyy <chengxuan99@ucla.edu> Co-Authored-By: RunjiaChen <runjia@u.nus.edu> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
tests/regression:
gbar_phase— global barrier does not advance its phaseReproducer test only — this PR does not change the RTL. Full analysis and a
suggested fix direction are in #401.
Summary
A completed global-barrier generation does not advance its phase on the RTL.
gbarrier::arrive()is documented to return the "phase (current generationnumber)" (
sw/kernel/include/vx_barrier.h:69), andgbarrier::wait()blocksuntil it changes — so two
arrive()calls either side of a completedgeneration must return different values. SimX advances it
(
sim/simx/barrier_unit.cpp:126); the RTL returns the same number on everycore.
Reproduction
PASSED!FAILED!Deterministic, both cores, every run. Two cores are the minimum for the global
barrier to be instantiated (
USE_GBAR = VX_CFG_NUM_CORES > 1); the Makefiledefaults
-DVX_CFG_NUM_CORES=2and the host program skips on a 1-core device.Why the existing
async_gbarriertest does not catch thisIt passes on both drivers today — on timing, not on the phase. Its kernel runs
32 loop iterations between
arrive()andwait(), short enough that every warpis already parked when the global response lands, and the response releases
warps unconditionally rather than by phase. Raising that one constant to 2000
makes the same test deadlock on rtlsim (
active_warps=1111, stalled_warps=1111) while still passing on simx. Details in #401.What this PR contains
One new directory,
tests/regression/gbar_phase/(4 files, +269 lines).No existing file is modified. The test samples the phase with
arrive()rather than blocking in
wait(), so the kernel always terminates and the defectis reported as data instead of a timeout; results are printed per core, since a
global barrier is a cluster object whose phase lives in per-core state.
It is deliberately not added to the
tests/regression/Makefileapp list, soCI is unaffected by a test expected to fail until a fix lands. Happy to wire it
in as a regression guard once one does.
Environment
Based on
5d62846c6. Also present byte-identical atd76b7f24e— between thosecommits exactly one commit touches
VX_bar_unit.sv(90a9b186b), whose entirediff for that file is one word inside a comment, and
VX_gbar_unit.svisuntouched.