fix(vk): give programs that read storage buffer lengths descriptor sets on MoltenVK #644
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!644
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:mac/mvk-sizes"
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 · Needs: Windows, Linux
Why
On a Mac, MoltenVK leaves a pushed storage buffer's length at 0, whatever the buffer's size: it writes the sizes a shader
reads for
OpArrayLengthonly for a descriptor set bound withvkCmdBindDescriptorSets, never for a pushed one. Therenderer's shaders find which storage buffer holds a game address by those lengths, so in a pushed program no buffer ever
matched. The program still ran, but wrongly and silently: its writes and atomics to game memory were dropped, the atomics
returned 0, and reads the page table does not serve read 0. This PR gives every program whose shader reads a storage
buffer's length an ordinary descriptor set on MoltenVK, where the lengths are exact. Other drivers are unchanged. It is
part of the macOS port and stacks on #643. See #420.
How to reproduce
Seen on: macOS 26.7.1, Apple M4 Max, MoltenVK 1.4.2, arm64 and x86_64 under Rosetta: the NullRangeProbe GPU check,
revision 02 (
reports/2026-11-14-null-range-probe.md; main does not build on macOS yet)Settings and switches: defaults
Steps: in the game nothing is reported: the wrong results are silent. Game-free: in #643's tree (this PR's base), copy
in this PR's
tests/test_set_for_lengths.py,tests/set_for_lengths_ir.cppandtests/test_vendor_fixes.py, and runpython -I -B tests/test_set_for_lengths.py --cxx clang++ --spirv-headers <SPIRV-Headers/include> --headers <Vulkan-Headers/include> --jobs 1. On the GPU: the probe's pushed and set arms.Expected: a program that reads a storage buffer's length gets a set on MoltenVK; the probe's pushed lengths equal the
ranges bound.
Actual: the probe read 0 for every pushed storage buffer's
OpArrayLength, in every shape (single views, aliasedviews, LS's 88-byte push-constant range, a module from LS's own emitter), on both CPU routes; ordinary sets read the exact
lengths. The test pushes 17 of 138 layouts that need a set (
push, expected set).What changed
On MoltenVK a program whose SPIR-V reads a storage buffer's length (
OpArrayLength) in any stage is no longer pushed: itgets an ordinary set, as #643's storage buffer tables do. Each stage's SPIR-V is scanned once for that instruction where the
program cache already handles it, and a compute program read from the cache without its code is decided by its global
memory use until the code is read. A switch forces it either way, and the log says whether it is on and, at close, how many
programs it moved. The SPIR-V, the translator's stamp and the program cache's format are unchanged.
gpu/vk/vk_vendor.h: a new fix,VendorFixes::set_for_lengths, on by default whendriver_id == VK_DRIVER_ID_MOLTENVK, forced byRX_VK_SET_FOR_LENGTHS.gpu/vk/spirv_lengths.h(new, no Vulkan, likefallback_size_policy.h):spirv_lengths::array_length(words, count)walks a module once: 1 when it has anOpArrayLength, 0 when not, 1 for words that do not walk as a module.Stage::array_lengthingpu/vk/vk_shader.h(-1until the stage's SPIR-V is seen; not written to the cache file),noted in
gpu/vk/vk_progcache.cppat the three places the stage's phis are:program_cache_save(every translatedprogram, before it is published), a whole record's read, and
complete_from(a light compute program's code read later).layout_pushes(p, bindings, total)ingpu/vk/vk_draw.cpp: with the fix on, a program witharray_length > 0in anystage, or
< 0(not read yet) in a stage that uses global memory, gets a set; two counters keep the moves, anddraw_layouts_report()logs them once at close (mkwvk_close_askedingpu/vk/vk_main.cpp, after the program census).Outside segmented variants, which the cache never keeps, the back end reads a length only in its global memory search, so
the not-read-yet rule is a superset.
vk: vendor fixesline increate_devicegainsprograms reading storage buffer lengths in descriptor sets, not pushed %d.RX_VK_SET_FOR_LENGTHSrow indocs/development.md;gpu/vk/VULKAN.md's Descriptors bullet names bothMoltenVK exceptions.
tests/test_set_for_lengths.pyandtests/set_for_lengths_ir.cpp(new) build six real modules through theback end (
CollectShaderInfoPass,EmitSPIRV: compute and vertex global memory with a storage buffer, 18OpArrayLengtheach; global memory with nothing to search; direct storage in compute, vertex and fragment stages, none),recount each module's
OpArrayLengthin Python, and compilevk_draw.cppfromkPushSizeto the end ofensure_layoutwith their descriptors and notes: 138 layouts on MoltenVK and ten other drivers, the switch unset,0and
1, stages not read yet, and the bindings the same pushed or not. They also check the scan on words that are not amodule and the counts kept for the close line.
tests/test_vendor_fixes.pycovers the new fix on 17 devices and checksthat both layout fixes stay out of
translator_stamp()andmake_profile(), thatlayout_pushesreads the notes, andthat the log line, the close and the three notes are there.
tests/test_set_for_tables.py's stub stage getsarray_length = 0; the program cache fixtures (tests/program_cache_one_variant_test.cpp,tests/program_cache_cleanup_test.cpp) includespirv_lengths.h, as they compilevk_progcache.cpp's functions.Measurements
Verdict: passes: the new test fails on #643's head (17 of 138 layouts pushed where a set is needed, and no scan) and
passes at this head, two interleaved runs each. In the game (history, an integration build) the close line counted 2,023
programs moved.
Baseline: #643 @
77a91fc9, this PR's base in the stack. Main lacks #643'slayout_pushes, which this PR extends, so mainagainst this PR would measure #643 too.
Hardware: macOS 26.7.1, Apple M4 Max, 16-core CPU, 64 GB, Apple clang 21.0.0 (game-free: no GPU, no game)
Scene: none for the tests (a layout fix: its proof is the test). The game row below is history: start 12's precompile,
windowless, defaults
Method:
tests/test_set_for_lengths.py(--jobs 1) andtests/test_vendor_fixes.pyrun as this head, base, this head,base (arm64, fresh HOME and TMPDIR,
tools/slot.py), the base tree with this PR's test files copied in (SHA-256 equal)How to reproduce:
python -I -B tests/test_set_for_lengths.py --cxx clang++ --spirv-headers <SPIRV-Headers/include> --headers <Vulkan-Headers/include> --jobs 1andpython -I -B tests/test_vendor_fixes.py --cxx clang++ --headers <Vulkan-Headers/include>, in this PR's tree and in #643's with the test files copied in77a91fc9e90ba9a6tests/test_set_for_lengths.py, arm64, 2 runs eachpush, expected set; no count kept; nospirv_lengths.htests/test_vendor_fixes.py, arm64, 2 runs eachlayout_pushes does not read dev.fixes.set_for_lengthsHistory, not this head: start 12 (
reports/2026-11-14-mac-startup-12.md, arm64, integration build a7711f3c, whichcarries this commit as
bf5ca961), the first start whose precompile laid out real pipelines: the close line counted2,023 programs given a set for their storage buffer lengths. The cause was measured by the NullRangeProbe GPU check,
revision 02, on both CPU routes (
reports/2026-11-14-null-range-probe.md): with a push layout everyOpArrayLengthread 0and the guarded loads were skipped; with an ordinary set every length was exact. Whether the game draws differently with
this change on was not compared.
Flags and settings
One new switch,
RX_VK_SET_FOR_LENGTHS; its default is on with MoltenVK only. (RX_VK_SET_FOR_TABLES, in this PR's diffuntil #643 merges, is #643's and unchanged here.)
RX_VK_SET_FOR_LENGTHS0or1: a program whose SPIR-V reads a storage buffer's length in any stage gets an ordinary descriptor set instead of a pushed one0off,1on, whatever the driver=1on another driver=0only to compare on a Mac,=1only to try the set path on another GPUTesting
Tested: on macOS 26.7.1, Apple M4 Max, at
e90ba9a6, this PR's head before the rebases ontoe090cc48andaa95d35f(main
6c9edffeplus #643 and this PR), game-free:tests/test_set_for_lengths.pyandtests/test_vendor_fixes.py, arm64: pass at this head (2 runs each); fail on #643'shead with this PR's test files (2 runs each), as in Measurements. The failing side ran on arm64 only (the owner's rule for
these fixtures' negatives, after the employer's endpoint agent killed an x86_64 negative in an earlier lane).
clang++ -arch x86_64, run under Rosetta):test_set_for_lengths(6 real modules, 138 layouts) and
test_vendor_fixes(17 devices) pass.test_set_for_tables' x86_64 fixture waskilled (SIGKILL, exit -9, no crash report: the endpoint agent [inference]); by policy it was not retried. It passes here
on arm64 (79 layouts) and, as #646's superset, on x86_64 at the top of the stack.
tests/test_program_cache_one_variant.pyandtests/test_program_cache_cleanup.py(the fixtures this PR touches) failat this head and on main
6c9edffealike, on arm64 and x86_64: they compile main'sposix_fadviseread-ahead hint, whichmacOS lacks (#476 gives it a Darwin path). With a test-only prelude declaring the hint as a no-op (given through the
compiler wrapper, the same on both sides) both pass at this head on arm64 and x86_64, and on main on arm64 (one variant:
11 modules whole, 1 light, the same variant for 9 picks; cleanup: 9 tests OK).
ensure_layout,make_pool,layout_pushesand their neighbours), throughtools/ls-suite.pyat this head and on main6c9edffe: here123 passed and 10 failed, main 121 passed and 10 failed. The same 10 fail on both (
test_appimage_player_build,test_crash_dialog,test_pcache_header_read,test_pipeline_cache_save,test_pipeline_play_cache,test_pipeline_tour,test_precompile_compute_guard,test_program_cache_cleanup,test_program_cache_one_variant,test_shader_module_guard: macOS gaps on main, the last two Windows-only checks); the only status changes are thisstack's two new tests passing (
related-P26-e90ba9a6/compare-main-names-summary.txt).vk_draw.cpp,vk_main.cppandvk_progcache.cppcompiled for arm64 and x86_64 at this head with main's renderer flags(
nx2/build.py'svflagsplus-arch) and a compile-only prelude declaringposix_fallocateandposix_fadvise,which main's renderer calls and macOS lacks (#476 gives them Darwin paths): exit 0 for all six.
nx2/translator_files.pystamp()) isebbf160ce9626041on main and at this head.git merge-treeof this head with each open macOS PR's head (#431, #437, #438, #479, #480, #490, #491, #492): noconflict.
reports/PubWave-files/PubMvk/proof/P26-*.log,related-P26-*,objects2-P26-*,merge-tree.txt.aa95d35f, atf08eb702(e90ba9a6before the rebases ontoe090cc48andaa95d35f):Sweep3's checks of this head (the C/C++ it changes compiled, and its own tests run, on arm64 and x86_64 where they
apply) gave the same results as before;
python -I -B tests/run_all.pyon wave B's mergedb0e2e16, which carriesthis head: 569/32/18 (passed/failed/skipped) against main aa95d35f's 543/33/17 on this Mac, no new failure
(
reports/2026-11-15-rebase-sweep-3.md).78ec1c47, which carries every MacPR through #708 and the I41 fixes: a start from the field-dawn save to a 1,197 s hold in play, the runner's 12 of 12
checks passed, 0 skipped draws in the hold, 0 XPC errors, a clean exit (
reports/2026-11-15-mac-perf-E9.md); x86_64under Rosetta 2: run #34 on I38
3f872b0c: 16 m 52 s of play, 19 of 19 checks, 0 skipped draws, 1,906 draws a frame,no device loss, a clean exit (
reports/2026-11-14-mac-startup-34-x86_64.md).… programs reading storage buffer lengths in descriptor sets, not pushed 1 …; at close(line 65749):
vk: storage buffer lengths (RX_VK_SET_FOR_LENGTHS): 11472 programs given a descriptor set, not pushed, because a stage reads a storage buffer's length, which MoltenVK leaves 0 in a pushed set (0 of them laid out before their code was read, by their global memory use). Under Rosetta, #34's 19 of 19 checks stand for it; that log's linesfor this PR are not quoted here.
Not tested: not built on Windows or Linux; its test request covers it. The game at this head (per-PR game runs are
not planned; the stack's runs with it, E9a on arm64 and #34 under Rosetta, are under Tested); the game with
RX_VK_SET_FOR_LENGTHS=0against unset, to see what the dropped writes did on screen; the cost of the extra sets(
RX_VK_PROF_FINE=1counts them); KosmicKrisp. The audit's other open items are outside this PR: an empty size bufferpicking up stale bytes, a pushed texel size query, and the prebuilt FSR 3 frame interpolation SPIR-V (back-end pipelines
with layouts of their own, which never pass
layout_pushes). Windows and Linux: nothing was built or run there; theirlayouts do not change (the test's ten other drivers).
Must be tested:
f08eb702, it carries #643) and mainaa95d35fthe same way, start each tothe title and quit, expecting the build to succeed, the
vk: vendor fixesline to endprograms reading storage buffer lengths in descriptor sets, not pushed 0, novk: storage buffer lengthsline at close, and every other line to matchmain's (by hand: test request T-HG4R)
Risks
What could break: on MoltenVK these programs (2,023 in start 12) now allocate, write and bind a set per draw or
dispatch instead of a push, and leave Metal's 31 discrete buffer slots; that cost is unmeasured. A light compute program
with global memory but no length gets a set it does not need until its code is read (counted apart at close). On every
platform each stage's SPIR-V gets one more walk over its words where the program cache already walks them for phis; on
other drivers nothing else changes.
Turning it off:
RX_VK_SET_FOR_LENGTHS=0(MoltenVK's pushed lengths then read 0 again), or revert the commit.Compatibility: Nothing changes in saves,
longshot_options.inior the vk ABI (MKWVK_API_VERSION). The shader cachesare kept: none of the changed files is in the translator's stamp (
ebbf160ce9626041before and after), and the note isnot written to the program cache, whose format is unchanged. On Windows and Linux the
vk: vendor fixesline has one morefield (
0). On macOS the moved programs' pipelines change layout, so Metal compiles them once.Reviewer notes
e90ba9a6(this PR's one commit, cherry-picked with-xfrommacos/mvk-sizesbf5ca961):layout_pushesand
draw_layouts_reportinvk_draw.cpp, then the three notes invk_progcache.cpp,spirv_lengths.hand the stagefield.
77a91fc9, and shows its commit too until #643 merges. #645 and #646stack on this one.
57f4eaec) gavecomplete_fromitsbusypath and #321(
76699aad) moved the cache file: the notes followmake_moduleas before, and a busy return reaches the note when thecode is read again. The three places a stage's SPIR-V is noted are still every place the program cache reads or saves it.
gpu/vk/vk_main.cpp'svk: vendor fixeslogf(the topic's basealso had #650's two fields there): one
logfwith #643's field and this PR's, without #650's. Anddocs/development.md:this PR's
RX_VK_SET_FOR_LENGTHSrow after #643's row and before main'sRX_VK_NATIVE_FADDrow (#1917ec25fd1rewrote that row; the topic's side also carried #649's
RX_VK_TEXEL_CODECSrow, left to #649).logf(a conflict ingit merge-treeagainst its branch tipf5905178): whichever lands second keeps onelogfholding every field, in order (set_for_tables, set_for_lengths, worker_caches, pcache_no_load, as the integration did). The plan has this PR rebase after #648 and #650.tests/test_program_cache_cleanup.pyandtests/test_program_cache_one_variant.pyfail on macOS on main6c9edffeandhere alike: their fixtures compile main's
posix_fadvisecall, which macOS lacks (#476 fixes that). With the hintdeclared as a no-op both pass here (arm64, x86_64) and on main (
proof/P26-program_cache_*-fadvise-*.log).tests/test_set_for_lengths.py's fixture (presence_words,seg_layout_count) when it lands, as the integration's071c628fdid.Issues
See #420
Checklist
Everyone
type(scope): description) of the status line's type.main: onaa95d35f(#761, 2026-11-15), headf08eb702, on #643ca17df3b, without aconflict; this PR's own commits, old → new:
e90ba9a6→f08eb702; the commit names elsewhere in this descriptionare from before the rebase.
python -I -B tests/run_all.pypasses (any failure that also fails onmainis named). (On the wave's merge of every macOS PR built with this one,453eb678: 553 passed, 32 failed, 18 skipped against main 6c9edffe's 531/32/17 on this Mac; the failures are main's own excepttest_split_ub_flush, which #658's tree at the time broke and its head fixes;test_collect_widenow passes; the new tests pass andtest_darwin_audio_service, Darwin-only, is skipped by the runner;reports/PubWave-files/waveA/. After the rebase onto mainaa95d35f(headf08eb702): wave B's mergedb0e2e16, 569/32/18 against main aa95d35f's 543/33/17, no new failure.)third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.mainand passing here. (Failing on #643's head, this PR's base.)tests/that reads changed source updated in this PR, not weakened.e90ba9a66cef43dffcc8ef43dffcc8f08eb70206WIP: fix(vk): give programs that read storage buffer lengths descriptor sets on MoltenVKto fix(vk): give programs that read storage buffer lengths descriptor sets on MoltenVKFolded into #847 (fix(vk): MoltenVK's descriptor limits in descriptor sets)
Pull request closed