fix(vk): handle RDNA 1 wave32 and wave64 shader stages #1657

Merged
brunobaccari merged 1 commit from fix/rdna1-wave32-wave64 into main 2026-11-11 01:16:46 +00:00
brunobaccari commented 2026-11-10 12:34:08 +00:00 (Migrated from github.com)

Unpinned SPIR-V 1.6 stages may use a subgroup wider than the device's default, so the translator must use the supported maximum to preserve 32-lane guest warp boundaries. Quad-only fragment operations must also keep their existing path instead of inheriting AMD's fragment shuffle broadcast workaround.

Rebased onto f8efe057 (current main). Main has since incorporated the stage-specific profiles and AMD fragment workaround from #1507; this PR preserves that implementation and carries only the remaining changes:

  • Use subgroup_max for the unpinned base profile, including devices whose default is 32 but maximum is 64. Existing compute and graphics stage pinning stays intact.
  • Keep quad broadcast emulation separate from the fragment shuffle workaround; INDEX/UP/DOWN/BFLY retain their existing behavior.
  • Include the presence of RX_VK_WARP64 in the cache stamp, since an explicit override can select a different graphics profile even when the base width is unchanged. This invalidates incompatible translated-program caches.
  • Adapt the regression test to the current per-test RUN_ALL_ARGS declarations, with 11 isolated device/override scenarios. Preserve the existing AMD fragment test.

The subgroup rule follows the Vulkan specification. Device selection remains capability-based, covering the RX 5500 XT / RX 5600 XT / RX 5700 XT family without a name allowlist.

Validation for this rebase:

  • Full Windows game-free runner: 320 passed, 8 failed, 25 skipped. Six failures reproduced on clean f8efe057 with the same toolchain: FPX, recorder sync, display startup fixture, precompile compute guard, NUP symlink privileges and missing zstd for bake_ds.
  • The other two failures were transient Windows locks on fixture executables (screensaver and temporal history); both passed on the base and on retest. The two AMD checks and these two checks passed together with --strict on final commit 363038d: 4 passed, 0 failed, 0 skipped.
  • Independent diff review and git diff --check passed. The six rebase conflicts are resolved. No new game binary or hardware run is claimed for this rebased revision.

Related: #1001; also relevant to the broader #996, #1000 and #995 trackers, without claiming to resolve them in full. Memory retention, pipeline scheduling, resolution, image quality and frame generation are outside this PR.

Historical hardware context: an earlier local experimental build showed 20 FPS in Link's house on the RX 5500 XT at 2560x1440, 100% scale, without upscaling or frame generation. The screenshot below is not a benchmark of this rebased revision. RX 5600 XT, RX 5700 XT and Linux/RADV still need hardware testing.

image

Credits: follows Robin Irfan's AMD warp work and butterfly/quad correction, including the Lichii #273 fragment workaround credited to jonmakegames. This PR carries only the remaining subgroup/fragment pieces needed on current main.

Unpinned SPIR-V 1.6 stages may use a subgroup wider than the device's default, so the translator must use the supported maximum to preserve 32-lane guest warp boundaries. Quad-only fragment operations must also keep their existing path instead of inheriting AMD's fragment shuffle broadcast workaround. Rebased onto `f8efe057` (current `main`). Main has since incorporated the stage-specific profiles and AMD fragment workaround from #1507; this PR preserves that implementation and carries only the remaining changes: - Use `subgroup_max` for the unpinned base profile, including devices whose default is 32 but maximum is 64. Existing compute and graphics stage pinning stays intact. - Keep quad broadcast emulation separate from the fragment shuffle workaround; INDEX/UP/DOWN/BFLY retain their existing behavior. - Include the presence of `RX_VK_WARP64` in the cache stamp, since an explicit override can select a different graphics profile even when the base width is unchanged. This invalidates incompatible translated-program caches. - Adapt the regression test to the current per-test `RUN_ALL_ARGS` declarations, with 11 isolated device/override scenarios. Preserve the existing AMD fragment test. The subgroup rule follows the [Vulkan specification](https://docs.vulkan.org/spec/latest/chapters/shaders.html#shaders-varying-subgroups). Device selection remains capability-based, covering the RX 5500 XT / RX 5600 XT / RX 5700 XT family without a name allowlist. Validation for this rebase: - Full Windows game-free runner: 320 passed, 8 failed, 25 skipped. Six failures reproduced on clean `f8efe057` with the same toolchain: FPX, recorder sync, display startup fixture, precompile compute guard, NUP symlink privileges and missing zstd for bake_ds. - The other two failures were transient Windows locks on fixture executables (screensaver and temporal history); both passed on the base and on retest. The two AMD checks and these two checks passed together with `--strict` on final commit `363038d`: 4 passed, 0 failed, 0 skipped. - Independent diff review and `git diff --check` passed. The six rebase conflicts are resolved. No new game binary or hardware run is claimed for this rebased revision. Related: #1001; also relevant to the broader #996, #1000 and #995 trackers, without claiming to resolve them in full. Memory retention, pipeline scheduling, resolution, image quality and frame generation are outside this PR. Historical hardware context: an earlier local experimental build showed 20 FPS in Link's house on the RX 5500 XT at 2560x1440, 100% scale, without upscaling or frame generation. The screenshot below is not a benchmark of this rebased revision. RX 5600 XT, RX 5700 XT and Linux/RADV still need hardware testing. <img width="1280" height="738" alt="image" src="https://github.com/user-attachments/assets/65df0aa7-d17e-4434-aff6-abf3a22e93b9" /> Credits: follows Robin Irfan's [AMD warp work](https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/b17e373cef65b16a88909519866905bba0a97e3d) and [butterfly/quad correction](https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/5c2004f50251bf68aeff1fdb90c2114376be35b1), including the [Lichii #273](https://github.com/zortos293/Lichii/pull/273) fragment workaround credited to [jonmakegames](https://github.com/jonmakegames/Lichii/commit/c179a360364acd3e6e7ceea8c706118184aa7ee2). This PR carries only the remaining subgroup/fragment pieces needed on current main.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 16:40:16 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. Read through the profile plumbing: make_profile(stage) now takes the stage, the old static profile() has no remaining callers, and every EmitSPIRV call site passes the stage that matches the pipeline stage it ends up in (compute for the task/mesh fallbacks). Using the same sg32_stages mask for the profile and the pipeline pin keeps the two in step, and on 32-lane-only devices (NVIDIA) the result is unchanged. The disk stamp now includes subgroup width, pinned stages and the workaround flag, so old caches won't be reused. Quad broadcast keeping the old path is consistent with the description, and the test is registered in run_all.py.

Nit: the new line in EmitShuffleIndex and the ternary in EmitShuffleButterfly run past the file's usual line width.

No CI ran on the branch, and the RX 5600 XT / 5700 XT / RADV runs are still untested, as the description says. Nothing here blocks merging.

Looks good. Read through the profile plumbing: make_profile(stage) now takes the stage, the old static profile() has no remaining callers, and every EmitSPIRV call site passes the stage that matches the pipeline stage it ends up in (compute for the task/mesh fallbacks). Using the same sg32_stages mask for the profile and the pipeline pin keeps the two in step, and on 32-lane-only devices (NVIDIA) the result is unchanged. The disk stamp now includes subgroup width, pinned stages and the workaround flag, so old caches won't be reused. Quad broadcast keeping the old path is consistent with the description, and the test is registered in run_all.py. Nit: the new line in EmitShuffleIndex and the ternary in EmitShuffleButterfly run past the file's usual line width. No CI ran on the branch, and the RX 5600 XT / 5700 XT / RADV runs are still untested, as the description says. Nothing here blocks merging.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 16:40:31 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. Read through the profile plumbing: make_profile(stage) now takes the stage, the old static profile() has no remaining callers, and every EmitSPIRV call site passes the stage that matches the pipeline stage it ends up in (compute for the task/mesh fallbacks). Using the same sg32_stages mask for the profile and the pipeline pin keeps the two in step, and on 32-lane-only devices (NVIDIA) the result is unchanged. The disk stamp now includes subgroup width, pinned stages and the workaround flag, so old caches won't be reused. Quad broadcast keeping the old path is consistent with the description, and the test is registered in run_all.py.

Nit: the new line in EmitShuffleIndex and the ternary in EmitShuffleButterfly run past the file's usual line width.

No CI ran on the branch, and the RX 5600 XT / 5700 XT / RADV runs are still untested, as the description says. Nothing here blocks merging.

Looks good. Read through the profile plumbing: make_profile(stage) now takes the stage, the old static profile() has no remaining callers, and every EmitSPIRV call site passes the stage that matches the pipeline stage it ends up in (compute for the task/mesh fallbacks). Using the same sg32_stages mask for the profile and the pipeline pin keeps the two in step, and on 32-lane-only devices (NVIDIA) the result is unchanged. The disk stamp now includes subgroup width, pinned stages and the workaround flag, so old caches won't be reused. Quad broadcast keeping the old path is consistent with the description, and the test is registered in run_all.py. Nit: the new line in EmitShuffleIndex and the ternary in EmitShuffleButterfly run past the file's usual line width. No CI ran on the branch, and the RX 5600 XT / 5700 XT / RADV runs are still untested, as the description says. Nothing here blocks merging.
hckrman101 commented 2026-11-10 17:50:09 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
brunobaccari commented 2026-11-11 01:15:39 +00:00 (Migrated from github.com)

@hckrman101 rebased

@hckrman101 rebased
Commenting is not possible because the repository is archived.
No description provided.