Repository navigation
Conversation
|
Independent native Linux check at exact head
Integration negative control: keeping that diagnostic and the PR's helper/tests, but replacing only All temporary edits were then removed, the stock graphics and array-window targets rebuilt, both registered graphics tests rerun successfully, and the stock GPU fixture rerun successfully on all three ICDs. Checkout is clean at the tested head. Ubuntu 24.04.3, GCC 13.3, Release; RTX 3090 / NVIDIA 595.99.02, gfx1036 ( |
|
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.
|
|
Windows run of the full sampled path on NVIDIA and AMD, plus one problem in the cube fallback. Head Host: Windows 11 (10.0.26300), Ryzen 5 9600X, WinLibs GCC 15.2.0, Release. GPUs: NVIDIA RTX 5070 Ti (617.42) and AMD Radeon RDNA2 iGPU Registered tests. The 81-test related set ( Local GPU probe (not for the PR). I adapted that fixture to build here:
Cube fallback.
The texture path then requires at least 6 slices for a cube ( AI-assisted: yes (Claude Code); results above were run on this machine. |
|
@MarcZX10 a related case from Ghost of Tsushima Director's Cut (PPSA02225). This PR doesn't cover it, by design, but you may want to know about it. On @EagleBearerYS's
I have no evidence of what the console reads past the allocation, or of which depth range the shader samples. I'll look at the shader's sampling before suggesting anything concrete. Release libs, Linux, RTX 4070 Ti, NVIDIA 615.71.09. AI-assisted: yes (Claude Code) |
|
@MarcZX10 a follow-up on the 3D view from Ghost of Tsushima I mentioned above, in case it helps decide whether I tried a local cut, not for a PR, on the same stack with this PR merged: a sampled
The catch is that a shorter depth changes what a normalized AI-assisted: yes (Claude Code) |
|
@milescoviello thanks for the depth-cut run and the offer to test. Here's proper k3D handling for Ghost of Tsushima Director's Cut (PPSA02225):
If it gets past 572 s, the next stop's log would be great. The 1622 s cap while the opening's pipelines build looks like a separate question, so a log of it would help as well. @MarcZX10 this sits in the 3D snapshot and upload path, not in #9318's view bound. It doesn't touch |
|
@EagleBearerYS thanks, that's the right shape: keep all 900 slices and fill only what's committed. The run is queued for after the current ones, in a couple of hours. It uses the same stack as my earlier runs ( AI-assisted: yes (Claude Code) |
|
@EagleBearerYS a first, inconclusive run of The cache is warm now, and warm runs reach that point sooner with more room left, so I'll rerun it the same way and post again. AI-assisted: yes (Claude Code) |
|
@EagleBearerYS the warm rerun didn't reach the view either. It was killed at 807 s by the same cap during the opening's pipeline builds, so The growth there is now traced. A jemalloc profile in that phase puts about 1 GiB live under AI-assisted: yes (Claude Code) |
|
@MarcZX10 Please preserve complete-cube bounds in |
|
@milescoviello an update to the 3D branch, for when a build with #9129 gets Ghost of Tsushima Director's Cut (PPSA02225) to the 320x1280x900 view: The slab line next to |
b21d32e to
8ed6ddb
Compare
|
Fixed on 8ed6ddb (rebased onto current main). |
|
@EagleBearerYS thanks, and sorry for the late reply. I've noted @MarcZX10 for the new cube cases, I've queued AI-assisted: yes (Claude Code) |
|
@MarcZX10 the run I offered gives less than I expected. On Linux (GCC 15.2, Release), The Khronos validation half turned out to be empty, though. AI-assisted: yes (Claude Code) |
|
@milescoviello thank you for running it and for checking whether the layer was actually loaded. You are right that it was not, and that is why the earlier note said I had not run |
|
@MarcZX10 the complete-cube rounding reads right now. One question about where the bound comes from. |
|
@MarcZX10 thanks for this, I tested it on The Last of Us Part I (PPSA03396), the game from #9295. Linux, RX 6650 XT (8 GB), 32 GB RAM. I cherry-picked the five commits onto an older main (base What got better: the 2560x1440 arrays with 40-142 layers (500-600 MiB each) that we mirrored before are gone from our big-texture log. I could also drop the local 512 MiB array clamp I had added for #9295. One problem: after about 6.5 minutes in the menu, the game ended at the new So a sampled array view whose base is inside a registered range, but with less than one slice left in that range, ends the whole game. Before this PR the same view was just mirrored. Would it be better to keep the view unbounded (or bind it absent) in that case instead of throwing? Locally I catch it in This may be the same thing @oneandonlydean asked about: if the texture continues into a neighbouring range, the first range alone holds less than one slice. |
Bug found — sampled array bounds truncate adjacent allocationsThe current head can stop at the first registered range and can throw when fewer than one full slice remains, matching the two newly reported failure modes. FindingsBlocking
Non-blocking
Not verifiedI did not run a new build or test for these findings in this response. Pacuka's report was from a cherry-pick onto older base To get this to a good stateI need to update the bound and fallback behavior, add regressions for adjacent ranges and incomplete slices, and rerun the focused tests on the new head before treating these findings as resolved. |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, 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. |
There was a problem hiding this comment.
Correction (2026-11-01): the title-ID/log demand below is not a CONTRIBUTING requirement; Unblocks: none is allowed. The technical points below are unaffected.
This is an approximation of what the hardware does with an out-of-range descriptor, and the PR says so itself ("does not establish what the console does with out-of-range descriptors"). Under the exact-or-throw rule that is the part that should not merge as written.
-
BoundArrayWindowinvents a view. When the requested range runs past the registered allocation it silently shrinksdepthOrLastArrayto the last whole slice, and whenbaseArrayis past the end it rewrites the view to slice 0 (or cube 0). A guest that really samples slice 5000 of a 90-slice allocation gets slice 0 back with no log and no throw. Nothing in the description says the console returns slice 0, or that the descriptor's slice count is meant to be clamped by the allocation size at all. If the general behaviour is unknown, the choices are to throw (as theavailableLayers == 0case already does) and record the gap indocs/dev/debt/libSceAgcDriver.md, not to pick a substitute view. The 6,145 versus 90 case in #9295 is a case where the descriptor probably does not mean what the code assumes; a throw naming the descriptor fields would show that, a clamp hides it. -
The clamp is keyed to registered allocations, not to what is mapped.
GuestAllocationsLookup_nid_postfix(baseAddress, false)decides the limit. A texture whose layers span two adjacent registered allocations, or memory mapped outside the registered list, gets truncated at the first allocation's end, which would produce a wrong image for a valid descriptor rather than an out-of-range one. Is the lookup guaranteed to return the whole contiguous mapping? The PR text says "contiguous mapped guest range", the code uses the registration. -
Per-view cost. The lookup and
DescribeSurfacenow run on every sampled 1D/2D array and cube view built, not only on the oversized ones, andguestBytes = 0is set whenever the window changes. Please state how often this path runs per draw in a real scene, or gate the lookup behind the existing size check (layers * layerBytesagainst a cheap bound) so the common case does not pay for it. -
Evidence. No title ID and log line showing a title reaches this on main; the justification is issue #9295 only. The regression cases test the clamp's arithmetic, but nothing checks that an in-range descriptor on an allocation that is not the first registered one is left alone.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — four review points checked, clamp reaches every array view
Three of CnRJay's four technical points are confirmed by the code at head 8ed6ddb, and the fourth is only partly checked.
What I ran
Head 8ed6ddb40781f559cf60b855a99c1e794147b42d (live, OPEN). Read-only: gh pr view, gh api GET on comments and reviews, git grep and git show at the head. Nothing was built or run.
Results
| Check | Result | Note |
|---|---|---|
| Hosted checks | 4 success, none failing | NVIDIA run by TechLevelZero passed on this head |
| Code reads (points 1-3) | Confirmed | Read at head, lines below |
| Test coverage (point 4) | Partly checked | Arithmetic tests found, placement not checked |
Findings
- Point 1, the window invents a view: confirmed.
TextureTiling.cpp:523-526resetsbaseArrayto 0 when the start is past the available layers, and:528shrinksdepthOrLastArrayto the last whole slice. No log or throw on those paths. The only throw is for zero layers (:521). - Point 2, registration-based limit: confirmed.
ShaderResources.cpp:714callsGuestAllocationsLookup_nid_postfix(resource.baseAddress, false)and clamps torange->bytes - offset(:716). The limit is the registered range, not the mapping. - Point 3, per-view cost: confirmed. The lookup at
ShaderResources.cpp:714runs for every 1D or 2D array or cube view, with no size check first.:720setsguestBytes = 0when the window changes. How often this runs per draw in a real scene is not measured. - Point 4, evidence:
tests/TextureTiling.cpp:535-564cover the clamp arithmetic on a 90-layer allocation and cube cases. I did not confirm whether any test puts an in-range view on a non-first registered allocation. Not verified.
Sources
AnyPS5/core/libs/prx/libSceAgcDriver/Graphics/src/TextureTiling.cpp
Lines 511 to 530 in 8ed6ddb
AnyPS5/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
Lines 711 to 720 in 8ed6ddb
AnyPS5/core/libs/prx/libSceAgcDriver/tests/TextureTiling.cpp
Lines 535 to 564 in 8ed6ddb
Not verified
- Console behavior for out-of-range descriptors (no hardware).
- Whether the lookup always returns the full contiguous mapping (point 2 asks this; the code only shows the registration is used).
- The #9295 descriptor values and any title log.
- Per-draw frequency of the lookup (point 3).
A cube-compatible image needs at least six layers, so a cube view cut at its allocation boundary is rounded down to complete cubes, rejected when none fits, and falls back to its first cube instead of one layer when it starts past the allocation.
Fewer than six mapped faces is rejected, a partial trailing cube is cut off, a base past the allocation falls back to the first cube, and a view inside its allocation keeps its descriptor.
8ed6ddb to
99a6c08
Compare
❌ NVIDIA GPU test: failedPR head
Failing testsThis PR changes test files, so each failing test was also built and run on main with only the PR's test changes applied. ✅ there means the failure comes from the PR's other changes; ❌ means the test fails on main too, so this run does not show the PR caused it.
agc_driver_submit_progress: Failed (also failed when re-run alone), assertion and diagnosticsUnstable on main (2, 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)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (9 IDs in 592 tests)Skipped (51)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. |
|
Hi @MarcZX10, thanks for the rework at The two earlier points need that call site to exist before they can be checked:
Could you restore the caller with those two changes? |
What
Issue #9295 reports sampled 2D-array descriptors naming 6,145 slices when only 90 complete slices fit in the contiguous mapped guest range. Building the full sampled image can request more than 100 GiB of host image memory and abort with
std::bad_alloc.Cap sampled 1D-array, 2D-array and cube views to complete slices in the mapped range before creating the cached image. If the requested starting slice is outside the range, use slice 0 alone when at least one complete slice fits; throw when none fit. Storage views are unchanged. Regression cases cover the 90-of-6,145 boundary, an in-range starting slice and the fallback/error cases.
This addresses only the oversized sampled-view allocation described in #9295. It does not establish what the console does with out-of-range descriptors and does not resolve the issue's other stops or black frames. Related open work #6481 validates image-table entries; #6977 adds GCN tiling. Neither implements allocation-bounded sampled array views.
Tested
Linux x86_64, GCC 12.2, Release.
ninja -C build agc_driver_graphics_testspassed after rebasing onmain6b6c701636. The focusedRunTextureTilingTestssuite passed in a standalone runner linked from the project-built objects.The registered
agc_driver_graphics_maps_fallbackCTest could not reach its test body: startup rejected this host's Intel Xeon E5-1620 v2 because it lacks LZCNT, BMI1, BMI2, MOVBE, AVX2 and FMA. No Vulkan or GPU test is claimed.python3 tools/check_conventions.py --base upstream/mainpassed.Checklist
main; no other open PR implements the same behavior