Repository navigation
fix(agc): route registered NGG workgroups through mesh - #2261
Conversation
|
Additional platform validation on standalone PR head Linux: failed tests — Arch Linux in WSL2 (kernel 6.6.87.2), GCC 16.2.1 20260810; CMake 4.3.1 / Ninja. Release / Tests: Reproduce after building the test targets: Vulkan: Mesa lavapipe 26.2.4 software Vulkan; no native Linux GPU/retail-title validation. Internal limitation: write-watched guest-heap mirror subcases returned early because WSL userfaultfd async write protection is unavailable. The other graphics assertions ran; zero CTest skips does not include this internal early return. Failed cases: The new Integration limitation: a local merge against main |
029b2e4 to
5314908
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Thanks. Two things before this can land:
- The branch still carries the seven prerequisite commits of #1732/#1825, which are now merged on
mainunder different SHAs, and it conflicts withmain. Please rebase so only the registered-NGG commits remain, and rerun the focused tests on the result. tests/execution/PreparedShaders.cpp:500(RegisteredVertexWorkgroup) only checks forMeshShadingEXT. On Mesa lavapipe (subgroup size 8) the wave64 shader fails to prepare withv_readlane_b32 of lane 63 is outside the 8-lane host subgroup, which failsagc_driver_prepared_shaders,_indirect,_uncachedand_debugon Linux (reported by @Mouhand-Kaddo). Gate the case on the host subgroup size the mesh path actually supports (and keep a CPU-side check for what is independent of the device), or make it pass on an 8-lane subgroup.
5c43862 to
00af1ba
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the rebase and the subgroup gate; the branch is now a single commit over main.
ShaderRegistry.cpp:625is the actual behaviour change (a registered type-2 NGG vertex program using LDS/s_barrieris no longer prepared at registration), but nothing that runs on Linux CI pins it. I deleted that line locally andagc_driver,agc_driver_prepared_shaders*(all 8) still pass.RegisteredVertexWorkgroup(PreparedShaders.cpp:528) only runs with mesh shading and a 32/64-lane subgroup, and prints "not run" on lavapipe (subgroup 8).- Please add a test that does not need that device: e.g. register the same header/code as
RegisteredVertexWorkgroupand assert the preparation is left pending (asDeferredRegistrationdoes) instead of prepared, and that it is prepared as before with a non-NGG routing value. It should fail with the line removed. - Not verified by us: the wave64 GPU subcase and console routing (Windows RTX 4070 run is yours only). Another run on a 32-lane NVIDIA device would help, but the test above is what lets CI guard the change.
VertexWorkgroupRequired and its tests (Submit.cpp:632) look fine: they fail to build without the change and pin the reserved-bit/alignment cases.
Defer registered NGG workgroup preparation until routing is known. Share the routing decision with static ABI resolution and draws. Gate the wave64 fixture on supported mesh subgroups; retain device-independent routing coverage.
00af1ba to
1fe527b
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Checked on the merged tree: libSceAgcDriver build and the agc_driver and agc_driver_prepared_shaders* tests pass, including both new registered_workgroup tests. With only the ShaderRegistry.cpp registration-deferral line removed, both new tests fail, so they pin the behaviour. TechnicalDebt entry is in place. Merging once CI is green.
|
Post-merge validation on pinned main Confirmed fixture problem. The unchanged The fixture reads lane 63 of constant zero. Current emitter now accepts a value proven zero in upper lanes, so this fixture no longer exercises the intended rejection. Suggested test correction, validated locally in alignas(256) static constexpr std::array<std::uint32_t, 9> code{
0xd8340000u, 0u, 0x7e000281u, 0xd7600000u, 0x00017f00u, 0x7e020200u, 0xf80008cfu, 0x04030201u, 0xbf810000u
};This preserves the LDS/workgroup marker and positive mesh-preparation path while keeping the unimplemented ordinary vertex lane read observable. Both registered-workgroup modes pass five times in each of a parallel and sequential group run. Build, Reproduce with Scope: this is not a build of the unchanged standalone #2261 head. The broader suite has separate intermittent |
|
Confirmed on a real title: High on Life (PPSA15388 v01.120.000, Unreal Engine 4). Its NGG culling vertex programs are registered: LDS compaction plus With #1732, #1825 and this PR cherry-picked, the hang is gone. With #3738 as well, the title gets into the level: its audio, an opening cutscene, and in-engine rendering. One note for anyone running the suite: on How the hang was locatedA local diagnostic put a Setup: CachyOS (Linux 7.2), RTX 4090 (NVIDIA 615.78, 32-lane subgroups), GCC, Release. The base is Tested and written by Claude (AI assistant) on a contributor's machine. |
What
Registered type-2 NGG vertex programs using LDS or
s_barrierneed the existing mesh path's workgroup storage and wave64 cross-lane operations. Preparing them as ordinary vertex shaders can reject a lane-63 read on a 32-lane host subgroup.Defer their registration preparation until routing and primitive state are available, and share the workgroup decision between static ABI resolution and draw decoding. Reserved address bits and misaligned entries still throw. Workgroup use follows Mesa's NGG lowering; console routing remains unmeasured and is recorded in TechnicalDebt.
The branch contains one commit over main
a11c8f7f70a0ed4e71504a89de69e8cdae687817, changing seven files. #1732/#1825 are already merged; their seven prerequisite commits are absent from this diff. Current main and existing PRs were checked for an equivalent registered-program extension. Related #3546 changes vertex-wave lowering; it does not implement this registration deferral or registered mesh routing. Depends on: none.Two registration-only CTests call the actual registration export in asynchronous and synchronous modes, with no mesh-shading or subgroup-size gate. The same valid LDS/lane-63 fixture registers on lavapipe's eight-lane subgroup. A missing-user-data variant also registers with NGG routing; changing only the routing to non-NGG requires the normal eager vertex-ABI validation to throw. This control checks that deferral is limited to NGG on any subgroup width. On a synchronous host narrower than 64 lanes, the valid non-NGG header additionally must reach preparation and produce the lane-63 error. These checks need Vulkan initialization, available through lavapipe, but no mesh compilation or hardware GPU.
The separate wave64 mesh preparation fixture requires mesh shading and a supported 32- or 64-lane host subgroup. It reports when that subcase cannot run and executes before the failure-capture check. CPU routing checks remain unconditional and use main's registry with multiple shader headers per address.
Tested
ShaderRegistry.cpp. Both new registration tests failed on lavapipe: asynchronous mode reached the missing-user-data validation, and synchronous mode hit the eight-lane/lane-63 error. Restored the production source byte for byte, rebuilt, and obtained the 12/12 result above. This directly guards the behavior requested in review.agc_driver_prepared_shaders,_indirect,_uncached,_debug) report "shader failure capture was incomplete". The previous same-environment baseline rebuild of main's registry/draw sources and original fixture reproduces all four failures. The Windows expected-failure scenario also stops at that capture error before its intended injection, so its CTest pass is not injection coverage; Linux covers the intended injection.tools/check_conventions.py --base origin/mainandgit diff --check origin/main...HEADpass. The original subgroup-gate mutation also reproduced the eight-lane/lane-63 error. macOS was not tested locally.Automatic CI on the preceding head passed; full hosted Linux/Windows library jobs require a collaborator's manual Build dispatch. A new push restarts automatic checks and requires a new dispatch for full hosted builds.
These fixtures contain synthetic code. The private game source, installed runtime and title assets were preserved; no gameplay or FPS claim is made.
Checklist
main; no other open PR implements the same registered-program extension