WIP: fix(vk): the MoltenVK nesting bound counts only the reads SPIRV-Cross certainly makes #692
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
beanstalk: approved @ 1697603
beanstalk: approved @ 2c5a62e
beanstalk: approved @ 4fd82b7
beanstalk: approved @ 64d6d09
beanstalk: approved @ 6f91fae
beanstalk: approved @ 70a78f9
beanstalk: approved @ 8d361dc
beanstalk: approved @ 9d8356e
beanstalk: approved @ abf2227
beanstalk: approved @ d82f202
beanstalk: approved @ e5f1e40
beanstalk: approved @ ee8ab5f
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!692
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:mac/msl-depth2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Type: fix · Risk: low · Tested on: macOS 26.7.1, Apple M4 Max (MoltenVK 1.4.2), game-free at this head; GPU probe MD-02 on both CPU routes (history) · Needs: Windows, Linux, macOS game run
Why
#676 keeps SPIRV-Cross, MoltenVK's translator to Metal, from nesting an expression deeper than Metal's compiler takes (256
brackets). Its model called a value a temporary (printed once by name, no nesting) wherever two instructions used it. But
SPIRV-Cross counts a read where it takes the value's text, and some users take it late or not at all: a scalar extract of a
forwarded vector reads the vector only where the extract is printed, two extracts of one vector rebuilt into a vector are one
swizzle (one read), and an extract nobody uses is never printed. So a chain of calls returning such vectors still nests without
bound while the first model saw a temporary at every level, and Metal refuses those modules as before. This PR makes the model
count only the reads SPIRV-Cross certainly makes, so every chain past 32 levels is broken. It is part of the macOS port. See
#420.
How to reproduce
Seen on: macOS 26.7.1, Apple M4 Max, MoltenVK 1.4.2 (SPIRV-Cross
6c09849f), arm64 and x86_64 (Rosetta)Settings and switches: defaults
Steps: game-free: make a compute pipeline from
tests/msl_depth_ir.cpp'sdepth-combineordepth-deadmodule (300 callsof a pure function returning a
uvec2, rebuilt from both extracts, or with the.yextract unused) with #676's bound applied,as the probe MD-02 does (
python -I -B tests/test_msl_depth.py … --probe <dir>writes its inputs)Expected: both pipelines are made
Actual: on the base both are refused:
program_source:27:2200: fatal error: bracket nesting level exceeded maximum of 256(
depth-combine) andprogram_source:27:3144(depth-dead), exactly where the dumped MSL's 257th bracket opens (602 and 301deep); #676's bound leaves both modules as they are. Whether the game's refused programs (Mac start 14) have this shape is not
known: they were not read
What changed
On MoltenVK only (the bound's flag, unchanged),
BoundForwardedDepthnow calls a value a temporary only where SPIRV-Crosscertainly reads it twice; every other value is forwarded, so its depth is its deepest operand's plus one, and a chain past the
bound (32) is materialized as before. A read too few only makes the model deeper; its temporaries are a subset of the first
model's, so it bounds every chain the first one did. Off MoltenVK nothing runs.
gpu/vk/eden/shader_recompiler/backend/spirv/msl_depth.cpp:Reach(the blocks a branch reaches from the first);Reads(per reached instruction, the operands whose text it takes as it is emitted: every operand of an operation, call,composite,
OpCopyObject, comparison, atomic or group operation,OpStore's value, a branch's selector, an imageinstruction's coordinate, GLSL.std.450's operands); a vector's scalar extracts counted both as the places that print them and
as reads of their own, the fewer taken; phis, loads, access chains and unknown instructions not counted. The model's header
cites SPIRV-Cross
6c09849f's lines for each rule.gpu/vk/vk_shader.cpp:translator_stamp's MoltenVK word"msld"becomes"msl2", so programs cached under thefirst model are translated again.
gpu/vk/VULKAN.md, the bound's paragraph (only certain reads count; the first model's miss).tests/msl_depth_ir.cpp:build_pair_chain, thedepth-combineanddepth-deadmodules (300 levels each, written wordby word, with CPU references), registered beside #676's four chains.
tests/test_msl_depth.py: its independent model follows (the reached blocks, the reads from SPIR-V's grammar classes andoperand names, the bracket estimate as SPIRV-Cross reads) and checks the two new chains.
Measurements
Verdict: fixes it on the GPU (history: MD-02, 4 of 4 predictions on both CPU routes: the two chains #676 leaves are
refused at their 257th bracket, and made and exact with this bound); game-free at this head the two chains fall from 906 and
603 levels to 32, and #676's four chains, their tables and the back end's 139 fixture modules are byte for byte as before.
Speed: not measured (3 instructions and a variable per materialized value: 39 and 19 in the two chains, none in the fixture).
Hardware: macOS 26.7.1, Apple M4 Max, 64 GB, Apple clang 21 (game-free at this head; GPU results are the probe's history)
Scene: none (synthetic deep modules and the back end's fixture, emitted, validated and run on the CPU interpreter)
Method:
tests/test_msl_depth.pyat this head, arm64 and--arch x86_64(Rosetta), and with--treeagit archiveof thisPR's base, which must fail;
tests/test_seg_shader.py --base-treethe same archive, arm64 and x86_64. On the GPU: probe MD-02(MslDepth2's report), one admission per route, at the topic's commit
337546f9. Exact checks: one run per route.How to reproduce:
python -I -B tests/test_msl_depth.py --headers <SPIRV-Headers/include> --spirv-val spirv-val [--arch x86_64] [--tree <git archive of the base>]Baseline: the "before" column is this PR's base, #676 (
1c21f295), whose bound this PR corrects (main has no bound).1c21f2956d50fd71depth-combine,depth-deadwith the bound, arm64 and x86_64test_msl_depth(this PR's test)--treethe base): "depth-combine: 906 deep with the bound, over 32; depth-dead: 603 deep with the bound, over 32"DEPTHlines)test_seg_shader), flag off, against the base, arm64 and x86_64test_msl_depth): 139 the same, deepest 12depth-combine-off,depth-dead-offnot made (bracket nesting level exceeded, 27:2200 and 27:3144)-onmodules made (22 and 17 deep), 64 of 64 words exactFlags and settings
None in this PR's own commits: the bound follows the driver (MoltenVK), with no switch. The diff against main also shows
the switches of the PRs it stacks on, theirs to list:
HOS_KEEP_FREED_PAGES,HOS_ZERO_ALL_PAGES,RX_NVN_WATCH_AFTER,RX_NVN_WATCH_ALL,RX_NVN_WATCH_TEXELS_SRC(#480),RX_VK_LIBRARY(#438),RX_VK_DLSS_ACTORS,RX_VK_GPU_DUMP,RX_VK_MESH_IND_COUNT_WORD,RX_VK_NO_UPSCALE_STUB(#658),RX_VK_TEXEL_CODECS(#649),RX_VK_SET_FOR_TABLES(#643)and
RX_VK_SET_FOR_LENGTHS(#644).Testing
Tested: on macOS 26.7.1, Apple M4 Max, at this head (main
6c9edffe, #676's stack, this PR), game-free, every run under theshared check slot with a fresh HOME and TMPDIR:
tests/test_msl_depth.py, arm64 and x86_64: exit 0, as in Measurements; with--treethe base (this PR's test and fixture,the base's back end): fails as required.
tests/test_msl_depth.py --probeat this head writes MD-02's 20 frozen inputs (the 12 modules, their 6 expected-word files,the table and
depth-probe.json) byte for byte, so MD-02's result holds for this head's modules.tests/test_seg_shader.py --base-treethe base, arm64 and x86_64: exit 0, 139 of 139 identical.test_amd_subgroup_profiles,test_program_stamp_knobs(19 switches),test_program_cache_format: exit 0.msl_depth.cppandvk_shader.cppcompiled whole with main 6c9edffe's renderer flags for arm64 and x86_64: 4 of 4,exit 0 (scratch tree with #476, #492 and #490).
objects, 1,216 name/arch checks, closure 565 symbols a route, 0 undefined.
both; the topic's full suite with only a timing test's load failure. Every changed line here is the topic's, line for line.
reports/PubWave-files/PubShader2/(proof/md2-6d50fd71-*.log,keep/md2-6d50fd71-probe,compile/md2-msl-depth2-6d50fd71/) andreports/PubWave-files/PubShader3/(equal-lines.txt,stamps.txt,offapple-md2-1c21f295-6d50fd71.txt,merge-tree.txt,wave/).Not tested: the game at this head (its refused programs were not read), and no GPU run at this head (MD-02 is history, on
modules byte-identical to this head's). Windows and Linux: not built there (both changed source files differ from the base's off
Apple:
msl_depth.cppis compiled everywhere); its test request covers it.Must be tested:
once (
msl2in the stamp), nobracket nesting level exceededline, and 0 programs left unmade by it (by hand: the owner,the log's
vk:and[mvk-error]lines)6d50fd71, it carries #676 with the PRs it stacks on) and main6c9edffethesame way and start each to the title, and run
python -I -B tests/run_all.pyon Linux (where the back end's checks run),expecting the build to compile this PR's changed
msl_depth.cppandvk_shader.cpp, the shaders prepared once more (a newtranslator stamp), and every other line to match main's (by hand: test request T-XXXX)
Risks
What could break: on MoltenVK only, the pass now materializes more values (where the old model wrongly saw a temporary): a
fault would throw at translation (the program fails loudly) or change a value (the CPU interpreter's runs of every deep chain
and the fixture match the reference with and without the bound). Other drivers: nothing runs; only the stamp changes.
Turning it off: revert the PR (no switch: the bound follows the driver; #676's first model comes back).
Compatibility: every platform's shaders are prepared again once (the back end is in the translator's stamp:
02ba705207103076→991689a2f695b7f8); on MoltenVK the translator stamp's word changes from"msld"to"msl2".kProgramCacheFormatstays 3. Nothing changes in saves,longshot_options.inior the vk ABI (MKWVK_API_VERSION).Reviewer notes
1c21f295..6d50fd71, one commit,-xfrommacos/msl-depth2(337546f9), picked without a conflict: its changedlines are the topic's line for line in all 5 files (
macos/integrated52f9a18c). Readmsl_depth.cpp'sReadsfirst,then
test_msl_depth.py'sreads.1c21f295): it corrects that bound's model. #693 stacks on this head(its scope bound runs after this one; its stamp word follows
"msl2").tests/test_amd_subgroup_profiles.py: the MoltenVK constant's comment and thecase list, from #676's line; keep both drivers' lines, as
macos/integrated).git merge-treewith #694,#695, #696, #697 and #677-#681, #686: clean. With #693' siblings the stamp's order is
"msl2","mslb","sgv1","gmlp","lzlp". The conflicts with #650, #652 (and #666, #667 on them) inVULKAN.md,vk_main.cppand others are the base's, as in #676's notes.msl_depth.cppis compiled everywhere and the stamp changes, so every platform prepares its shaders once;the pass runs only on MoltenVK. Not split for that reason.
reports/2026-11-14-*.md(the port's design notes), which are not in this repository.Issues
See #420
Checklist
Everyone
type(scope): description) of the status line's type.main(maine090cc48).python -I -B tests/run_all.pypasses (any failure that also fails onmainis named). (On the wave's merge77333ce0: #693 on this PR, #697 with #696, #694 and #695, #698 with #654, and one fixture commit for the cut-outmake_profilestubs: 565 passed, 31 failed, 18 skipped against main 6c9edffe's 531/32/17 on this Mac; the 31 failures are main's own and nothing that passes on main fails;test_collect_widenow passes; 33 new tests pass,test_msl_depthamong them;test_darwin_audio_service, Darwin-only, is skipped by the runner;reports/PubWave-files/PubShader3/wave/full-77333ce0/.)third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.mainand passing here. (It fails on this PR's base, #676, with--tree;on
maintoo, which has no bound.)tests/that reads changed source updated in this PR, not weakened.Every scan down vk_memory.cpp's import table by start (pool_lookup's covering import and merge plan, bg_drain, prefetch_chunks, import_in_background, memory_note_gpu_write and memory_guest_imported) stopped once an import ended more than 1 GiB below its address, on the belief that no import is larger than that. pool_lookup's own merge escape leaves an import above 1 GiB over the smaller ones inside it ("left overlapping"), and the device allows imports up to max_import_size: a scan stopped at a small import inside a large one, missed the large one, and the same bytes were imported again, two memories over the same bytes. import_reach.h counts the live imports by span class; its bound is above every live span, within twice the largest. imported_add and imported_take, now the table's only ways in and out, keep the count. A scan stops only where no import further down can reach its address, so it also stops sooner than before when the imports are small. The covering-import checks in pool_lookup and import_in_background look past the last import at or below the range for the same reason. Tests: test_import_reach.py runs the actual scans with a 4 GiB import over a 16 MiB chunk, left by the merge escape (imports are recorded ranges: no payload, no Vulkan, no game). Its lookup, query, note, drain, background and prefetch cases fail on main5e072fc8and pass here, and a source check keeps every change to the table in the two helpers. test_import_windows runs pool_lookup with the helpers; test_mirror_gpu_only names the new publication point. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>044635ea added vma_startup_rollback (vk_vma.cpp, run after a failed replay_start), which calls vmaDestroyAllocator. vk_vma.cpp includes vk_work.h last so that RX_VK_RECORDER=defer/thread drains before its destructions, and every such boundary call in a vk_work.h file must be in RX_REC_BOUNDARIES with a drain-first macro. This one was not, and test_recorder_sync caught it ("vk_vma.cpp calls ['vmaDestroyAllocator'], which vk_work.h does not drain before"): it passes at 58c5c5aa (044635ea's parent) and fails at 044635ea. Add it to the list and to vk_work.h, next to vmaDestroyBuffer and vmaDestroyImage. With the recorder off (the default) boundary() is a relaxed load and a branch. Rollback runs before any image or buffer exists, so nothing should be held there today, but the allocator is no longer destroyed while a recorder could still hold packets. The test is unchanged (sources: 42 boundaries drained, was 41). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>A guest span larger than one device allocation (the cap on MoltenVK/M4 Max is 41,747,087,360 bytes) is bound as pieces: one descriptor per allocation segment, at the descriptor's own binding (reports/2026-11-14-full-range-design.md sections 4.1-4.4). This is the shader half; the draw side (descriptors, layout, gathers) comes later. - runtime_info.h: SegEntry {Storage|Table|TexBuf|ImgBuf, desc, pieces} and RuntimeInfo::segmented. - Back end: a segmented storage buffer is an array of pieces at its binding (a table count * pieces, entry k's piece j at k * pieces + j); its loads, stores and atomics call DontInline helpers made after main, which find the piece by the pieces' OpArrayLength in order, with constant indices. The global memory fallbacks sum the pieces' lengths for a segmented descriptor's bound end and branch to its piece (single, native candidate, table written out and in the loop). Texel and image buffers are arrays too, their fetches, reads, writes and atomics through helpers that pick the piece by OpImageQuerySize, and a texel buffer's size query sums the pieces. ImageQuery was declared for every module already. With RuntimeInfo::segmented empty nothing new is made: the SPIR-V is byte-identical. - vk_shader: SegSig (entries, hash) on Program, seg_sig and seg_variant; program_segmented finds or translates the variant of a plain program with plain's texture questions and answers and checks its descriptors are plain's; program_pick, mesh_program_locked, program_find_plain, loaded_variants and portable_variant keep the plain and segmented variants apart; every stage's RuntimeInfo::segmented comes from Program::seg. - vk_progcache: a segmented variant is never written to the program cache nor listed among the loaded programs. - tests/test_seg_shader.py + tests/seg_shader_ir.cpp: synthetic IR programs through the real back end, plain and segmented; structure checks, spirv-val (Vulkan 1.3, uniform buffer standard layout), and with --base-tree a byte comparison of the plain modules against an older tree. Five source-reading checks follow the new Program field and the atomics' new branch. The translator stamp changes with this (eden/, vk_shader.cpp, sizeof(Program)): every program cache is rebuilt once. Land it in the same release as the vk_api.h change that already does so. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit b7928876dab846485f667c1c934d34d25ecef035)The interfaces the four segmented-consumer lanes (SegBind, SegXfer, TexelDesc, NullShader; reports/2026-11-14-seg-consumer-plan.md section 3) code against, frozen before they fork. Inert: no caller of anything new, no SPIR-V, layout, push range or stream change, sizeof(Program) unchanged. - vk_internal.h / vk_memory.cpp, SegMem's consumer interface: gpu_span_any() (a relaxed count of the live domains, stored in seg_commit; domains are kept for the run), gpu_span (gpu_buffer's flavour of gpu_span_bind: import buffers, no written marks, no mirror), SpanLayout::bind_stamp (gpu_bind_stamp(): a segment can be imported again with its cuts kept, so cut_gen alone never keys piece buffers), span_unused_piece's semantics ({null_storage, 0, 1}), and the SegArena API (SegArenaRef, seg_arena_take, seg_gather, seg_scatter; stub bodies after stream_alloc that give no room and log, SegBind's to fill). - Review fixes in the same interface (renderer-core review P2-3, P3.2): every piece of gpu_span_bind carries its buffer's device address (the import's or the mirror's; 0 only with bufferDeviceAddress off), and its one-piece path stores the translated guest address, not a virtual one. - vk_texture.h/.cpp: texel_view_pieces / texel_piece_views, stubs that return 1 and false (TexelDesc's to fill). - vk_shader.h: Program::presence_words, in what was padding. - Back end: PRESENCE_PUSH_OFFSET (88), Profile::emulate_null_descriptors, Bindings::presence / presence_read with the bit numbering rule. - tests: test_resolver_coverage lists gpu_span_bind / gpu_span sites too (scoped to gpu/vk: gpu/render has an rx::gpu_span of its own), checks that gpu_span follows its own gpu_buffer, and splits SITES into SITES_BASE plus one block per lane. test_segmented_domains executes the contract (the count 0, 1, 2 -> 1 on a merge, 0; gpu_span's pieces; addresses with the fixture's gpu_buffer_bind returning none as the real one does; the bind stamp; a virtual address's guest). test_seg_shader accepts a base tree of this back end's generation and compares every module. - docs/segmented-binding.md: the gate (every new behaviour Apple-only, other platforms byte-identical) and one heading per lane. Conflict on this branch (not with main): tests/test_seg_shader.py's docstring, against the array_pointer_calls paragraph #P20's msl-arrayptr port added; both kept, this paragraph first. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 95fa06d4e6526601bd2e1bc88c2f8f5720855350)The segmented single storage array's case (plan 4.3 item 5) had no negative: the check always cut its own tree. With --source <B2 archive> it fails as required ("moltenvk.segmented_single want=set got=push"). Conflict with #P28 (read_write textures; not with main): the docstring keeps #P28's paragraph with this usage line, and main() cuts from --source's vk_draw.cpp with #P28's read_write count (as in the macOS integration, Integration 17). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 5824f6828e21cf810dcceaa6be425a9224b15994)plaine37c097da2plaina997b3629aDefineImages and DefineImageBuffers declare one image whatever the descriptor's count, while Image() indexes the variable when the count is above 1, so such a module fails spirv-val ("OpAccessChain reached non-composite type"); B2 has it already, on every platform. The null_img* and null_imgbuf* helpers take their element the same way and share it. Found while extending the fixture; declaring the arrays changes every platform's SPIR-V and is left to the owner. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 83e7e18b4fee477f1e6901871ad37efe8f23d5bf)SPIRV-Cross (MoltenVK 1.4.2's pin 6c09849f) prints an OpImageRead's result, and a pure read helper's call, where the value is first used, and nothing orders it against the same invocation's later writes and atomics on that image: in MSL the read ran after them and returned the later value. TexOrder's TO-01 probe proved it on the GPU for LS's plain output, with and without the NullShader bit, on both CPU routes (reports/2026-11-14-tex-order.md). - Profile::materialize_image_reads, set by make_profile on Apple for MoltenVK only, with its translator stamp word ("rord") only when on. - emit_spirv_image.cpp InPlace: with the flag, every storage image and image buffer read's result (the plain read, the segmented read helper's call, a carrier codec's element reads) is stored to a Private variable at the read and loaded back; SPIRV-Cross prints a store where it stands. Private, since a Function variable must lead its function's first block; in the entry point's interface from SPIR-V 1.4. Off: no new ID, every module byte for byte as before. - tests/seg_shader_ir.cpp: order-* modules with the flag; the read-in-place rule (every read and read helper call stored at once), their flag-off twins breaking it, CPU runs equal to the modules without the flag, and the flag alone changing nothing without image reads. test_seg_shader.py: order- modules required; a base tree without the flag builds them with TEX_ORDER_BASE and they are not compared; --source against it fails. - tests/spirv_run.h runs Private variables (fresh per invocation) and OpMemoryBarrier (a no-op one invocation at a time). - docs/segmented-binding.md: a TexOrder section. Conflicts on mac/null-shader (#P33), with MslArrayPtr's tests that #P33's last commit carries, not with main; resolved as macos/integrated did (Integration 22, b08694d8): tests/seg_shader_ir.cpp keeps both sections and both calls (msl_arrayptr, then tex_read_order); tests/spirv_run.h keeps both storage classes (Uniform, then Private); tests/test_seg_shader.py keeps both docstring paragraphs and presence checks, leaves 'msl-' and 'order-' out of an older generation's set, and compares with a base in one loop (an order- module skipped for a base without the flag, identical counts as the same, a different segmented one listed under --changed-segmented, anything else fails). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit a9aad5bcd6f4cce3d9fcff8b231da9628e7da4b3)6d50fd71ead68dbc742ad68dbc742aa67e1934ecFolded into #848 (feat(vk): the SPIR-V back end on MoltenVK and guest spans past one device allocation)
Pull request closed