Skip to content

Pass sort buffer addresses through push constants - #96

Open
natevm wants to merge 6 commits into
masterfrom
fix/sort-descriptor-lifetime
Open

natevm wants to merge 6 commits into
masterfrom
fix/sort-descriptor-lifetime

Conversation

@natevm

@natevm natevm commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Sort calls update shared descriptor sets that earlier compute submissions may still use. Pass buffer device addresses through push constants so each recorded dispatch retains its addresses.

Remove all sort descriptor sets, their pool, and their layouts. The shared host/shader structure contains six device addresses and the existing sort parameters in 80 bytes. The shader's radix-sort algorithm is unchanged. Memory barriers order the sort stages and reuse of scratch on the same queue.

Remove lastSortSubmission and waitForSort; buffer resize and destruction have no sort-specific state or waits. Callers use the existing synchronization APIs before host access, resizing, destruction, or access from another queue. Growing an existing scratch allocation and releasing internally allocated scratch use Context::synchronizeCompute(). Sufficient caller-provided scratch retains its size and device address, and those sorts remain asynchronous.

Validation after rebasing onto master 2c6ee03: Windows Release sort, buffer-resize, and context-destruction tests pass on AMD Radeon AI PRO R9700 with synchronization validation enabled and no reported Vulkan validation errors. The sort test checks CPU-matched keys and payloads for 1, 511, 1024, 2049, and 524289 elements, including duplicates, high-bit keys, UINT64_MAX, partial workgroups, repeated submissions across command-buffer reuse, shared scratch, scratch growth, and omitted scratch for both APIs. It explicitly synchronizes before resizing or destroying caller-owned buffers.

An additional AMD run matches a CPU reference for 1,048,576, 8,388,608, and 100,000,000 64-bit keys, both with and without payloads, with synchronization validation enabled and no reported Vulkan validation errors. The installed NVIDIA 582.16 driver lacks VK_KHR_maintenance11, now required by master, so the rebased build cannot run there. Earlier NVIDIA runs passed both sort modes through 8,388,608 elements; the sort shader and shared parameter layout are unchanged since those runs.

spirv-val --target-env vulkan1.2 passes. SPIR-V inspection confirms the host/shader offsets and zero descriptor bindings. Linux, shared-library builds, and timeout injection were not tested in this revision.

Performance comparison against 1da86fa: identical Release builds, validation disabled, preallocated scratch, GPU timestamps around the sort command buffer, and separate CPU submission timing. Before/after/after/before runs collected 50 samples per revision for random inputs and 200 for repeatedly sorted inputs. Both GPUs passed all correctness checks. NVIDIA GPU time improved in seven of eight cases in each dataset. AMD results were mixed with substantial timing variability.

Steady-state GPU medians in milliseconds; negative deltas mean less time:

Elements Mode AMD before → after Delta NVIDIA before → after Delta
1,048,576 Keys 1.0445 → 1.0791 +3.3% 0.5037 → 0.4656 -7.6%
1,048,576 Payload 1.2846 → 1.2652 -1.5% 0.5991 → 0.5508 -8.1%
8,388,608 Keys 8.4091 → 8.2017 -2.5% 3.4681 → 3.1857 -8.1%
8,388,608 Payload 20.4385 → 21.2700 +4.1% 7.0474 → 7.3296 +4.0%

CPU submission time decreased in 15 of 16 steady-state cases across the GPUs. For NVIDIA's 8,388,608-element payload case, it fell from 0.1256 to 0.1101 ms. The same case's GPU time increased by 1.0% with random inputs. These measurements cover the full revision, including barrier changes; they do not isolate the effect of device-address loads. GPU clocks were not locked.

@natevm
natevm force-pushed the fix/sort-descriptor-lifetime branch from 4c805de to 1da86fa Compare October 10, 2026 14:13
@natevm
natevm marked this pull request as ready for review October 10, 2026 14:16
Comment thread gprt/gprt.cpp Outdated
Comment thread gprt/gprt.cpp Outdated
Comment thread gprt/gprt.cpp Outdated
@natevm natevm changed the title Protect pending sort descriptors and buffer lifetimes Pass sort buffer addresses through push constants Oct 10, 2026
@natevm
natevm force-pushed the fix/sort-descriptor-lifetime branch from 212ae82 to 2854fa0 Compare October 10, 2026 23:40
@natevm
natevm force-pushed the fix/sort-descriptor-lifetime branch from 2854fa0 to ef4bece Compare October 11, 2026 03:22

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant