Repository navigation
feat(shader): implement triangle-patch tessellation - #1491
adriadam10 merged 4 commits into
Conversation
9849333 to
680c202
Compare
|
Tested on top of main Head AI-assisted: yes (Claude Code). |
680c202 to
7d8b561
Compare
7d8b561 to
55473d0
Compare
adriadam10
left a comment
There was a problem hiding this comment.
agc_driver_tessellation crashes on RADV, so this can't go in as is. The crash happens inside the driver, but on main these draws throw a clean error, while with this PR the whole process dies on the default Linux AMD driver.
- Where: Linux, Mesa 26.2.4 RADV, Radeon 660M (gfx1035, RDNA2, 64-wide subgroups). The PR is merged onto
main787239e. The test segfaults invkCreateGraphicsPipelineson its first draw (RequirePatches→Graphics::Pipeline::Pipeline). It fails the same way under ACO and underRADV_DEBUG=llvm,nonggandnooptimizations. It passes on CI's lavapipe (Mesa 25.2.8). - The input is valid:
spirv-val --target-env vulkan1.3accepts all four modules, and the Khronos validation layer (1.4.357) reports nothing before the crash. So this is a RADV bug, but this PR is the first thing that sends tessellation pipelines to it. - What triggers it (I swapped single modules with
MESA_SPIRV_READ_PATH): the crash needs both the hull module and the domain module this PR emits. If either one is swapped for a hand-written GLSL stage with the same interface, the crash goes away. If both are run throughspirv-opt -O(which folds the runtime-computed indices, e.g.(32752 - 32752) >> 4for the patch output, and theUMod/UDivcomponent indices into constant access chains), the crash also goes away, but then the level-0 patch is not culled. I haven't checked whether that last part comes from spirv-opt or RADV. - Suggested direction: reduce it to the construct RADV can't handle. The first suspects are the dynamically indexed components of the per-vertex and patch outputs/inputs when the address is already a constant. Emitting constant access chains in that case would avoid it, and would also give simpler SPIR-V. Then check that the level-0 patch is still culled on RADV. If you have no AMD GPU on Linux, I can rerun a new head here. A reduced case is also worth filing with Mesa.
To reproduce: MESA_SPIRV_DUMP_PATH=<dir> ctest -R agc_driver_tessellation on RADV dumps the four modules (.vert/.tesc/.tese/.frag). Put modified copies with the same names in a directory and point MESA_SPIRV_READ_PATH at it to swap them.
…eam) - AGC shader binary type 3 (Hs, front+back bundle) is accepted where HsBack (7) is - a type 3 header over a registered HsBack keeps the HsBack snapshot (its user data) and only contributes its registers to the graphics ABI - port of PR boykopovar#1491's front-half decode: the Local stage's s_setpc_b64 s[6:7] hand-off to the back half ends the program Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1W4jdNSKTV7xwuS6xQPJV
…ar#1491 onto main) Port of boykopovar#1491 (Celegans12) onto main 197ffac. Conflicts resolved: DrawKeyRegisters keeps main's 0x005 range and adds 0x286-0x287; the Local front-half decode moved into the cached frontend; the Local prologue keeps main's drawIndex with the RelativeIndex tessellation base. Known failure: agc_driver_tessellation ("push constant range crosses a stage slot") since main's fixed push slots (boykopovar#2661). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1W4jdNSKTV7xwuS6xQPJV
…eam) - AGC shader binary type 3 (Hs, front+back bundle) is accepted where HsBack (7) is - a type 3 header over a registered HsBack keeps the HsBack snapshot (its user data) and only contributes its registers to the graphics ABI - port of PR boykopovar#1491's front-half decode: the Local stage's s_setpc_b64 s[6:7] hand-off to the back half ends the program Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1W4jdNSKTV7xwuS6xQPJV
…eam) - AGC shader binary type 3 (Hs, front+back bundle) is accepted where HsBack (7) is - a type 3 header over a registered HsBack keeps the HsBack snapshot (its user data) and only contributes its registers to the graphics ABI - port of PR boykopovar#1491's front-half decode: the Local stage's s_setpc_b64 s[6:7] hand-off to the back half ends the program Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1W4jdNSKTV7xwuS6xQPJV
DecodeShaderStages accepts LS, HS and an NGG passthrough ES, reads the 3-bit VGT_TF_PARAM fields, refuses its other bits but DISTRIBUTION_MODE, and needs HOS levels that keep [1, 63]; 0x286-0x287 join the draw key. Local and domain programs are prepared with their stage group: its headers are merged over one set of defaults, and a local program left unbound by the hull header is prepared at its first draw. The local program ends at its jump into the hull half. TessellationLowering lowers LDS and ring addresses that are affine in the control point and relative index to tessellation interfaces with strides from the code, and refuses accesses outside the patch, per-patch writes from other control points and a modified primitive export. Hull lanes read their own saved exec bit, also of host-subgroup-masked copies; other masks are refused, gl_InvocationID loads as a scalar and factors clamp to 63.
…reparation at use Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ation Co-Authored-By: Claude <noreply@anthropic.com>
55473d0 to
c8f8014
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Since there was no new push, I went through the points from my last review myself and pushed them to this branch on top of a rebase onto current main:
- Rebase: resolved the conflicts with #639's shared frontend (
DecodeStageProgramnow feedsPreparedControlFlow), the embedded-fetchdrawIndex(v3 keeps the relative index), the per-address shader registry, andmain's preparation at first use, which replaces the special case for local programs.DrawKeyRegistersis now 48. - Tests:
Tessellation.cppuses the fixed per-stage push constant slots (PixelPushLayout/PixelPushOffset).PreparedGraphics.cppnow expects a second tessellation configuration of a local program to be prepared at its draw rather than throw, which matches whatmaindoes now. - RADV crash: reduced with
MESA_SPIRV_READ_PATH. RADV 26.2.4 crashes when the slot index of a hull per-vertex input ((address % stride) >> 4) is dynamic. A dynamic component index is fine. The lowering always producesinvocation * stride + cfor those reads, soTessellationPointernow indexes them as[invocation + c / stride][c % stride / 16][c % 16 / 4]with constant slot and component.agc_driver_tessellationpasses on RADV (660M, gfx1035), including the level-0 patch being culled. It also passes on CI's lavapipe (Mesa 25.2.8), and the Khronos validation layer reports nothing. - TechnicalDebt: local programs are now prepared at their first draw once per tessellation configuration, so a later configuration no longer throws.
Checked: scoped build + ctest on RADV, 570/570 passed (VideoOut window tests skipped, no display).
What
Every tessellated draw throws today:
DecodeShaderStagesrejects PRIMGEN_PASSTHRU_EN, the local program ends in an unresolvables_setpc_b64, and hull and domain programs keep unlowered ring bases. Since #639 a title's local programs also stop registration.VGT_TF_PARAMbits except DISTRIBUTION_MODE, and needs HOS levels that keep [1, 63] (AMD's Sea Islands 3D register reference); 0x286-0x287 join the draw key.ResolveGraphicsStagesAbimerges the headers over one set of defaults, which used to reset the hull's LS_HS_CONFIG and TF_PARAM. A local program the hull header leaves at address 0 is prepared at its first draw.TessellationLoweringlowers LDS and ring addresses affine in the control point and relative index, with strides from the code; accesses outside the patch, per-patch writes not from control point 0, other relative-index uses and a non-passthrough primitive export throw.decodeDraw.Tested
main4b6ab6b: configure-DBUILD_TESTING=ON, full build,--target libs,ctest:ctest --output-on-failure)VK_DRIVER_FILES): 479/479 pass;agc_driver_flat_storeis left out there because it takes several minutes on Windows llvmpipeagc_driver_tessellation: level-1, 0 and 5 patches through hand-assembled programs (coverage, culling, interior points, winding), six refusals, and title-shaped LS/HS/DS headers through registration andResolveGraphicsStagesAbi; fails on main.agc_driver_prepared_graphics: a hull ABI without its local program, then that program prepared at its first draw; new rows inagc_driver_graphics; both fail on main.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)