Repository navigation
feat(agc): implement primitive-ordered pixel shaders with fragment shader interlock - #1130
Conversation
890cd80 to
cbbe157
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.
|
|
The linux job here didn't get to the build: it hung on |
adriadam10
left a comment
There was a problem hiding this comment.
Checked on current main (build + ctest 425/425, agc_driver_pixel_interlock runs on a device with fragmentShaderPixelInterlock; the new tests don't compile without the change). The linux job failed on a hung apt-get update; I re-ran it.
One thing before merging:
- SPV_EXT_fragment_shader_interlock requires each invocation to dynamically execute
OpBeginInvocationInterlockEXTand thenOpEndInvocationInterlockEXTexactly once. An End before everyOpReturn(SpirvFlowEmitter.cpp:91) is fine for that, butStopBdaInvocationIf(SpirvBdaFault.cpp:95) emitsOpKillin pixel shaders between Begin and End when a BDA read faults (the four call sites inSpirvBdaRead.cpp), so that invocation leaves the critical section without End. Please emit End before thatOpKillwhen the ordered mode is on, and add a test case where an ordered pixel shader takes the fault path.
Everything else (key/serializer v10, legacy request, TechnicalDebt placement) looks right. Note #423/#448/#1182/#1190 also take request version 10: whichever lands second moves to the next one.
cbbe157 to
6a9fef5
Compare
…ader interlock DecodeState and the draw precheck rejected DB_SHADER_CONTROL bit 16, PRIMITIVE_ORDERED_PIXEL_SHADER (POPS), which runs the ordered section of overlapping pixels' shaders in primitive order. The bit now reaches the recompiler as a pixel stage field, and such a pixel shader declares PixelInterlockOrderedEXT and runs its whole body between OpBeginInvocationInterlockEXT and OpEndInvocationInterlockEXT; an invocation that a BDA read fault kills ends the interlock first. The POPS collision SGPRs read 0, so a title skips its POPS wait and runs its per-collision loop once. An image_load still reads a sampled binding, which misses an earlier overlapping primitive's image_store in the same draw. The device enables VK_EXT_fragment_shader_interlock when it has fragmentShaderPixelInterlock; without it the recompile throws, as does a nonzero POPS_OVERLAP_NUM_SAMPLES. The field is part of the recompile cache key, the draw key and the serialized request (version 10).
6a9fef5 to
e1d4e69
Compare
adriadam10
left a comment
There was a problem hiding this comment.
main now uses request version 10 for the compute scratch size (#1182), so I rebased this onto it and moved the ordered-pixel field to version 11 (RequestSerializer.cpp write/read/limit, ShaderMemory.cpp: new prefix, unsupported versions 0 and 12, a legacy v10 request added, mapping offset unchanged at +163). Build + ctest 431/431 on Linux (RADV); the new tests don't compile without the change. The OpKill/End fix from the earlier review is in and the mutation without it fails.
Pushing the rebase now; merging once CI is green on the new head. The PR description still says "version 10": it is 11 now.
|
Thanks for doing the rebase and the move to version 11 yourself, and for checking the mutation. I've updated the description: it now says version 11 and lists which of the other request-changing PRs still write 10. I'll move our #1485 and #1492, which also wrote 10, to 12 and 13 so they don't collide with this one. |
What
DecodeStateand the draw precheck rejected DB_SHADER_CONTROL bit 16, PRIMITIVE_ORDERED_PIXEL_SHADER (field), which orders the pixel shaders of overlapping primitives.orderedPixelShader, and such a shader runs in aPixelInterlockOrderedEXTinterlock (extension) up to its return, not to ORDERED_PS_DONE. Its collision SGPRs read 0, so the title skips its POPS wait and runs its per-collision loop once (TechnicalDebt). An invocation that a BDA read fault kills ends the interlock before itsOpKill.fragmentShaderPixelInterlockthe recompile throws, as does a nonzero POPS_OVERLAP_NUM_SAMPLES. The field is in the cache and draw keys and the serialized request (version 11, after feat(agc): compute dispatches with scratch memory #1182's 10); the disk cache format stays 7, as only the key grows.ValidateShadersmode line withPixelInterlockOrderedEXT. feat(agc): vertex clip and cull distances #423, feat(agc): single-sample sample masks as a per-quad-pixel discard #448, feat(agc): render layered draws into 3D color targets #1077, feat(recompiler): FLAT addresses in the shared and private apertures reach LDS and scratch #1190, fix(shader): read s_memtime from the device clock where RADV's subgroup clock is 20 bits #1383 and test(shader): cover stage-specific float modes and legacy captures #1442 change the serialized request too; feat(agc): single-sample sample masks as a per-quad-pixel discard #448, feat(recompiler): FLAT addresses in the shared and private apertures reach LDS and scratch #1190, fix(shader): read s_memtime from the device clock where RADV's subgroup clock is 20 bits #1383 and test(shader): cover stage-specific float modes and legacy captures #1442 write version 10 as well, so they need a later number now, and whichever of them lands after this one needs 12 or more. feat(agc): implement overestimating conservative rasterization #1112 and feat(recompiler): bindless image tables up to the device's sampled-image limit #1184 add lines next to ours inVulkanDevice.cpp, feat(agc): implement overestimating conservative rasterization #1112, feat(recompiler): convert f32 to f16 natively when the device rounds 16-bit floats to nearest even #1252 and perf(agc): recycle descriptor pools after their last set is released #1333 intests/Graphics.cpp, and feat(recompiler): FLAT addresses in the shared and private apertures reach LDS and scratch #1190 inShaderMemory.cpp.Tested
main0e173a4: configure-DBUILD_TESTING=ON, full build,--target libs,ctest:ctest --output-on-failure)VK_DRIVER_FILES): 423/423 pass;agc_driver_flat_storeis left out there because it takes several minutes on Windows llvmpipeorderedPixelShaderTests(agc_driver_graphics) also walks every path of an ordered shader with BDA reads and requires oneOpEndInvocationInterlockEXTbefore each exit, 5 fault kills included; without the End inStopBdaInvocationIfit fails. It does not compile onmain.agc_driver_pixel_interlock: 64 overlapping triangles add 1 to a dword per pixel; every pixel counts 64 on an RTX 3080 and lavapipe (2 on the RTX 3080 without the interlock); when a BDA read fault kills every 8th column, the others still count 64. It does not compile onmain.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)