Skip to content

feat(shader): track MODE rounding changes through the control flow graph - #5180

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
semirkabir:feat/shader-mode-rounding-tracking
Oct 14, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
semirkabir:feat/shader-mode-rounding-tracking

Conversation

@semirkabir

Copy link
Copy Markdown
Contributor

What

Addresses #981 for immediate rounding writes. The recompiler threw on any s_round_mode or s_setreg_imm32_b32 that changed the MODE rounding fields from the shader's initial FLOAT_MODE. vkQuake (PPSA99010) stops there on Windows, main c13dc7e (iDiesIrae-Req on #1578 and #981, 7 of 9 runs on an RTX 4070, RT shadow path):

compute shader 0x20bf82000: shader 0x20bf82000 stage 0: ShaderRecompiler::Recompile: s_round_mode 8 at pc 316 changes the initial MODE rounding fields: runtime MODE changes are not implemented

ACO emits this shape: it sets a rounding mode around the instructions that need it and restores it afterwards (aco_insert_fp_mode.cpp, set_mode: s_round_mode on GFX10+, s_setreg_imm32_b32 before).

  • RoundingMode.cpp computes the rounding fields (MODE bits 0-3) at the entry of every block. It walks the control flow graph from the initial FLOAT_MODE, applies immediate writes, and joins the paths. Bits that differ between paths, or that are written from an SGPR (s_setreg_b32), become unknown.
  • Inside a block, each immediate write updates the mode the translator reads. The instructions that already read FLOAT_MODE rounding (v_cvt_f32_i32/u32, v_ldexp_f32, the f16 conversions, v_ldexp_f16 with f16 denormals kept, and the mix/interp/packed f16 instructions that check their own mode) follow the current value.
  • s_getreg_b32 of the rounding bits returns the tracked value.
  • Any other float instruction that runs while a directed rounding mode differs from the initial one throws at that instruction, not at the mode write. So does any use of unknown bits. A shader without FLOAT_MODE metadata still throws on any rounding change.
  • The debt entry in docs/dev/debt/shader.md is updated.

Tested

Linux (GCC), lavapipe, on current main e152ba8:

  • agc_shader_mode_register_tests cover:
    • every immediate write of the 4 rounding bits, read back through s_getreg_b32;
    • s_round_mode 0-15 from every initial rounding;
    • a write then restore before a use (accepted);
    • a directed f32 conversion after s_round_mode (follows the mode);
    • an unmodelled op under a changed mode (throws at the op);
    • SGPR writes (throw at the read);
    • paths that join with different modes (throw at the read or use);
    • shaders without float metadata (throw).
  • 112 shader, float, f16, rounding, conversion, interpolation, VOP and packed tests: 111 pass and 1 is skipped (agc_driver_f16_to_f32_unpack, which needs a feature lavapipe lacks).
  • tools/check_conventions.py --base origin/main passes.

Not tested on the title. Under lavapipe here, vkQuake stops earlier than this shader: SDL cannot open Vulkan on the offscreen driver, and the title's error path then faults in its own libunwind. So I can't show that it gets past pc 316. s_round_mode 8 sets f16/f64 rounding toward -inf. If the RT shader runs an f16/f64 instruction outside the modelled set in that mode, it will now throw at that instruction, with its pc, instead of at the s_round_mode. A run on a GPU would settle it.

Checklist

@semirkabir

Copy link
Copy Markdown
Contributor Author

@iDiesIrae-Req could you run vkQuake (PPSA99010) with this branch on your RTX 4070 setup, with RT shadows on (the default, no r_rtshadows 0)? Your runs on c13dc7e are the only evidence for the s_round_mode 8 at pc 316 stop, and I can't reach that shader here.

What I'd like to know is what happens after that point on this branch:

  • it plays past demo1's first seconds, or
  • it stops at a different line. If the RT shader runs an f16/f64 instruction that this PR doesn't model under a changed rounding mode, the throw now names that instruction and its pc instead of s_round_mode.

Either line settles whether this PR unblocks the title.

Why I couldn't check it myself: my box has no GPU. On lavapipe (Mesa 25.0.7) under Xvfb, on main e152ba8 plus this branch, vkQuake gets through Playing shareware version. and its Vulkan init. Then its own driver reports radv/ps5: the GPU has not reached submission 2 (at 1) in 10 s and QUAKE ERROR: vkWaitForFences failed with code -4, and it exits before any RT shader runs. AnyPS5 reports no failure there (checked with #5177 applied), so it looks like software rendering missing the title's 10 s fence wait rather than a driver error. That is a guess I can't confirm on this machine.

AI-assisted: yes

@iDiesIrae-Req

Copy link
Copy Markdown

@semirkabir Ran it. Setup: Windows 11, RTX 4070 Laptop (driver 32.0.16.1714), WinLibs GCC 15.2.0 r7 Release, vkQuake (PPSA99010) with the Quake 1.06 shareware id1/pak0.pak, default settings (RT shadows on, no autoexec.cfg). Each build used a fresh relink with empty shader caches, then 4 runs of 60 s. The stop lines are from run.log and the title's app0/trace.txt.

Build Result (4/4 runs each)
main e152ba8 (control) Aborts right after Playing demo from demo1.dem: compute shader 0x20bf82000: ... s_round_mode 8 at pc 316 changes the initial MODE rounding fields: runtime MODE changes are not implemented
this PR, 6b9fbeb Same place, new line: compute shader 0x20bf82000: shader 0x20bf82000 stage 0: ShaderRecompiler::Recompile: s_round_mode at pc 316 changes the rounding fields of a shader without a declared FLOAT_MODE: not implemented

So on this title the PR moves the stop but doesn't unblock it, because the shader reaches the recompiler with no FLOAT_MODE. The title's RADV port writes PM4 itself, so its compute shaders are never registered (Compute shader 0x20bf82000 was not registered; preparing its artifact at dispatch). RegisteredFloatMode only reads COMPUTE_PGM_RSRC1 from a registered state, which is why it returns nullopt here, and Driver::dispatch passes that on (Dispatch.cpp:99). The live COMPUTE_PGM_RSRC1 (0x212) is in queue.shader at that point; Dispatch.cpp:150 already mixes it into the dispatch key.

As a local experiment only (not proposing it as is), I fell back to the live register when the shader isn't registered, decoded the same way RegisteredFloatMode decodes type 0:

[&]() -> std::optional<ShaderRecompiler::ShaderFloatMode> {
    if (auto registered = RegisteredFloatMode(snapshot)) return registered;
    const auto found = queue.shader.find(0x212u);
    if (found == queue.shader.end()) return std::nullopt;
    const auto value = found->second;
    return ShaderRecompiler::ShaderFloatMode{(value >> 12u) & 0xffu, ((value >> 21u) & 1u) != 0u, ((value >> 23u) & 1u) != 0u, ((value >> 26u) & 1u) != 0u};
}()

With that on top of this PR, 4/4 runs no longer throw. 0x20bf82000 is recompiled and dispatched, with no f16/f64 instruction named, so nothing else in it needed modelling under the changed mode. The title then stalls on the RT submission: vkDeviceWaitIdle -> -4 and radv/ps5: the GPU has not reached submission 18 (at 17) in 10 s, stuck on the loading screen at 12 presented frames. That is the same stall I saw on c13dc7e in runs that never reached the shader compile. So with a float mode for unregistered compute shaders, this PR gets vkQuake past the s_round_mode stop, and the next blocker is the GPU work of the RT shadow pass itself.

Happy to run another build here if useful.

AI-assisted: yes (Claude Code); all runs were on my machine.

@iDiesIrae-Req

Copy link
Copy Markdown

@semirkabir Follow-up from the same RTX 4070 setup, now on main 61b8742. These runs also cover #5177.

The stall isn't a hidden failure. One thread runs at 100% inside NVIDIA's shader compiler (nvgpucomp64.dll) under AgcDriver::VulkanDevice::dispatch, and after 180 s it is still compiling, with the title still at 12 frames. The last shader prepared before it is 0x20bf82000. So for vkQuake's RT path, the next blocker after s_round_mode is pipeline creation for that recompiled shader taking minutes on the host driver, longer than the title's 10 s fence wait. The stack is in the comment on #5214.

AI-assisted: yes (Claude Code); all runs were on my machine.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

On PPSA99010 this only moves the stop (iDiesIrae-Req's runs above): 0x20bf82000 is never registered, so RegisteredFloatMode returns nullopt and it now throws ... a shader without a declared FLOAT_MODE. Please take the float mode from the live COMPUTE_PGM_RSRC1 for unregistered dispatches, here or in a PR this one depends on, so the title gets past pc 316.

@semirkabir
semirkabir force-pushed the feat/shader-mode-rounding-tracking branch from 6b9fbeb to c48628c Compare October 14, 2026 05:14
@semirkabir

Copy link
Copy Markdown
Contributor Author

Added in c48628c: a dispatch whose shader was never registered now takes its FLOAT_MODE from the live COMPUTE_PGM_RSRC1. Registered shaders still read their registered state. Rebased on 75690a1.

testDispatchFloatMode in agc_driver_tests covers live, missing, and registered RSRC1. Reverting the change makes it fail with "an unregistered compute shader does not take its float mode from the live COMPUTE_PGM_RSRC1". 112 shader/float/f16/rounding/conversion tests: 110 pass, 2 skipped (f16, lavapipe), Linux GCC.

This is the fallback @iDiesIrae-Req ran on PPSA99010: there's no throw at pc 316 anymore. The title then stalls in NVIDIA's shader compiler at submission 18. That's separate from this PR, and I can't reach it here because this box has no GPU.

AI-assisted: yes

@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the MODE rounding tracking against gfx1035 (s_round_mode, s_setreg_imm32_b32 fields, s_getreg read-back), the agc_shader_mode_register and agc_driver tests (they fail to build without the change), and the RSRC1 fallback requested above; confirmed on PPSA99010 by @iDiesIrae-Req (no throw at pc 316). Merging once the full Build is green.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main: the only conflict was the docs/dev/debt/shader.md line next to the new waterfall entry, resolved by keeping both. No code changes. Re-ran the scoped build and ctest (agc_shader_mode_register, agc_driver pass). Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the feat/shader-mode-rounding-tracking branch from c48628c to e511d12 Compare October 14, 2026 12:22
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main: the only conflict was the docs/dev/debt/shader.md line. The waterfall entry from my previous rebase is gone because #5089 was reverted on main, so only the MODE register line remains. No code changes. Re-ran the scoped build and ctest (agc_shader_mode_register, agc_driver pass; agc_driver_tessellation segfaults as before). Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the feat/shader-mode-rounding-tracking branch from e511d12 to ee1e341 Compare October 14, 2026 13:26
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#4072 core/shader/recompiler/Translation/include/Translation/TranslationContext.hpp L19-20
core/shader/recompiler/Translation/src/InstructionTranslator.cpp L526
#5820 docs/dev/debt/shader.md L47-48

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@adriadam10
adriadam10 merged commit f9cc924 into boykopovar:main Oct 14, 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* build docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants