Skip to content

fix(agc): accept geometry programs with fewer GS and ES input VGPRs - #5604

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
babyhuey:fix/agc-geometry-vgpr-count
Oct 15, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
babyhuey:fix/agc-geometry-vgpr-count

Conversation

@babyhuey

@babyhuey babyhuey commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

DecodeShaderStages rejected a geometry program (non-NGG VGT_SHADER_STAGES_EN path) unless its GS and ES input VGPR counts were both 3: SPI_SHADER_PGM_RSRC1_GS bits 29-30 and SPI_SHADER_PGM_RSRC2_GS bits 16-17. A smaller count throws unsupported geometry VGPR allocation. This PR removes that check.

The counts only say how many of the hardware's input VGPRs the program reads. In the merged ES/GS layout the inputs sit at fixed registers: the GS inputs in v0-v4 (vertex offsets, primitive ID, instance ID) and the ES inputs from v5. The mesh translation in InstructionTranslator.cpp always initializes v0-v8 at those positions, whatever the counts are. So a program that asks for fewer inputs reads a subset of what is already there.

Graphics.cpp gains a decode check with the registers XPSemu's probe sets, SPI_SHADER_PGM_RSRC1_GS = 0x222c0007 (GS count 1) and SPI_SHADER_PGM_RSRC2_GS = 0x00c00010 (ES count 0). The state now decodes to the same geometry configuration as with 3/3, where main throws.

Tested

On the PR head f535918 (rebased onto main a5f493f, which added a fast-launch exemption to the same check; the line is still removed whole), Linux (Ubuntu 24.04, GCC 13) and Windows 11 (MinGW-w64 GCC 15.2 from WinLibs), Release:

  • agc_driver_graphics passes 5/5 (--repeat until-fail:5) on both. It fails on main.
  • No warnings in State.cpp or Graphics.cpp.

XPSemu (PPSA97358, an xemu port), Windows 11, RTX 3080: its stop is #5437. With #5061, #5187 and #4958 merged locally and relinked with --windows-fixed-base 0x400000, its GPU feature probe (render_geom_shader_triangles in xemu's gpuprops.c) draws with this shader. On that stack without this change, the draw fails with unsupported geometry VGPR allocation (shown with #5177), the probe's submission never completes, and xemu asserts after 10 s (VK_ERROR_DEVICE_LOST). With this change (fork test/verify-xps-gs, the same one-line change), the submission completes, the probe passes, and xemu runs its UI loop at about 72 fps. It then needs the user's own Xbox BIOS files. On Linux (llvmpipe) the draw goes through too; llvmpipe's own LLVM then fails to compile a later shader (Cannot emit physreg copy instruction).

check_conventions.py passes.

Checklist

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head and the relevant translation path. DecodeShaderStages does not use the input-allocation counts to build MeshConfiguration; the mesh prologue initializes the fixed v0-v8 layout that the translated shader consumes. The regression exercises XPSemu’s GS=1/ES=0 registers, and the reported graphics tests plus RTX 3080 probe give direct coverage of the rejected path. I found no blocker in this diff. The open stack dependencies are documented and should remain prerequisites to merging.

@github-actions github-actions Bot added agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@xXPBSXx

xXPBSXx commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Windows test of #5604: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, ninja -j 4, ctest called directly; ctest ran without -V, so the logs of the passing runs do not show which GPU, if any, the tests used (not verified). Tree: main 34355eeea + merge of PR head 74c043e81b0f803804c5969d357834e635851d70 (local merge ba45bea27, no conflict). Logs: rtx5604_*_N.log, rtx5604_result.txt.
1 ctest targets, 3 runs each, exit code 0 in 3 of 3 runs: agc_driver_graphics.
Not tested: that the new or changed tests fail without the change (no baseline run), other ctest targets, the AMD GPU unless named above, Linux, a title run.

AI-assisted (Claude Code).

@babyhuey
babyhuey force-pushed the fix/agc-geometry-vgpr-count branch from 74c043e to f535918 Compare October 14, 2026 03:25
@babyhuey

Copy link
Copy Markdown
Contributor Author

Rebased onto main a5f493f (now f535918). main had added a fastLaunch exemption to the same validate line. This PR still removes the whole line, since the mesh prologue fills v0-v8 for fast-launch programs too. agc_driver_graphics passes 5/5 again on Linux and Windows, with no warnings in State.cpp or Graphics.cpp.

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

milescoviello commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

A Linux run on NVIDIA, for the hardware-driver case: Ubuntu 24.04, RTX 4070 Ti, driver 595.99.02, GCC 13.3, Release, main e6ef803 with this PR's State.cpp hunk applied (the same one-line removal), relinked with --linux-fixed-base 0x400000.

  • On main, XPSemu (PPSA97358) aborts at 12 s. APS5_CAPTURE_TRACE shows the probe's draw failing with VGT_SHADER_STAGES_EN=0x12030: unsupported geometry VGPR allocation. The title's xemu.log ends with radv/ps5: the GPU has not reached submission 2 (at 1) in 10 s, vk_result = -4, then the render_geom_shader_triangles assertion at gpuprops.c:456.
  • With the change, the three probe draws in that submission are drawn, the probe completes, and the title runs the full 60 s. Its log counts Vulkan UI: 5400 frames, and a dumped frame shows its grid background. It then waits for the user's own BIOS files, as on Windows.
  • agc_driver_graphics_tests passes with the change. A decode check with the same two register values fails with unsupported geometry VGPR allocation against main's State.cpp.

AI-assisted: yes

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked: the VGPR-count check removed in State.cpp is not used to build MeshConfiguration (the mesh prologue fills v0-v8 at fixed positions), agc_driver_graphics passes on the merged tree and fails without the change, and the new decode case pins the GS=1/ES=0 registers. Also fixed the "Depends on" line of the body so Conventions parses it; merging.

@adriadam10
adriadam10 merged commit b3f9f89 into boykopovar:main Oct 15, 2026
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants