Repository navigation
fix(libSceAgcDriver): normalize reserved bit 30 in guest buffer descriptors - #7089
Conversation
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
github-actions
Bot
added
docs
agc
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Two tests fail with this PR on both devices I have. Both failures come from test expectations, not from the fix.
Machine: Ubuntu 26.04, i7-8700, GCC 15.2, Release, BUILD_TESTING=ON. NVIDIA GeForce RTX 3070 (driver 595.99.02) and lavapipe (Mesa 26.0.3, LLVM 21.1.8) on the same host. Head 2c7d0bf0d merged onto main 47a266cf9 (clean). ANYPS5_NO_SHADER_CACHE=1, -j1 --timeout 30.
| Run | RTX 3070 | lavapipe |
|---|---|---|
agc_driver_buffer_descriptor_runtime, PR head |
fails 3/3: vector or mixed buffer read changed with reserved bit 30 set |
same failure |
same, ShaderResources.cpp from main |
fails: buffer descriptor has reserved bits set |
same |
| PR head, last check changed as below | pass | pass |
^agc (649 tests), PR vs main |
647/649: agc_driver_buffer_descriptor_runtime and agc_driver_graphics fail (expected graphics rejection: reserved bits); main passes 649/649 |
same two failures; main 649/649 |
The last loop in ScalarReads compares every lane with expected, which is the dword at Input + 4 (the scalar load's value). For the vector-only program Code, lane N stores the dword at Input + 4 * N, the same layout Run checks. So only lane 1 can match. Printed on the 3070 with bit 30 set:
vector lane 0 out 7e552c03 scalarExpected 22f9d0a7 perLane 7e552c03
vector lane 1 out 22f9d0a7 scalarExpected 22f9d0a7 perLane 22f9d0a7
vector lane 2 out c79e754c scalarExpected 22f9d0a7 perLane c79e754c
With the vector-only program checked against Input + 4 * lane and the mixed program kept on expected, all 32 lanes match on both devices and the test passes. In mixedCode the second store (v3 = s8) goes to the same lane address as the vector result, so that case only checks the scalar value. The vector load with bit 30 set is checked only by the vector-only program.
agc_driver_graphics still expects the old rejection at tests/Graphics.cpp:4552: expectSingleFailure(changed([](auto& binding) { binding.guestDescriptor[1] |= 0x40000000u; }), "reserved bits");. With bit 30 now cleared, that binding is accepted, so this line needs to expect acceptance or be removed.
The CI Linux and Windows jobs were skipped on this head, so neither failure showed up there.
Not run: Windows, AMD, Code Violet or any title.
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02) and lavapipe (Mesa 26.0.3), Ubuntu 26.04
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks @jotitadev for the testing reproduction and for pointing out the lane indexing expectation in BufferDescriptorRuntime.cpp and the graphics test expectation at core/libs/prx/libSceAgcDriver/tests/Graphics.cpp:4552. I will update both test expectations to match the normalized descriptor semantics. As a quick note on repository workflow: please do not submit formal GitHub Review decisions unless you are a designated project maintainer with write or merge permissions on �oykopovar/AnyPS5. If you have testing results or observations to share, please post them strictly as regular discussion comments on the PR thread rather than formal reviews. |
|
Windows NVIDIA run, since Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), AMD Ryzen 7 5800X, WinLibs GCC 15.2.0 r7, Ninja, Release, bundled dependencies,
That is the new check at
With this change bit 30 is accepted, so that expectation needs updating if accepting it is intended. The other buffer tests ( AI-assisted: yes (runs done with Claude Code). |
2c7d0bf to
df02f8b
Compare
|
Thanks @Neithennn for running the full hardware suite on Windows / RTX 4080 and catching both test discrepancies! Fixed in updated commit
|
|
I checked the current head |
|
@MarcZX100 @pushkarreddyy Rerun of the exact head Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), WinLibs GCC 15.2.0 r7, Ninja, Release, bundled dependencies,
No title run. AI-assisted: yes (runs done with Claude Code). |
df02f8b to
e62b037
Compare
|
Thank you @Neithennn for the Windows 11 / RTX 4080 verification and test run! Rebased cleanly on latest |
e62b037 to
64fa204
Compare
|
@oneandonlydean Cleanly rebased onto latest All hardware test discrepancies were resolved, and @Neithennn confirmed on Windows 11 / RTX 4080 that full suite tests ( |
|
Thanks, the change is right. One naming point: on GFX10 (gfx1013), bit 30 of V# word 1 is |
64fa204 to
c3c400a
Compare
|
@oneandonlydean Updated the technical debt entry in
|
5 participants
What
Normalizes reserved bit 30 of word 1 (
words[1] & 0x40000000u) to zero when buildingShaderBufferResourceinShaderResources::addGuestBuffer, instead of requiring it to be clear.On RDNA hardware, bit 30 in word 1 of a buffer resource descriptor is reserved and ignored by buffer sampling/resource hardware. While PR #5049 normalized bit 30 for scalar-only buffer resources before binding, non-scalar and compute buffers reaching
ShaderResources::addGuestBufferpreviously threw withbuffer descriptor has reserved bits set.Unblocks Code Violet (PPSA26528, Unreal Engine 5.5) which aborts at compute shader
0x147b9e400with reserved bit 30 set (#7051).Tested
core/libs/prx/libSceAgcDriver/tests/execution/BufferDescriptorRuntime.cppupdated: verifies that vector and mixed buffer accesses with bit 30 set in word 1 are accepted and produce identical dispatch outputs as untouched descriptors.python tools/check_conventions.py --base origin/mainpassed cleanly with 0 findings.docs/dev/debt/libSceAgcDriver.md.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)