Barrier's phase update mechanism is wrong

[RunjiaChen]

Environment Setup

vortex: 5d62846

Bug Description

We noticed that VX_bar_unit reloads its working phase register from a write
to a different barrier slot, so a barrier arrival can end up flipping and
storing the wrong slot's phase.

Each barrier slot carries an independent 1-bit phase, and a completing arrival
must advance its own slot's phase and no other's. When two barrier requests
addressing different slots of one core are processed on consecutive cycles, the
second request's wait compare, phase flip and phase write-back all use the
first slot's phase; its own slot is left unchanged.

Only the async barrier API can observe 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.

Below is our regression test. Please place it in the tests/regression/
directory. Then execute:
./ci/blackbox.sh --cores=2 --app=bar_slot_phase --driver=rtlsim
We obtain:

CONFIGS: num_threads=4, num_warps=4, num_cores=2, num_clusters=1, socket_size=1, local_mem_base=0x1ffff0000, num_barriers=8
num_cores=2, num_warps=4, num_threads=4, rounds=256
allocate device memory
load kernel module
start device
download results
wait for completion

core  warp  missed-flips  first(pre,post)
  0     0        0            (0,0)
  0     1        256            (0,0)
    core 0 warp 1: SLOT PHASE error: 256 of 256 rounds saw a count-1 arrival fail to advance its own slot's phase (pre=0 post=0)
  1     0        0            (0,0)
  1     1        256            (0,0)
    core 1 warp 1: SLOT PHASE error: 256 of 256 rounds saw a count-1 arrival fail to advance its own slot's phase (pre=0 post=0)
slot phase errors: 512
cleanup
PERF: instrs=31704, cycles=87316, IPC=0.363
Found 512 errors!
FAILED!
make: *** [../common.mk:222: run-rtlsim] Error 1

Further cross-validation with simx:
./ci/blackbox.sh --cores=2 --app=bar_slot_phase --driver=simx
We obtain:

core  warp  missed-flips  first(pre,post)
  0     0        0            (0,0)
  0     1        0            (0,0)
  1     0        0            (0,0)
  1     1        0            (0,0)
slot phase errors: 0
cleanup
PERF: instrs=27096, cycles=65859, IPC=0.411
PASSED!

512 = every one of the 256 audited rounds on the second-issuing warp, on both
cores; deterministic, not intermittent. SimX keeps a phase per slot
(sim/simx/barrier_unit.cpp) and is unaffected, which is also what shows the
audit itself is sound.

The test releases warp 0 and warp 1 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 with pre = arrive(); post = arrive(); where post must be the
complement of pre. Binding both warps to the same slot makes it pass on both
drivers — that is the case where the missing check does not change the result.

Patch

We found that the root cause is hw/rtl/core/VX_bar_unit.sv:236, which
forwards the in-flight write into phase_r whenever any write happened,
without checking that the write targeted the slot now being read:

224:  wire is_rdw_hazard = store_write && (store_waddr == store_raddr);
236:  phase_r <= store_write   ? store_phase_wdata : store_phase_rdata_v;
243:  wire phase_async = is_rdw_hazard ? phase_n   : store_phase_rdata_v;

store_phase_wdata is phase_n (:185) and the else-arm is the same signal,
so :236 and :243 are the same read-during-write bypass — but only :243 is
address-qualified. store_waddr is store_raddr delayed one cycle (:238),
i.e. the slot of the request one stage ahead, so is_rdw_hazard is exactly the
right condition and :236 reaches past it for the weaker store_write.

The bypass is needed at all because the phase store is instantiated with
OUT_REG = 0, which in VX_dp_ram resolves to a bare
assign rdata = ram[raddr] (:288, :323) against a non-blocking write, so a
write issued this cycle is not visible on the read port until the next one.
(The instantiation also passes RADDR_REG(1), which looks like it registers
the read address — it does not; RADDR_REG is declared a "read address
registered hint" and is `UNUSED_PARAM at VX_dp_ram.sv:78,99.)

read_phase (:257) is driven from phase_async, the guarded path, so the
value written back to rd is correct. Only the stored state is corrupted,
which is why this stays invisible unless the slot is read a second time.

Note also that store_write = req_valid || gbar_bus_if.rsp_valid (:186), so
the writer need not be an instruction — a global-barrier response writes into
this port too.

The patch is as follows:

--- a/hw/rtl/core/VX_bar_unit.sv
+++ b/hw/rtl/core/VX_bar_unit.sv
@@ -233,7 +233,7 @@
             if (store_write) begin
                 store_valids[store_waddr] <= 1'b1;
             end
-            phase_r <= store_write ? store_phase_wdata : store_phase_rdata_v;
+            phase_r <= is_rdw_hazard ? store_phase_wdata : store_phase_rdata_v;
         end
         store_waddr <= store_raddr;
     end

With this one-line change applied to 5d62846c6, the same test passes on
both drivers:

  RESULT bar_slot_phase/simx   exit=0 verdict=PASSED!   slot phase errors: 0
     PERF: instrs=27096, cycles=65859, IPC=0.411
  RESULT bar_slot_phase/rtlsim exit=0 verdict=PASSED!   slot phase errors: 0
     PERF: instrs=27096, cycles=64764, IPC=0.418

The retired-instruction count is worth noting: rtlsim goes from 31704
(unpatched) to 27096, which is exactly SimX's count. The 4608 extra
instructions in the unpatched run are the test's own detection and
parity-restore path firing on 512 corrupted rounds; with the fix that path is
never taken and the two drivers retire the same instruction stream.

We have not evaluated timing or area impact, and we have not audited whether
the same omission exists on the DXA/txbar paths that share this port.

The test is on this branch, based directly on 5d62846c6:
https://github.com/RunjiaChen/vortex-visualiser/tree/bug/bar-slot-phase-v2
It adds one directory and modifies no existing file. It 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.

Also present, byte-identical, at d76b7f24e. Between the two commits exactly
one touches VX_bar_unit.sv — 90a9b186b, whose entire diff for that file is
one word inside a comment.