Bug/gbar response phase - #403
Open
RunjiaChen wants to merge 2 commits into
Open
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>
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.
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.