tests/regression: bar_slot_phase — expose cross-slot barrier phase co… - #402
Open
RunjiaChen wants to merge 1 commit into
Open
tests/regression: bar_slot_phase — expose cross-slot barrier phase co…#402RunjiaChen wants to merge 1 commit into
RunjiaChen wants to merge 1 commit into
Conversation
…rruption
A count-1 barrier arrival completes its generation immediately, so it must
advance that slot's phase: the next arrival on the same slot must observe the
complement. Barrier slots are independent state, so a concurrent arrival on a
DIFFERENT slot must not disturb it. On RTL it does.
hw/rtl/core/VX_bar_unit.sv:236
phase_r <= store_write ? store_phase_wdata : store_phase_rdata_v;
reloads the working phase from whatever was just written, without checking
that the write targeted the slot now being read -- unlike the rd-return
path, which IS address-qualified (:224 is_rdw_hazard, :243 phase_async).
The phase store is the async-read, write-first VX_dp_ram, so when two
barrier requests for DIFFERENT slots are processed on consecutive cycles,
the second one's phase compare, flip and write-back all use the FIRST
slot's just-written phase. Its own slot is left unchanged. Requests can be
back-to-back: VX_wctl_unit's wctl_reg is a plain one-per-cycle pipe with
enable=1, and nothing serialises different warps' BAR-class ops.
SimX keeps a phase per slot (sim/simx/barrier_unit.cpp) and is unaffected.
Only the async barrier API can see this. A sync vx.bar releases on the
arrival count and never compares the phase, so the corrupted bit is invisible
there; it is the phase token handed out by barrier::arrive() and consumed by
barrier::wait() that goes stale, which can park a warp forever or release it
a generation early.
bar_slot_phase releases warp 0 and warp 1 together on a gate barrier so the
scheduler issues their arrivals on consecutive cycles, on two different
slots, and has each warp audit its OWN slot: pre = arrive(), post = arrive(),
and post must be the complement of pre. Two details make the observation
stable: the two probe slots must hold EQUAL phases for the cross-slot
interaction to be observable, so a detected miss is followed by one extra
arrival to restore the parity for the next round; and `pre` and `post` are
separated by nops so the audit read is never itself the back-to-back partner
of the arrival it is auditing.
The existing coverage cannot see it: tests/regression/async_barrier uses one
barrier id per phase of its pipeline and never puts two different slots in
flight in the same cycle window, so every barrier request it issues is
either isolated or targets the slot that was just written.
Measured on upstream master @ 5d62846 (clean tree, cold build)
(2 cores, 4 warps, 4 threads, num_barriers=8, 256 rounds):
./ci/blackbox.sh --cores=2 --driver=simx --app=bar_slot_phase PASSED
./ci/blackbox.sh --cores=2 --driver=rtlsim --app=bar_slot_phase FAILED
core warp missed-flips first(pre,post) (rtlsim)
0 0 0 (0,0) <- issues first, unaffected
0 1 256 (0,0) <- issues one cycle later, other slot
1 0 0 (0,0)
1 1 256 (0,0)
512 errors: every one of the 256 audited rounds on the second-issuing warp,
on both cores. The stock-config simx arm must pass; that is what proves the
audit itself is sound.
This change is purely additive: no existing file is modified, and
bar_slot_phase is deliberately NOT added to the TESTS list in
tests/regression/Makefile, so CI is unaffected by a test that is
expected to fail until a fix lands.
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.
Adds a regression test for #400 —
VX_bar_unitreloading its working phase register from a write to a different barrier slot.The test releases two warps together on a gate barrier so their arrivals issue on consecutive cycles, puts each warp on its own count-1 barrier so every arrival must flip its slot's phase, and has each warp audit its own slot:
pre = arrive(); post = arrive();wherepostmust be the complement ofpre. Binding both warps to the same slot makes it pass on both drivers — that is the case where the missing address check does not change the result.On
5d62846c6:PASSED!FAILED!(exit 2)512 = every one of the 256 audited rounds on the second-issuing warp, on both cores; deterministic. With the one-line fix proposed in #400 applied, rtlsim passes and retires 27096 instructions — exactly SimX's count.
Purely additive: one new directory, no existing file modified.
bar_slot_phaseis deliberately not added to theTESTSlist intests/regression/Makefile, so CI is unaffected by a test that is expected to fail until a fix lands. Happy to wire it in as a regression guard once one does.