Repository navigation
fix(shaders): a subgroup barrier between a warp's shared memory store and its next load - #2524
Conversation
… and its next load
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a careful one: the dumped SPIR-V shape, a test that fails on main, and on/off numbers from the same DLL make it easy to follow. The change matches the LDSM barrier already on main (same MEMBAR bit 20 → SubgroupBarrier). The off switch is under RX_VK_SHARED, so it is in the program stamp. Old cached programs get rebuilt because the translator sources feed HOOKSHOT_SHADER_STAMP. The test declares RUN_ALL_ARGS and passes on both platforms in CI, and the head already includes #2443's change to this file, so there is no conflict there.
Two notes, neither blocking:
- Divergence. The check covers the LDS's own predicate but not an enclosing branch. An unpredicated LDS inside a divergent
if(after an STS) would get anOpControlBarrier Subgroupin non-uniform control flow. LDSM doesn't have this problem because it is warp-collective. This network is fine (you checked that the barrier lands next to its workgroup barriers), but the rule applies to every program. If you can, log how many programs it fires in during a run (or note whether any of them have a BRA between the STS and the LDS). That would show how far it reaches beyond this network, which matters more since AMD, Intel and Linux are untested. - Loops (nit). The pending flag follows instruction order, so a loop that loads at the top what the previous iteration stored at the bottom gets no barrier. That's no worse than main, just a case this does not cover. A word in the comment would help whoever looks at #2477.
Approving: CI is green and the fix is narrow, with a switch to turn it off.
| static const bool lds_barrier = !env_on("RX_VK_SHARED_NO_LDS_BARRIER"); | ||
| if (opcode == 0x388) { | ||
| shared_store_pending = true; | ||
| } else if (shared_store_pending && lds_barrier && native_predicate == 7) { |
There was a problem hiding this comment.
native_predicate == 7 covers this instruction's predicate, but not a divergent branch around it. An LDS in an if taken by some lanes still gets the subgroup barrier. Fine for fd029a317a9a4b94; see the review note about checking the other programs it fires in.
…s next load On NVIDIA's native cooperative-matrix path the game's own upscaler below 100% Render resolution draws flickering bright bars, 16 px wide and 2 rows tall, in different places every frame (issue 2455). Each bar's origin: OoT's programs store to shared memory and read it back, other lanes' words included, with no BAR between. The Switch runs a warp's shared memory accesses in order; Vulkan orders another lane's store and this lane's load only through a barrier, and the driver's compiler may move them past each other. The 4/3 gameplay network fd029a317a9a4b94 ends with 8 STS then 16 LDS and nothing between them. After an STS, the next LDS now gets the subgroup barrier LDSM's path already uses (MEMBAR, bit 20), only where the LDS is not predicated so every lane of the warp reaches it. A BAR.SYNC clears the pending store. RX_VK_SHARED_NO_LDS_BARRIER=1 (under RX_VK_SHARED, so in translator_stamp) turns it off as before. lower_lds_after_sts_test lowers STS/LDS/BAR sequences against the real header: a barrier between a store and its next load, one per store, none after a BAR.SYNC, none before a predicated LDS, none with the switch. It fails on the old header. Ported from upstream dd2369e (NotRexed#2524). NVIDIA cooperative-matrix hardware not available here: the lowering is checked, the bars in game are unverified.
On NVIDIA's native cooperative-matrix path, the game's own upscaler below 100% Render resolution (Auto included) draws flickering bright bars. Each bar is 16 px wide and 2 rows tall, at
x % 16 == 0andy % 8 in {0, 1}of the output, in different places every frame (#2455, tracked in #2511). OoT's programs store to shared memory and read it back, other lanes' words included, with no BAR between: the Switch runs a warp's shared memory accesses in order. Vulkan orders another lane's store and this lane's load only through a barrier.Cause
The 4/3 gameplay network
fd029a317a9a4b94ends with 8 STS and then 16 LDS (one per output pixel it writes) with no barrier between them. I dumped its SPIR-V withRX_VK_DUMP_SPV. The main function's shared memory accesses, barriers and matrix ops in program order:The last
S8 L16has nothing between them, so the driver's compiler is free to move the loads before other lanes' stores. That is the same mechanism as LDSM's (lower_to_maxwell.h, nxnative's "LDSM reads rows other lanes stored"), but with a plain LDS. It shows on the cooperative-matrix path. On the software HMMA path (subgroup shuffles everywhere) it did not show in my runs.Change
lower_to_maxwell.h, STS/LDS (0x388 / 0x984): after an STS, the next LDS gets a subgroup barrier first (MEMBAR bit 20,SubgroupBarrier, as LDSM's), but only when the LDS is unpredicated, so every lane of the warp reaches it. A BAR.SYNC clears the pending store. In this network it adds exactly oneOpControlBarrier Subgroupbetween theS8and theL16, in the same block as the program's workgroup barriers (so in uniform control flow).RX_VK_SHARED_NO_LDS_BARRIER=1turns it off, as before. It is under theRX_VK_SHAREDprefix, so it is intranslator_stamp(test_program_stamp_knobspasses).tests/test_lower_lds_after_sts.pylowers STS/LDS/BAR sequences against the real header: a barrier between a store and its next load, one per store, none after a BAR.SYNC, none before a predicated LDS, none with the switch. It fails on main (FAIL: count(one, 'B') == 1).Tested
RTX 2060 SUPER (Turing), driver 610.62, Windows 11. Kokiri Forest from a save, in a separate copy of the game folder. The game was driven through the control bridge (headless,
option_set, 10 shots at 2048x1152 per setting). The bars were counted by a detector: 16x2, brighter than the rows above and below, ending sharply on both sides, counting only places seen in at most half of the shots. The same patchedhookshot_vk.dll(built withRX_VK_ONLY=1on b744b1e, the exe's API version) was used for both the on and off runs, each from a fresh program cache. Every run's log has the network's dispatch (and its pipeline compiled) before the shots.RX_VK_SHARED_NO_LDS_BARRIER=1fd029a317a9a4b94)6676619641320ba7)For reference on main: the 75% stand-in (
RX_VK_TAAU_KEYS=fd029a317a9a4b94) gave 0, and AMD FSR 3.1 at 75% gave 1.GPU time a frame at 75% (the log's
GPU time, median of the last 12 lines): 12.8 ms on main, 12.3 ms with this change, 12.3 ms with it switched off. No measurable cost.tests/run_all.py(Windows, llvm-mingw 20261104): everything passes excepttest_vram_budget_row, which fails the same way without this change (-Werror,-Wunused-const-variableonkVramMinimumOnly, from #1859).Not tested
Fixes #2455 (part of #2511).
[Investigated with the help of an AI agent (Hermes) on my machine; I reviewed the change and the numbers.]