Repository navigation
feat(shader): give reads the masked-select pass keeps the whole pixel ancillary word - #4031
Conversation
|
Rebase handoff — rebased onto current Four files conflicted, all the additive-field shape: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
|
@blawesom thanks for the rebase and for running the whole suite on RADV. The branch now has The one difference is the request serialization version. AI-assisted: yes (Claude Code) |
|
Verified your merged head ( And credit where due: the version bump is a catch my rebase missed. I merged both fields but kept the primitive class read at version 17 — which is exactly the one-byte-short truncation you describe for main-written version-17 requests. Writing 18 and splitting the reads (stencil from 17, primitive from 18) is the right wire behavior, and the |
|
Ran this on AMD hardware, since the GPU tests in CI run only on lavapipe: Windows 11 (26300), Radeon 860M (RDNA 3.5, driver 32.0.13058.8001), WinLibs GCC 15.2.0, Release, GPU tests (
The target is filled with With
Other tests: Titles. Seven homebrew titles that render on I haven't looked into why the draw leaves pixels unwritten on this GPU. I can run a diagnostic build or more cases if that helps. AI-assisted: yes (Claude Code) |
|
@LeandroLP23 thanks for running both tests on the 860M, and for noticing that the It is not the sample index and not invalid SPIR-V from this PR. It is AMD's shader compiler (LLPC) programming the position exports inconsistently when a pixel shader reads Both failing draws read the reconstructed ancillary word, so their pixel shaders read
So the rasterizer takes clip distance 0 from How I checked it, all on the CPU:
With 2.13, the same clip distance with a pixel shader that reads only A minimal repro outside AnyPS5: the vertex shader writes One check would confirm it on your machine. In If it confirms, AnyPS5 can keep clear of this on older drivers. When the pre-rasterization stage doesn't write AI-assisted: yes (Claude Code) |
|
@AlxcBTW the AMD failure above comes from reading the unwritten |
|
Three things on dd84031 before the question of the unknown bits:
On the word itself nothing has changed since #1179: bits 2-7, 12-15 and 29-31 are zero by argument, not by measurement, and that is the rule adriadam10 set there, so it is his or the owner's call and not mine. Kena's remaining shader may not need the word at all. |
… ancillary word A packed ancillary word that still reaches an export, or any other read the masked-select pass cannot drop, no longer stops the recompile. After that pass every remaining use gets the composed word: the primitive class at bits 0-1, SampleId at bits 8-11 and Layer at bits 16-28, with every other bit zero. Those lanes do export the ancillary VGPR on the console, so the word has to be right, not just present. The primitive class (0 point, 1 line, 2 triangle, 3 rectangle) comes from the draw. RasterPrimitiveOf maps rect lists to 3, line lists on the vertex path to 1 and everything else the driver accepts to 2, because its geometry and tessellation paths only output triangles. It reaches the shader as pipeline specialization constant 8233, the way dual-source blending does: a request field that is folded into the specialized module and is part of the result memo, the draw key and the serialized request (version 17). Only shaders that read the class pay for it. ShaderInfo records the read (disk cache format 28), and only those shaders put the class in their module key, so a shader that reads just the sample or layer field keeps one module for every primitive class. The draw sets the class only when the pixel shader loads the ancillary VGPR, so other draws keep the draw cache entries they have today. The VRS rates (bits 2-5) and the eye id (bit 7) are 0 because the driver supports neither VRS nor multiview. SampleId is 0 because the driver rejects MSAA and per-sample shading, and Layer is 0 while it ignores vertex layer exports. TechnicalDebt.md records the layout sources and these assumptions. Tests: - pixel_ancillary builds an export to target 0 whose two channels select between a packed value and a phi carrying the word. It checks the word for each primitive class, sample and layer, checks that the three shapes the masked-select pass keeps now compile with the composed word, and that a program whose select the pass drops reads no class. - agc_driver_pixel_ancillary_word exports the word from a draw and reads back 2, 3 and 1 for triangle, rectangle and line. A shader that reads only the sample field shares one module across the classes, and the whole-word shader gets one per class. It also checks RasterPrimitiveOf. - agc_shader_memory covers the version 17 round trip, and agc_shader_disk_cache the new flag. Both ancillary tests fail with the old refusal when the composition is disabled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sed word The relative-ancillary test expected the whole-word read through v_movrels to be refused, which was main's behaviour before the word was composed. It now converts that read to float and exports it: the composed word of a triangle draw is 2 (primitive class 2, sample 0, layer 0), so the pixel is 255, with and without the implicit position inputs in front of the ancillary VGPR. TechnicalDebt's relative addressing entry no longer says that whole-word uses throw. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean here is the request for It is the file I have no IR dump at the failing point, the tool only prints the instruction listing for it. I can post that listing, or the other 16 Kena requests, if either helps with the select rule. |
…se the ancillary word with render target index 0 main reads target.vertexSubgroups at request version 18 since boykopovar#4470, so reading the pixel primitive class at 18 as well read a version 18 request written by main one byte short. The class is now written and read at version 19. agc_shader_memory loads a version 18 request as main writes it, and it fails with "truncated data" when the class is read at 18. The composed word no longer reads the Layer builtin for bits 16-28. Draws render to a single layer and guest layer exports are ignored, so no stage writes Layer, and AMD's Windows driver drops the writes of a fragment shader that reads it unwritten (boykopovar#4800). The field is the constant 0 and pixel_ancillary checks that the composed program has no Layer read. The debt entry names the bits that have no source and are assumed zero (2-7, 12-15, 29-31), and says that a change which makes draws export a layer must restore the read.
dd84031 to
c52321f
Compare
|
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.
|
|
Pushed the rebase and one commit on top (c52321f), your two commits kept as they are:
|
|
@LeandroLP23 thanks for #4842, and for confirming With that commit, #4031 reads the render target index field (bits 16+) of the whole composed word as the constant 0, with the same reasoning as #4842: no pre-rasterization stage writes Layer today, and reading it breaks the position exports on AMD's Windows driver (#4800). The The extract path in AI-assisted: yes (Claude Code) |
5 participants
What
Refs #2548 and #3504. With this change a select that the masked-lane walk keeps no longer stops the recompile, because every read the walk keeps gets the whole word. That lets #3550's walk budget stay low without breaking titles that need the select gone for the ancillary lowering (Stray's
0x2013010000, see #3504).A select the masked-lane walk can't drop still stops the recompile on #3279. Palworld's
0x1b40e5c000is one (Rohan03x on #2548: the kept select is reached through aReadFirstLane, which reads other lanes), and so are Minecraft Dungeons II's0x190095e900and0x190095fc00. With this commit every read the walk keeps gets the whole word:RasterPrimitiveOf);The layout is from the RDNA 3/3.5 ISA guides. The GCN register guides agree for layers below 2048, Mesa reads sample and layer from those bits, and AMD's LLPC reads the prim type from bits 1:0 with the same encoding. Nobody has checked it on gfx1013; TechnicalDebt.md says so.
Where the primitive class comes from (owner's call)
IrBindingLayoutinDescriptorBinding.hpp), and no fragment builtin can tell a rect list (drawn as tessellated triangles) from a triangle. The smallest run-time variant adds one dword to that block in shaders that read the class, and patches it per draw fromGraphics::Statethe wayShaderResourcespatches guest buffer offsets (pushBytefor push constants,dataBytefor the ShaderData buffer). The module key, result memo, draw key and request format could then stay as onmain. But the shader-data layout changes (IrBindingLayout, disk cache format), the block spills to the ShaderData buffer one dword sooner, and every pixel wave loads the class instead of using a constant.VkSpecializationInfofromGraphics::State, whose pipeline key already holds the topology and rect-list state. The request format, module key, result memo and draw key would then stay as onmain. Prepared modules would have to leave that one constant open (buildSpecializedModulerequires every SpecId today), and the GPL fragment-library key would need the class for shaders that read it.Tested
maind592d54e1is merged into the branch (no rebase). The head ise24413c44.mainwrites request version 17 withstencilRefExportEnableas its last pixel field, sorasterPrimitiveis written after it and read from version 18: a version 17 request frommainstill reads, andShaderMemory.cppchecks the version 18 prefix, rejects 19 and truncates over all 20 trailing pixel bytes. The disk cache format is 28. The conflict resolutions match @blawesom's rebase (771c00783) except for that version.#4150, merged since, added
agc_driver_relative_ancillary. Its last case expected a whole-word read throughv_movrelsto be refused; #4150 notes that this PR composes that word. The one commit on top of the merge turns that case into a check of the composed word: the shader converts the word to float and exports it, and a triangle draw's word is 2, so the pixel is 255, with and without the implicit position inputs in front of the ancillary VGPR. In TechnicalDebt, the relative addressing entry no longer says that whole-word uses throw.agc_driver_pixel_ancillary_word,agc_driver_relative_ancillary,agc_driver_graphics,agc_driver_dual_source_blendandagc_driver_alpha_to_coveragepass.relative_ancillaryfailed on the bare merge (77f0a568b), before the test update, with "an unsupported ancillary word was accepted".e24413c44merged into the fork'smain: Linux passes 968 of 968 (lavapipe; 43 skipped for features lavapipe lacks), includingagc_driver_pixel_ancillary_word,agc_driver_relative_ancillary,pixel_ancillaryandagc_shader_memory. Windows passes 974 of 974, with the GPU tests skipped for lack of a device.ctest -E '^agc_driver|^video_out|^guest_video': 618 of 619 pass, includingpixel_ancillary,agc_shader_memoryandagc_shader_disk_cache.log_macrosfails there on every branch and passes in CI.tools/check_conventions.py --base origin/mainpasses.Earlier, rebased onto
maina11c8f7f7as one commit (db92ece3c):agc_driverset passes 296 of 297.agc_driver_emulated_color_comparefails the same way onmain(sRGB sampling refusal).b58c0428) onmain, tested by @ThatsLeo on Intel Arc (Mesa 26.2.3) and lavapipe (comment): all 43 Stray (PPSA02101) requests compile at a 32x budget and passspirv-val --target-env vulkan1.3. The two slow shaders of Shader preparation: masked-lane walk takes 150 to 400 s on Stray's large pixel shaders since fda4aa90 (#3279), and the title no longer starts #3504 take 1.0 and 2.2 s, and0x2013010000compiles at every budget.Before the rebase (on
mainplus #3279):Windows 11, RTX 5070 Laptop GPU (NVIDIA driver; the AMD iGPU is hidden from the Vulkan loader):
ctest -j4: 764 of 766 pass.guest_filesystemfails the same way on a tree without this commit, andguest_sce_nettimed out under load and passes on its own.agc_driver_pixel_ancillary_wordexports the word from a draw and reads back 2, 3 and 1 for the triangle, rectangle and line classes. The draw itself is a triangle and the request carries the class, asDrawDecodesets it; the test checksRasterPrimitiveOfon its own. It also compiles one whole-word shader and one that reads only the sample field (v_bfe_u32 v20, v0, 8, 4) for the three classes: the whole-word shader gets three modules of 629 SPIR-V words, and the sample-only shader shares one module of 616 words.pixel_ancillaryandagc_driver_pixel_ancillary_wordfail with the oldunsupported live userefusal. If the module key is keyed on ANCILLARY_ENA instead of on the read, the module check fails (sample field only: a line or rectangle draw built its own module).tools/check_conventions.pypasses.0x1b40e5c000compiled in Rohan03x's offline replay.0x20b8770000is refused onmain1e8fff6 and with fix(shader): drop masked selects proven unobserved through whole quad mode #3391 added, and compiles with this head (74,245 SPIR-V words). In game the per-frame skips of that draw are gone, with no GPU hang in two runs (2.5 and 4 min). Other draws on that frame are still skipped, so this shader's output is not checked yet.agc_driver_pixel_ancillary_wordpasses there. The one failure isagc_unit_shadow, which fails on this base without the change too (agc_driver_recorder_tests: partial publish drops a shadowed unit without copying it #1429, fixed onmainby fix(agc): compare host imports when device writes reach the write watch #3226).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)0x2013010000(Shader preparation: masked-lane walk takes 150 to 400 s on Stray's large pixel shaders since fda4aa90 (#3279), and the title no longer starts #3504; Game bring-up Stray (PPSA02100) on Linux #2461 is PPSA02100)🤖 Generated with Claude Code