Repository navigation
Conversation
CnRJay
left a comment
There was a problem hiding this comment.
Carrying the stride through the plan and the disk-cache encoder is the right shape, and requiring the eight dwords to sit inside one entry keeps the matcher safe. The new partial-descriptor check in resolveTableImage is too strict for strides other than 32, though:
if (heapSize != 0u && (heapSize < table.entryOffset + TableEntryBytes || (heapSize - table.entryOffset - TableEntryBytes) % table.entryStride != 0u)) throw ...This requires the heap to end exactly at the end of the last T#. That holds for RE4 only because its T# is the last 32 bytes of the entry (16 + 32 == 48). Take a table with 64-byte entries and the T# first (offset 0), with a V# that covers whole entries (heapSize = 64 * n, the natural size): (64n - 32) % 64 == 32, so it throws "bindless heap contains a partial descriptor" although every descriptor is complete. The same happens for any layout where data follows the T# in the entry.
A descriptor is only partial when the heap ends inside a T#. With tail = (heapSize - entryOffset) % entryStride, that is tail != 0 && tail < TableEntryBytes. The entry count stays (heapSize - entryOffset - TableEntryBytes) / entryStride + 1, as written. For stride 32 this reduces to the old check, so 32-byte tables still behave as on main. A 64-byte/offset-0 case in agc_shader_bindless, next to the 48-byte one, would cover it.
|
Thanks, you are right: with Windows 10 (19045), MinGW-w64 GCC 15.2, Release, AI-assisted (Claude Code). |
|
Title evidence for this pull request: PRAGMATA (PPSA02530), Windows 10 (19045), RTX 4080 SUPER, one 90 s run per state, no picture checked.
Test conditions: the merge trees are local and not pushed (the base AI-assisted (Claude Code). |
|
AMD and NVIDIA runs for this PR at head
No title run. AI-assisted: yes (Claude Code ran the builds and tests). |
|
Data point from another title. Marvel's Wolverine (PPSA03671, 01.001.005), Linux, RTX 3080 Ti (NVIDIA 615.71.09): merged on top of AI-assisted; the runs are from my machine. |
|
Title evidence: Onimusha: Way of the Sword (PPSA27836 01.000.001, RE Engine), Windows 11 Pro 26200, RTX 5070. On A later program reads the same table T# for two image operations. With #5620 and #5550 both merged, both pass and the title stops next at AI-assisted: yes (Claude Code). |
98afd6b to
a4146d3
Compare
a4146d3 to
d7e6318
Compare
|
Title run on the rebased head: Resident Evil 4 (PPSA07411), Windows 10 (19045), RTX 4080 SUPER, one run of 120 s requested, no picture checked. On head Before this change the same title stopped at that Test conditions: all 128 AI-assisted (Claude Code). |
|
Heads-up: rebased onto current That test reads its T# with Reproduced on Windows 11 with only this PR's two commits rebased onto AI-assisted: yes (Claude Code). |
|
Thanks for the check. The head On that head, AI-assisted (Claude Code). |
The bindless table planner only recognised a T# table whose entries are exactly 32 bytes apart (key << 5 or key * 32). A title (PPSA07411) loads its T# from a table with 48-byte entries, the T# in the last 32 bytes (key * 48, load offset 16), so the descriptor stayed a ReadConstBuffer and the recompiler stopped with "GetImageResource dword 0 is not a valid runtime value". IndirectImage now carries entryStride (default 32). MatchTableOffset accepts a multiplier that is a multiple of 4 and at least 32, the planner requires the eight T# dwords to lie inside one entry, and the materializer uses the stride for the entry count, the partial-descriptor check and the entry address. A 32-byte table behaves as before. agc_shader_bindless gets a 48-byte-stride case; it fails on main with the same message and passes with this change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The partial-descriptor check required the heap to end exactly at the end of the last T#, which only holds when the T# is the last 32 bytes of each entry. A table with 64-byte entries and the T# first, backed by a V# that covers whole entries, was refused although every descriptor is complete. A descriptor is partial only when the heap ends inside a T#: the remainder of the heap after the entry offset, modulo the stride, is non-zero and below 32 bytes. 32-byte tables keep the old behaviour; the entry count is unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…direct images A table entry stride other than 32 now only plans a bindless table when the T# is not a valid uniform runtime value; otherwise the existing direct-image path (uniform table image in tests/ShaderMemory.cpp) applies as before. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Title evidence for the evaluable-key case above. Onimusha (PPSA27836) has a draw whose T# is read from the same 48-byte table, at a key the SRT walk can evaluate ( Locally, AI-assisted: yes (Claude Code). |
d7e6318 to
ee95066
Compare
|
Follow-up to my earlier reply: the head is now I checked your AI-assisted (Claude Code). |
|
Baseline for the title run above, on a recent
With this pull request (head AI-assisted (Claude Code). |
|
Thanks for the Onimusha (PPSA27836) report. I compared your local fix (return What the third commit does ( Difference to your fix:
Does the head cover your case? By the code, yes: a 48-byte stride ( AI-assisted (Claude Code). |
|
@xXPBSXx thanks for the comparison. Your head covers the Onimusha (PPSA27836) case in practice. On my local branch I replaced my fix with ee95066 (reverted my commit, cherry-picked yours, nothing else changed). On Windows with the RTX 5070, the title prepares the same programs and stops at the same later point as with my fix: the first presentation, which now fails with "a color target's sample count differs from the rasterizer's" (checkerboard rendering, after #5861). No bindless table error. So I'm dropping my local fix in favour of your commit. I'd rather not post the title's shader binary. The shape is a 48-byte stride ( #5820 is now rebuilt on your head ee95066, and AI-assisted (Claude Code). |
|
Windows measurement for PRAGMATA (PPSA02530), with and without this PR. One 200 s run each, RTX 4080 SUPER, Windows 10, same
So in these two runs this PR removes the AI-assisted (Claude Code). |
|
Closing as superseded by Title evidence on Second title (PRAGMATA, PPSA02530, whose AI-assisted (Claude Code). |
What
The bindless image-table planner only recognised a T# table whose entries are exactly 32 bytes apart (
key << 5orkey * 32). Resident Evil 4 (PPSA07411) loads its T# from a table with 48-byte entries, the T# in the last 32 bytes (key * 48, load offset 16). The descriptor therefore stayed aReadConstBufferand the recompiler stopped withGetImageResource dword 0 is not a valid runtime value.DescriptorSource::IndirectImagenow carriesentryStride(default 32).MatchTableOffsetaccepts a multiplier that is a multiple of 4 and at least 32; the planner requires the eight T# dwords to lie inside one entry;ResourceMaterializeruses the stride for the entry count, the partial-descriptor check and the entry address.ShaderDiskCache.cpp) writes and readsentryStride; without it a stored plan would reset the stride to 32, andmaindoes not build with the new field until the encoder is updated (static_assert ... IndirectImage changed: update the plan encoder).uniform table imagecase intests/ShaderMemory.cpp) stay direct images; only records it cannot evaluate become a table.New test cases:
agc_shader_bindless(inagc_shader_memory_tests) gets a 48-byte table; the round-trip case ofagc_shader_disk_cachenow uses a table with stride 48.Tested
Windows 10 (19045), MinGW-w64 GCC 15.2, Release,
ninja -j 4of the sevenagc_shader_*test targets, ctest called directly in PowerShell. Headee950660358c6d199a23131ad00de6f96db50fba(three commits) onmain63fb0cd92e83eaba3b3d1ac88fc532c7dd32ae58;git merge-treeagainstmain5a6ad0ed36893e86022929b808ac547672445cd9(later) reports no conflict:ninjaexit 0;ctest -R agc_shader_bindlessexits 0 (1 of 1);ctest -R agc_shader_exits 8, 14 of 15 passed. The one failure isagc_shader_disk_cachewithcannot remove the shader cache test directory: Zugriff verweigertafter all of its round-trip checks printed; the same test fails with the same message onmain23be5a02b075bd05c7c27d8b18a3d47ead6fd0e5without this change (run alone; not re-run on63fb0cd92), so it is not caused by this PR.agc_shader_memorypasses (ctest -V -R ^agc_shader_memory$exits 0; ctest prints only the test name, theuniform table imagecase is in its source,tests/ShaderMemory.cpp).ResourceMaterializer.cppandResourceTracker.cpptaken back tomain; tests, header field and disk cache encoder unchanged):ctest -R agc_shader_bindlessexits 8, 0 of 1, withGetImageResource dword 0 is not a valid runtime value.python tools/check_conventions.py --base upstream/mainpasses.maind1670009f(logPPSA07411_rB/run_022642.err) ends withShaderRegistry.cpp:782 ... Shader 0x2060e9c100 could not be prepared after registration: ShaderRecompiler::Recompile: GetImageResource dword 0 is not a valid runtime value.mainand without the disk cache change; no picture was checked, so nothing is claimed about the rendered image. Not re-run on the rebased head.agc_shader_*tests above), AMD.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); no silent stubs added.mdfiles or images added to the repositoryAI-assisted (Claude Code).