Skip to content

fix(shader): declare the GDS buffer coherent - #4279

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
karanvora2599:fix/shader-coherent-gds
Oct 13, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
karanvora2599:fix/shader-coherent-gds

Conversation

@karanvora2599

Copy link
Copy Markdown
Contributor

What

The storage buffer that holds GDS is now declared Coherent.

Background.

  • GDS is one on-chip memory for the whole GPU on AMD hardware. Every work-group's ds_read/ds_write with gds sees the others' writes, with no cache in between.
  • The recompiler keeps GDS in a storage buffer (gds, SpirvEmitterHelpers.cpp). Every GDS access goes through it: reads, writes and atomics (SpirvBufferAccess.cpp, ResourceKind::Gds).
  • That buffer wasn't decorated Coherent. On a GPU whose L1 caches storage-buffer loads (NVIDIA's from Turing on, among others), a plain GDS read could therefore return a value cached before another work-group's GDS write.
  • Atomics weren't affected, since they're performed at the device.

Why results don't change. Coherent only stops loads and stores from being served by a non-coherent cache. A program without cross-work-group GDS traffic reads the same values. GDS traffic is small (counters and small tables), so the cost of going to L2 is negligible.

Tested

Windows 11, MinGW-w64 GCC 15.2.0, Release, this branch on main at d146ec1. Devices:

  • RTX 4060 Laptop (Ada, driver 591.86)

  • Intel UHD Graphics (Raptor Lake, driver 101.6790)

  • lavapipe from Mesa 25.2.8 (llvmpipe, LLVM 21.1.7): the version CI installs; Windows build with GALLIUM_OVERRIDE_CPU_CAPS=avx.

  • New verifyCoherentGds in agc_shader_memory:

    • It compiles a ds_read_b32 and a ds_write_b32 with gds and checks that the gds buffer is decorated Coherent.
    • It passes on all three devices.
    • With the decoration removed it fails: coherent GDS: the gds buffer is not Coherent.
  • agc_driver_gds_append_consume passes on the RTX 4060 and the Intel UHD. lavapipe skips it.

  • Shader and driver tests (agc_driver_*, agc_shader_*, agc_spirv_*, vector_*, agc_masked_select, agc_validate_program, agc_denormal*, agc_float16_*; 338 tests):

Limits.

  • I have no test that catches a stale GDS read on hardware. The failure needs two work-groups and a cached line, so the test only checks the declaration.
  • fma-rc's census of nine titles (perf(shader): compute v_dot4 with packed integer dot products #4061) doesn't record the gds bit, so I don't know how often titles read GDS with plain loads. ds_append (25) and ds_consume (2) appear, but they are atomics.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions github-actions Bot added shader core/shader agc libSceAgc* labels Oct 12, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
GDS is one memory for the whole GPU on AMD hardware: every work-group's
ds_read and ds_write with gds see the others' writes, with no cache in
between. The recompiler keeps it in a storage buffer that was not decorated
Coherent, so on a GPU whose L1 caches storage buffer loads, NVIDIA's from
Turing on among them, a plain GDS read could return a value cached before
another work-group's GDS write. Atomics were not affected. The gds buffer is
now declared Coherent.

verifyCoherentGds in agc_shader_memory compiles a ds_read_b32 and a
ds_write_b32 on GDS and checks that the gds buffer is Coherent.
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (only an adjacent-test conflict in ShaderMemory.cpp, no logic change). Checked: agc_shader_memory passes with the change and fails without it; the scoped ctest passes apart from agc_driver_tessellation, which segfaults here without this PR's change too. Merging once CI is green.

@adriadam10
adriadam10 force-pushed the fix/shader-coherent-gds branch from 5328f13 to 0998eb4 Compare October 13, 2026 13:54
@adriadam10
adriadam10 merged commit d071124 into boykopovar:main Oct 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* needs rebase Conflicts with main, rebase needed shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants