Repository navigation
Conversation
github-actions
Bot
added
the
unblocks title
|
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.
|
|
Reviewed; verified this is behavior-preserving and the message is honest. The throw condition is exactly the old one ( I checked the exact-string assertions against the test fixtures rather than trusting them: One note: the run legs ( |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The change is behavior-preserving: the record-count throw condition is unchanged, the diagnostic is constructed only on the failure path, and subsequent zero-stride/address-range checks remain in their prior order. The exact expected strings match the fixture field decoding. Exact-head conventions and git diff --check pass; hosted conventions, eligibility, intake, labels, and progress report pass. Product Linux/Windows jobs are skipped, so those checks did not independently run the test here.
Before approval, please attach or link the paired ctest -V validation-layer logs required for AGC changes and state explicitly that neither run contains was not found but was requested by env var VK_INSTANCE_LAYERS and that the PR introduces no new VUID- errors versus main. The checklist currently asserts this, but the test section only says the focused test passes and does not provide the required validation-layer/no-cache invocation or the log evidence.
The reported Kena rate changes from 14.0 fps on main to 13.6 fps on this change. Since this is a performance comparison, please add the two valid same-machine/scene summary lines and attach the traces using the repository Performance report format, or remove the unsupported fps comparison if it is incidental and not a claimed performance result. Include the exact title bring-up discussion link (currently #4073) and keep its PR list current as required by the contribution rules.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I refreshed exact head ecc035657c814259a1d47dd7f500ebb410094b34 against current main 3ca07d086d5f44fbe3c00370e7f03953ab46ca1b: git diff --check and conventions pass, and the current-main merge tree is clean. The diagnostic is still failure-path-only and preserves the existing throw condition; the exact-string test expectations were independently checked against the fixture fields. Hosted conventions/eligibility/labels/report pass, but Linux/Windows product jobs are skipped. I did not run the GPU test locally.
The existing review evidence requests remain unresolved: please attach the paired ctest -V validation-layer logs for main and this head, including the exact invocation and confirmation that neither run has the VK_INSTANCE_LAYERS warning or new VUIDs. The Tested section also compares Kena at 14.0 and 13.6 fps. Since the change is diagnostic-only, remove that incidental performance comparison unless you intend to report a performance result; if retained as a result, include same-scene summary lines and traces using the repository performance-report format.
The PR base is dce41a89478876a8e3ea9c346b1dac00bb31c8cd, behind current main. Please update to current main and refresh the convention/test evidence on the resulting head. This is a request for the current-main and review-evidence requirements only; it is unrelated to the obsolete intake hold.
…or record count A vertex fetch past NUM_RECORDS threw only 'vertex fetch exceeds descriptor record count', which does not say which V# fields decided the RDNA 2 range check. The message now appends OOB_SELECT, stride, NUM_RECORDS and whether the index is the vertex or the instance index, with the index. The behaviour is unchanged; it is still a throw. Kena PPSA01746 stops on this in the bring-up of boykopovar#4073, and the values show what the next step must implement.
ecc0356 to
56ea15a
Compare
|
Additional title hitting this diagnostic path: The Eternal Cylinder (PPSA06369) aborted after 90.4 seconds on clean main Reviewed this PR at Independent run review, report/log/image SHA-256s and limitations. Full local logs were retained. |
|
A second title with this PR's message: The Eternal Cylinder (PPSA06369) 01.003.000, on Without this PR, the same run stops at 90.4 s with the plain message. That's OOB_SELECT 2 with a nonzero stride. Mesa's RDNA 2 descriptor code says what the range check is for each mode ( So with OOB_SELECT 2 and NUM_RECORDS 303, the fetch at index 319706 is not out of bounds on the hardware. It reads at @ebarr24 if you're already implementing this next step, I'll leave it to you. Otherwise I can open a PR for OOB_SELECT 2 with a strided buffer and test it on this title. Kena's values from #4073 would show whether it covers that case too. AI-assisted: yes (Claude Code). |
…h OOB_SELECT 2 and 3 A strided vertex fetch with OOB_SELECT 2 or 3 threw whenever its index was at or past NUM_RECORDS, which stopped Kena PPSA01746 (its own descriptor values are not captured yet) with the same message that The Eternal Cylinder PPSA06369 printed for stride 12, NUM_RECORDS 303 and vertex index 319706 (reported by JJenkx on boykopovar#9604). RDNA 2 checks only NUM_RECORDS == 0 for OOB_SELECT 2, and for OOB_SELECT 3 compares NUM_RECORDS with the fetch's byte range without using the index (Mesa ac_descriptors.c at mesa-26.2.4, measured on gfx1036 in boykopovar#2241). The fetch is now sized from its index in those cases, and the cases the hardware reads as zeros (NUM_RECORDS 0 for select 2, NUM_RECORDS below the fetch's bytes for select 3) still throw, as does the extent of an indirect draw's fetch, which NUM_RECORDS no longer bounds.
…h OOB_SELECT 2 and 3 A strided vertex fetch with OOB_SELECT 2 or 3 threw whenever its index was at or past NUM_RECORDS, which stopped Kena PPSA01746 (its own descriptor values are not captured yet) with the same message that The Eternal Cylinder PPSA06369 printed for stride 12, NUM_RECORDS 303 and vertex index 319706 (reported by JJenkx on boykopovar#9604). RDNA 2 checks only NUM_RECORDS == 0 for OOB_SELECT 2, and for OOB_SELECT 3 compares NUM_RECORDS with the fetch's byte range without using the index (Mesa ac_descriptors.c at mesa-26.2.4, measured on gfx1036 in boykopovar#2241). The fetch is now sized from its index in those cases, and the cases the hardware reads as zeros (NUM_RECORDS 0 for select 2, NUM_RECORDS below the fetch's bytes for select 3) still throw, while an indirect draw, whose counts are unknown when its vertex ranges are copied, still copies the descriptor's NUM_RECORDS range.
Dismissing a stale review: the requested changes were addressed after it (new head and/or updated description with the asked-for runs) and the merge queue blocks on it. Please request changes again if anything is still open.
|
A Windows run of this head: Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release.
This PR and #9694 both change AI-assisted: yes (Claude Code) |
What
A vertex fetch past NUM_RECORDS threw only 'vertex fetch exceeds descriptor record count', which does not say which V# fields decided the RDNA 2 range check. The message now appends OOB_SELECT, stride, NUM_RECORDS and whether the index is the vertex or the instance index, with the index. The behaviour is unchanged; it is still a throw. Kena PPSA01746 stops on this in the bring-up of #4073, and the values show what the next step must implement.
Tested
Windows 11, MinGW-w64 GCC 15.2, Release build with tests.
agc_driver_graphicspasses with this change; the new test fails onmain.Validation layer (vulkan-sdk-1.4.363.0), on
main11f976b and on this change, withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,VK_ADD_LAYER_PATHset to the layer,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1:ctest -V -R "agc_driver_graphics". Neither log containswas not found but was requested by env var VK_INSTANCE_LAYERS; the change adds noVUID-errors (0 with the change, 0 onmain).ctest -V log, main
ctest -V log, this change
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)mainand this change, no new VUIDs🤖 Generated with Claude Code