Repository navigation
fix(agc): bind a V# of exactly the range limit within the limit - #7717
Conversation
github-actions
Bot
added
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.
|
karimibrahim07
left a comment
karimibrahim07
left a comment
There was a problem hiding this comment.
Windows run of head e74c6abb4, merged onto main ccbd40240 together with #7669, #7670, #7709 and #7680 (each merge clean, built once). Windows 11 Pro (26300), Ryzen 5 3600, NVIDIA GeForce RTX 3060 (driver 610.88), WinLibs GCC 15.2.0 r7, Ninja Release.
agc_driver_graphicswithviewAtRangeLimitTestspasses,--repeat until-fail:3.- Negative control: with
main'sGuestBufferMemory.cppit fails withAGC graphics: guest buffer view exceeds descriptor range limit, as in your description.
Code: ViewBytes(bytes, adjustment) adds the offset adjustment to the view, so a V# of exactly maxStorageBufferRange at an address off minStorageBufferOffsetAlignment always ended up past the limit and threw. Capping it at the limit rounded down to a DWORD fixes that. The cap only applies when bytes <= maxStorageBufferRange, so a V# over the limit still goes through the existing allocation-end and placeholder handling.
One wording point for the debt line: the cap applies to any V# whose size plus its adjustment passes the limit, not only one of exactly the limit. A V# a few bytes under the limit at an unaligned address also loses its last bytes. Something like "a V# within its offset adjustment of the limit" would describe what the code does. The test covers the exact-limit case, which is the Teardown one.
Not checked: a title. I don't have Teardown.
AI-assisted: yes. I ran the build, the test and the negative control above.
e74c6ab to
72abbb5
Compare
|
Thanks for the Windows run and the negative control. You are right that the cap applies to any V# whose size plus its offset adjustment passes the limit, not only to one of exactly the limit. The debt line now says "a V# within its offset adjustment of the limit (for example exactly the limit)" (76012a2, documentation only; the code and the test are unchanged from 72abbb5, on which AI-assisted: yes. |
|
Intel and AMD Windows data: with this PR's test changes, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
AI-assisted: yes (Claude Code) |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ustment of the limit Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
76012a2 to
4c615cb
Compare
|
Title data point for this PR (data only). Title: PPSA15246, 300 s per run through the runner, Windows 10 Pro 10.0.19045, WinLibs GCC 15.2.0, Release, relinker and all libs built from one tree. Baseline: NVIDIA GeForce RTX 4080 SUPER (
AMD Radeon RX 6400 (
One run per card and state. I did not look at the picture, and I did not run the PR's test on this machine for this comment. Not run: other GPUs, other titles, the console. AI-assisted (Claude Code). |
github-actions
Bot
added
needs rebase
|
Merged AI-assisted (Claude Code). |
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
Teardown (PPSA15246) stops with
guest buffer view exceeds descriptor range limitfrom compute shader0x1409cd300. A local print before the check inGuestBufferMemory::Descriptorshows why (lines 29-34 of the 71 lines of that run's stderr;main5d9dcc42dplus the print; lines 32 and 34 are the SIGABRT output of the same abort):The V# size is exactly
maxStorageBufferRange(0xffffffff), so the clamp that #5778 added inShaderResources::addGuestBuffer(size past the limit) does not apply. The address is 12 bytes past a 16-byte offset-alignment boundary, soDescriptorbinds from the alignment below and adds that 12 to the range (ViewBytes): 0x10000000b, one DWORD-rounded step over the limit, and it throws. The registered region behind the address is larger than the limit.The debt entry for #5778 gets one sentence on this case.
Descriptornow limits the range tomaxStorageBufferRangerounded down to a DWORD when the V# itself fits the limit. A V# larger than the limit still reaches the existing check. What the console does for an access past the bound range is not known, as for #5778.New
viewAtRangeLimitTestsbinds a V# of exactly the limit 12 bytes into a registered block, with a second V# at the block start so that the view's region begins before it and the offset alignment is 16. It fails onmainwithguest buffer view exceeds descriptor range limitand passes with the change.Tested
Windows 10 (19045), MinGW-w64, Release; the run below is from the earlier head
e74c6abb4onupstream/main43797ca8a. The current head4c615cbac4has the same source and test hunks (compared by patch id) plus two debt-only commits; it merges cleanly withmain8b426927d7andtools/check_conventions.py --base upstream/mainpasses on it. The test run was not repeated on it.ctest -R '^agc_driver_graphics$': passed (11.5 s). Without the one-line change, the same test fails withAGC graphics: guest buffer view exceeds descriptor range limit(0.66 s, run before the rebase onto43797ca8a; the test file did not change with it).tools/check_conventions.py --base upstream/main: all checks passed. The test module runs against the mock Vulkan device; no GPU was used for the PASS. The title reproduction above ran on an NVIDIA GeForce RTX 4080 SUPER (Windows 10); Teardown (PPSA15246) with this change (e74c6abb4on43797ca8a, fresh libs, no input from me, played by hand from the menu into the first area): the run's stderr (160 lines) no longer containsexceeds descriptor range limit(0 matches, counted with Python). The run then stops at line 139 withdriver failure: AGC driver: unsupported PM4 packet type: header 0x00000000 at DWORD 15 of 6369(SIGABRT, lines 139-142); that is a separate stop and not part of this change. Measured by others on earlier heads: karimibrahim07 on Windows 11 (NVIDIA GeForce RTX 3060, driver 610.88) at heade74c6abb4:agc_driver_graphicspasses 3 of 3 with the change and fails withGuestBufferMemory.cppofmain; savvaniss on Windows 11 (Intel UHD 770 and AMD Radeon RX 9070 XT) at head76012a2bf: all 21 tests pass with the change,agc_driver_graphicsfails without it on both. Linux/lavapipe not tested.Checklist
main; no open PR implements the same functions (related: fix(agc): bind a V# past the range limit by the memory behind it #5778 merged, fix(agc): bind a buffer descriptor past the range limit by its committed bytes, or empty #3202 closed).mdfiles or images addedAI-assisted (Claude Code).
🤖 Generated with Claude Code