From 23b5a8134516e0529d4bc9c404aee52b74d97f79 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 07:40:14 +0000 Subject: [PATCH 1/2] Check every Vulkan result instead of running on past failures 52 vk* call sites, 26 of which return a VkResult, and none were checked. A failed call left an unusable handle in place and execution carried on, which showed up in three ways: - the VM died. With no Vulkan driver present, vkCreateInstance failed, the null instance went to vkEnumeratePhysicalDevices, and the process aborted (exit 134) with no Ruby-level error. Same shape in create_buffer: when no memory type was both host-visible and host-coherent the search fell back to index 0, and map_buffer then returned NULL straight into a write loop. - the error named the wrong cause. vkCreateComputePipelines leaves the handle VK_NULL_HANDLE on failure, which ensure_pipeline read as "the .spv is missing" -- telling the user to run `make -C shader` for shaders that were already built and had been rejected by the driver. - the numbers were quietly wrong. An unchecked vkQueueSubmit meant a lost device never signalled its fence, vkWaitForFences returned at once, and the caller read the unwritten buffer back as ordinary Floats. Adds VK_CHECK / gpu_check, raising a RuntimeError naming the call and the VkResult. gpu_init and dispatch_compute now take mrb_state so they can raise; map_buffer too, and it no longer returns NULL. Pipeline state is recorded per pipeline (PIPE_MISSING_SPV vs PIPE_CREATE_FAILED) so the two cases give opposite advice. A failed gpu_init sets init_failed rather than being retried on every later operation, which would leak a fresh context each time. Since mrb_raise unwinds, buffers are now wrapped before the dispatch that fills them -- the GC can only free what it owns. That also covers the case where GPU::SFloat.cast is handed a non-numeric element. narray_sum's partial buffer is wrapped for the same reason, so destroy_buffer is now only the finalizer and create_buffer's own unwind path. Two silent-truncation guards in the same family: GPU::SFloat.new now rejects a size past UINT32_MAX rather than wrapping it into a different length, and a dispatch past maxComputeWorkGroupCount is rejected rather than left to the driver -- lavapipe runs it and returns the right answer, so the limit is easy to miss in testing, while a hardware-bounded driver may not. GPU.info gains :max_workgroups so callers and bug reports can see the bound. Verified on Mesa lavapipe: 46 tests pass (2 new), plus the new test/shader_error_test.rb in both modes. It is a separate file because it has to break the process-wide GPU context. Checked by hand that the no-device path now raises and leaves the VM alive where it previously aborted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JAgXRCCQ43jwXzeXKS6hG4 --- CONTRIBUTING.md | 20 ++++ README.md | 14 ++- src/gpu_buffer.c | 70 ++++++++++---- src/gpu_internal.h | 35 ++++++- src/gpu_narray.c | 94 ++++++++++++------ src/gpu_vulkan.c | 197 +++++++++++++++++++++++++++++++------- test/narray_test.rb | 19 ++++ test/shader_error_test.rb | 63 ++++++++++++ 8 files changed, 423 insertions(+), 89 deletions(-) create mode 100644 test/shader_error_test.rb diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9ab6838..33b3e16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,6 +59,26 @@ git-ignored — do not commit them. parity yet). Please add cases there for any behavior you change or add, and make sure it prints `ALL TESTS PASSED` before opening a PR. +`test/shader_error_test.rb` is separate because it has to break the GPU context, +which is a process-wide singleton — running it alongside the main suite would +leave every later test without pipelines. Run it on its own: + +```sh +./build/host/bin/mruby test/shader_error_test.rb # missing .spv files +./build/host/bin/mruby test/shader_error_test.rb # .spv the driver rejects +``` + +## Error handling + +Every Vulkan call that returns a `VkResult` goes through `VK_CHECK`, which +raises an mruby exception instead of leaving an unusable handle behind. Please +keep new calls checked — an unchecked failure surfaces either as a crash of the +whole VM or, worse, as a plausible-looking wrong number. + +Because `mrb_raise` unwinds, anything already allocated has to be released +*before* the check. In particular, wrap a new `GpuBuffer` with `wrap_buffer` +before the dispatch that fills it, so the GC owns it if the dispatch raises. + ## Reporting issues GPU behavior is environment-specific, so please include: diff --git a/README.md b/README.md index c82e11d..bf35c00 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ built on it: | Spectral | `#rfft` → `GPU::SComplex`; `#magnitude(k)`, `#power_spectrum(k)` | | Host transfer | `#to_a`, `#head(k)` | | Metadata | `#size` / `#length`, `#shape`, `#ndim` | -| Device | `GPU.info`, `GPU.device_name`, `GPU.init(dir)` | +| Device | `GPU.info` (device, API version, backend, `:max_workgroups`), `GPU.device_name`, `GPU.init(dir)` | Data lives in a `VkBuffer` the whole time; the only host copies happen in `#to_a` / `#head`. Arithmetic, reduction and the FFT are Vulkan compute dispatches. @@ -154,6 +154,18 @@ buffer of `2n` floats (interleaved re/im, wrapped as `GPU::SComplex`): passes. 3. `cmag` reduces the spectrum to a real `GPU::SFloat` of magnitudes or powers. +### Limits and failures + +One operation is one dispatch, covering 256 elements per workgroup up to the +device's `maxComputeWorkGroupCount` — `GPU.info[:max_workgroups] * 256` +elements, about 16.7M where that limit is 65535. Larger arrays raise rather +than leaving the answer up to the driver. + +Failed Vulkan calls raise `RuntimeError` with the call and the `VkResult`. +A missing shader and a shader the driver rejects are reported differently: the +first tells you to build the shaders, the second tells you not to bother, +because the SPIR-V was already there. + ## Roadmap - **FFT growth**: the radix-2 transform above is in place. Next: an inverse transform, diff --git a/src/gpu_buffer.c b/src/gpu_buffer.c index ff84f98..ba80291 100644 --- a/src/gpu_buffer.c +++ b/src/gpu_buffer.c @@ -5,10 +5,10 @@ * Raspberry Pi 5 the GPU memory is unified, so a mapped pointer is a cheap * view of the same bytes the shader reads/writes. * - * Two ownership models share one GpuBuffer type: - * - wrap_buffer() -> owned by a Ruby object, freed by the GC finalizer. - * - create_buffer() + destroy_buffer() -> caller-managed scratch buffers - * (used for reduction partials that never become Ruby objects). + * Every buffer ends up owned by a Ruby object and freed by the GC finalizer. + * Callers wrap as soon as create_buffer returns, before anything that can + * raise, so a Vulkan failure unwinds without stranding GPU memory. + * destroy_buffer is the finalizer itself, plus create_buffer's own unwind. */ #include "gpu_internal.h" @@ -19,10 +19,16 @@ static void gpu_buffer_free(mrb_state *mrb, void *p) { const struct mrb_data_type gpu_buffer_type = {"GPU::NArray", gpu_buffer_free}; -/* ---- Create a host-visible FP32 buffer of n elements ---- */ +/* ---- Create a host-visible FP32 buffer of n elements ---- + * + * Raises rather than returning a half-built buffer. Because a raise unwinds + * out of this function, each step releases what earlier steps created before + * handing the failure to gpu_check. */ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) { GpuBuffer *buf = mrb_malloc(mrb, sizeof(GpuBuffer)); buf->n = n; + buf->buffer = VK_NULL_HANDLE; + buf->memory = VK_NULL_HANDLE; buf->bytes = sizeof(float) * (VkDeviceSize)n; if (buf->bytes == 0) buf->bytes = sizeof(float); /* avoid zero-size allocation */ @@ -32,31 +38,57 @@ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) { .usage = VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, .sharingMode = VK_SHARING_MODE_EXCLUSIVE }; - vkCreateBuffer(g_ctx.device, &bi, NULL, &buf->buffer); + VkResult r = vkCreateBuffer(g_ctx.device, &bi, NULL, &buf->buffer); + if (r != VK_SUCCESS) { + mrb_free(mrb, buf); + gpu_check(mrb, r, "vkCreateBuffer"); + } VkMemoryRequirements req; vkGetBufferMemoryRequirements(g_ctx.device, buf->buffer, &req); + /* The mapped pointer is how every host read and write reaches this buffer, + * so a memory type that is not both host-visible and host-coherent is no + * use. Defaulting to index 0 when none matches would allocate from an + * arbitrary heap and turn every later map_buffer into a NULL dereference. */ VkPhysicalDeviceMemoryProperties mem_props; vkGetPhysicalDeviceMemoryProperties(g_ctx.physical_device, &mem_props); - uint32_t mem_idx = 0; + const VkMemoryPropertyFlags want = + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT; + uint32_t mem_idx = UINT32_MAX; for (uint32_t i = 0; i < mem_props.memoryTypeCount; i++) { - if ((req.memoryTypeBits & (1 << i)) && - (mem_props.memoryTypes[i].propertyFlags & - (VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) == - (VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) { + if ((req.memoryTypeBits & (1u << i)) && + (mem_props.memoryTypes[i].propertyFlags & want) == want) { mem_idx = i; break; } } + if (mem_idx == UINT32_MAX) { + vkDestroyBuffer(g_ctx.device, buf->buffer, NULL); + mrb_free(mrb, buf); + mrb_raise(mrb, E_RUNTIME_ERROR, + "no host-visible, host-coherent memory type is available for a storage buffer"); + } VkMemoryAllocateInfo ai = { .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, .allocationSize = req.size, .memoryTypeIndex = mem_idx }; - vkAllocateMemory(g_ctx.device, &ai, NULL, &buf->memory); - vkBindBufferMemory(g_ctx.device, buf->buffer, buf->memory, 0); + r = vkAllocateMemory(g_ctx.device, &ai, NULL, &buf->memory); + if (r != VK_SUCCESS) { + vkDestroyBuffer(g_ctx.device, buf->buffer, NULL); + mrb_free(mrb, buf); + gpu_check(mrb, r, "vkAllocateMemory"); + } + + r = vkBindBufferMemory(g_ctx.device, buf->buffer, buf->memory, 0); + if (r != VK_SUCCESS) { + vkFreeMemory(g_ctx.device, buf->memory, NULL); + vkDestroyBuffer(g_ctx.device, buf->buffer, NULL); + mrb_free(mrb, buf); + gpu_check(mrb, r, "vkBindBufferMemory"); + } return buf; } @@ -77,10 +109,16 @@ mrb_value wrap_buffer(mrb_state *mrb, struct RClass *klass, GpuBuffer *buf) { return mrb_obj_value(data); } -/* ---- Host mapping helpers (coherent memory, no flush needed) ---- */ -float *map_buffer(GpuBuffer *buf) { +/* ---- Host mapping helpers (coherent memory, no flush needed) ---- + * + * Never returns NULL: every caller writes through the pointer immediately, so + * an unreported map failure would be a NULL dereference inside the VM. */ +float *map_buffer(mrb_state *mrb, GpuBuffer *buf) { float *mapped = NULL; - vkMapMemory(g_ctx.device, buf->memory, 0, buf->bytes, 0, (void **)&mapped); + VK_CHECK(mrb, vkMapMemory(g_ctx.device, buf->memory, 0, buf->bytes, 0, (void **)&mapped)); + if (!mapped) { + mrb_raise(mrb, E_RUNTIME_ERROR, "vkMapMemory returned no pointer"); + } return mapped; } diff --git a/src/gpu_internal.h b/src/gpu_internal.h index 339e3d8..78b70cb 100644 --- a/src/gpu_internal.h +++ b/src/gpu_internal.h @@ -38,6 +38,16 @@ typedef enum { typedef enum { LAYOUT_3BUF = 0, LAYOUT_2BUF, LAYOUT_1BUF, LAYOUT_COUNT } LayoutId; +/* Why a pipeline handle is missing. Without this, "the .spv file is absent" + * and "the driver rejected the shader" both look like VK_NULL_HANDLE, and the + * second one gets reported as the first -- telling the user to run a build + * step that has already run. */ +typedef enum { + PIPE_READY = 0, /* created and usable */ + PIPE_MISSING_SPV, /* .spv could not be read */ + PIPE_CREATE_FAILED /* the driver rejected it; see pipe_error[] */ +} PipeState; + /* ---- GPU Context (singleton) ---- */ typedef struct { VkInstance instance; @@ -49,8 +59,12 @@ typedef struct { VkDescriptorSetLayout desc_layouts[LAYOUT_COUNT]; VkPipelineLayout pipe_layouts[LAYOUT_COUNT]; VkPipeline pipelines[PIPE_COUNT]; + PipeState pipe_state[PIPE_COUNT]; + VkResult pipe_error[PIPE_COUNT]; VkDescriptorPool desc_pool; + uint32_t max_workgroups; /* maxComputeWorkGroupCount[0] */ int initialized; + int init_failed; /* a previous gpu_init raised; do not retry */ } GpuCtx; extern GpuCtx g_ctx; @@ -65,19 +79,32 @@ typedef struct { extern const struct mrb_data_type gpu_buffer_type; +/* ---- Error handling ---- + * + * Every Vulkan call that returns a VkResult goes through VK_CHECK, which turns + * a failure into an mruby exception. Unchecked, a failed call leaves an + * unusable handle behind and the next call either segfaults the VM or -- worse + * -- returns whatever happened to be in the buffer as if it were a result. + * + * gpu_check raises, so it does not return on failure: anything the caller + * allocated must be released *before* the call. */ +void gpu_check(mrb_state *mrb, VkResult r, const char *call); +const char *gpu_result_name(VkResult r); +#define VK_CHECK(mrb, expr) gpu_check((mrb), (expr), #expr) + /* ---- gpu_vulkan.c ---- */ -void gpu_init(const char *shader_dir); +void gpu_init(mrb_state *mrb, const char *shader_dir); const char *gpu_pipe_name(PipeId pipe_id); -void dispatch_compute(PipeId pipe_id, +void dispatch_compute(mrb_state *mrb, PipeId pipe_id, VkBuffer *buffers, VkDeviceSize *sizes, int num_buffers, const void *push_data, uint32_t push_size, uint32_t group_x, uint32_t group_y, uint32_t group_z); /* ---- gpu_buffer.c ---- */ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n); -void destroy_buffer(mrb_state *mrb, GpuBuffer *buf); /* for un-wrapped scratch buffers */ +void destroy_buffer(mrb_state *mrb, GpuBuffer *buf); /* GC finalizer + create_buffer unwind */ mrb_value wrap_buffer(mrb_state *mrb, struct RClass *klass, GpuBuffer *buf); -float *map_buffer(GpuBuffer *buf); +float *map_buffer(mrb_state *mrb, GpuBuffer *buf); void unmap_buffer(GpuBuffer *buf); #endif diff --git a/src/gpu_narray.c b/src/gpu_narray.c index 98aa627..095bc5b 100644 --- a/src/gpu_narray.c +++ b/src/gpu_narray.c @@ -25,16 +25,29 @@ /* ---- lazy initialization ---- */ static void ensure_initialized(mrb_state *mrb) { - (void)mrb; - if (!g_ctx.initialized) gpu_init(GPU_NARRAY_SHADER_DIR); + if (!g_ctx.initialized) gpu_init(mrb, GPU_NARRAY_SHADER_DIR); } static void ensure_pipeline(mrb_state *mrb, PipeId pipe) { ensure_initialized(mrb); - if (g_ctx.pipelines[pipe] == VK_NULL_HANDLE) { - mrb_raisef(mrb, E_RUNTIME_ERROR, - "shader '%s.spv' is not compiled. Run `make -C shader` first.", - gpu_pipe_name(pipe)); + switch (g_ctx.pipe_state[pipe]) { + case PIPE_READY: + return; + case PIPE_MISSING_SPV: + mrb_raisef(mrb, E_RUNTIME_ERROR, + "shader '%s.spv' is not compiled. Run `make -C shader` first.", + gpu_pipe_name(pipe)); + break; + case PIPE_CREATE_FAILED: + /* The SPIR-V was there and the driver refused it, so telling the user + * to build the shaders would send them after the wrong problem. */ + mrb_raisef(mrb, E_RUNTIME_ERROR, + "the Vulkan driver rejected shader '%s.spv': %s (VkResult %d). " + "The SPIR-V was found and loaded, so this is a device or driver " + "limitation, not a missing build step.", + gpu_pipe_name(pipe), gpu_result_name(g_ctx.pipe_error[pipe]), + (int)g_ctx.pipe_error[pipe]); + break; } } @@ -55,6 +68,12 @@ static mrb_value sfloat_s_new(mrb_state *mrb, mrb_value self) { mrb_int n; mrb_get_args(mrb, "i", &n); if (n < 0) mrb_raise(mrb, E_ARGUMENT_ERROR, "negative array size"); + /* Element counts are uint32_t on the GPU side; without this the cast below + * would wrap and hand back an array of a completely different length. */ + if ((uint64_t)n > (uint64_t)UINT32_MAX) { + mrb_raisef(mrb, E_ARGUMENT_ERROR, "array size %i exceeds the maximum of %i", + n, (mrb_int)UINT32_MAX); + } ensure_initialized(mrb); GpuBuffer *buf = create_buffer(mrb, (uint32_t)n); return wrap_buffer(mrb, mrb_class_ptr(self), buf); @@ -68,12 +87,16 @@ static mrb_value sfloat_s_cast(mrb_state *mrb, mrb_value self) { mrb_int n = RARRAY_LEN(ary); GpuBuffer *buf = create_buffer(mrb, (uint32_t)n); - float *m = map_buffer(buf); + /* Hand the buffer to the GC before anything that can raise -- mapping can + * fail, and mrb_as_float rejects a non-numeric element. An unwrapped buffer + * belongs to nobody, so a raise here used to leak it. */ + mrb_value result = wrap_buffer(mrb, mrb_class_ptr(self), buf); + float *m = map_buffer(mrb, buf); for (mrb_int i = 0; i < n; i++) { m[i] = (float)mrb_as_float(mrb, mrb_ary_ref(mrb, ary, i)); } unmap_buffer(buf); - return wrap_buffer(mrb, mrb_class_ptr(self), buf); + return result; } /* ========================================================================= @@ -83,7 +106,7 @@ static mrb_value sfloat_s_cast(mrb_state *mrb, mrb_value self) { /* #to_a -> Ruby Array of every element (host copy) */ static mrb_value narray_to_a(mrb_state *mrb, mrb_value self) { GpuBuffer *buf = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); - float *m = map_buffer(buf); + float *m = map_buffer(mrb, buf); mrb_value ary = mrb_ary_new_capa(mrb, buf->n); for (uint32_t i = 0; i < buf->n; i++) { mrb_ary_push(mrb, ary, mrb_float_value(mrb, (mrb_float)m[i])); @@ -99,7 +122,7 @@ static mrb_value narray_head(mrb_state *mrb, mrb_value self) { GpuBuffer *buf = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); if (k < 0) k = 0; if ((uint32_t)k > buf->n) k = buf->n; - float *m = map_buffer(buf); + float *m = map_buffer(mrb, buf); mrb_value ary = mrb_ary_new_capa(mrb, k); for (mrb_int i = 0; i < k; i++) { mrb_ary_push(mrb, ary, mrb_float_value(mrb, (mrb_float)m[i])); @@ -119,7 +142,7 @@ static mrb_value narray_seq(mrb_state *mrb, mrb_value self) { mrb_float start = 0.0, step = 1.0; mrb_get_args(mrb, "|ff", &start, &step); GpuBuffer *buf = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); - float *m = map_buffer(buf); + float *m = map_buffer(mrb, buf); for (uint32_t i = 0; i < buf->n; i++) { m[i] = (float)(start + step * (double)i); } @@ -132,7 +155,7 @@ static mrb_value narray_fill(mrb_state *mrb, mrb_value self) { mrb_float v; mrb_get_args(mrb, "f", &v); GpuBuffer *buf = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); - float *m = map_buffer(buf); + float *m = map_buffer(mrb, buf); for (uint32_t i = 0; i < buf->n; i++) m[i] = (float)v; unmap_buffer(buf); return self; @@ -151,12 +174,15 @@ static mrb_value binop_nn(mrb_state *mrb, mrb_value self, GpuBuffer *rhs, PipeId } ensure_pipeline(mrb, pipe); GpuBuffer *c = create_buffer(mrb, a->n); + /* Wrapped before dispatching: dispatch_compute raises on a Vulkan failure, + * and the GC can only free what it owns. */ + mrb_value result = wrap_buffer(mrb, mrb_obj_class(mrb, self), c); VkBuffer bufs[3] = {a->buffer, rhs->buffer, c->buffer}; VkDeviceSize sizes[3] = {a->bytes, rhs->bytes, c->bytes}; uint32_t push = a->n; - dispatch_compute(pipe, bufs, sizes, 3, &push, sizeof(uint32_t), + dispatch_compute(mrb, pipe, bufs, sizes, 3, &push, sizeof(uint32_t), (a->n + 255) / 256, 1, 1); - return wrap_buffer(mrb, mrb_obj_class(mrb, self), c); + return result; } /* scalar op: PIPE_SCALE gives b = a * scalar, PIPE_ADDS gives b = a + scalar */ @@ -164,12 +190,13 @@ static mrb_value scalar_op(mrb_state *mrb, mrb_value self, float scalar, PipeId GpuBuffer *a = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); ensure_pipeline(mrb, pipe); GpuBuffer *b = create_buffer(mrb, a->n); + mrb_value result = wrap_buffer(mrb, mrb_obj_class(mrb, self), b); VkBuffer bufs[2] = {a->buffer, b->buffer}; VkDeviceSize sizes[2] = {a->bytes, b->bytes}; struct { uint32_t n; float s; } push = {a->n, scalar}; - dispatch_compute(pipe, bufs, sizes, 2, &push, sizeof(push), + dispatch_compute(mrb, pipe, bufs, sizes, 2, &push, sizeof(push), (a->n + 255) / 256, 1, 1); - return wrap_buffer(mrb, mrb_obj_class(mrb, self), b); + return result; } static mrb_value type_err(mrb_state *mrb, mrb_value o, const char *op) { @@ -224,8 +251,9 @@ static mrb_value narray_neg(mrb_state *mrb, mrb_value self) { } /* #sum -> Float. GPU produces one partial per workgroup; host sums them - * in double precision. The partial buffer is scratch (not a Ruby object), - * so it is freed explicitly. */ + * in double precision. The partial buffer is wrapped and left to the GC -- + * it is never returned, but owning it means a raise from the dispatch or the + * mapping cannot strand it. */ static mrb_value narray_sum(mrb_state *mrb, mrb_value self) { GpuBuffer *a = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); if (a->n == 0) return mrb_float_value(mrb, 0.0); @@ -233,17 +261,17 @@ static mrb_value narray_sum(mrb_state *mrb, mrb_value self) { uint32_t groups = (a->n + 255) / 256; GpuBuffer *partial = create_buffer(mrb, groups); + wrap_buffer(mrb, mrb_obj_class(mrb, self), partial); VkBuffer bufs[2] = {a->buffer, partial->buffer}; VkDeviceSize sizes[2] = {a->bytes, partial->bytes}; uint32_t push = a->n; - dispatch_compute(PIPE_SUM, bufs, sizes, 2, &push, sizeof(uint32_t), + dispatch_compute(mrb, PIPE_SUM, bufs, sizes, 2, &push, sizeof(uint32_t), groups, 1, 1); - float *pm = map_buffer(partial); + float *pm = map_buffer(mrb, partial); double total = 0.0; for (uint32_t i = 0; i < groups; i++) total += (double)pm[i]; unmap_buffer(partial); - destroy_buffer(mrb, partial); return mrb_float_value(mrb, (mrb_float)total); } @@ -277,15 +305,15 @@ static mrb_value narray_rfft(mrb_state *mrb, mrb_value self) { uint32_t log2n = 0; while ((1u << log2n) < n) log2n++; - /* Resolve the class before allocating, so nothing can raise between - * create_buffer and wrap_buffer (the buffer is untracked until wrapped). */ - struct RClass *scomplex = gpu_class(mrb, "SComplex"); + /* Wrapped up front: each of the log2(n) dispatches below can raise, and the + * GC can only free a buffer it owns. */ GpuBuffer *x = create_buffer(mrb, 2 * n); + mrb_value result = wrap_buffer(mrb, gpu_class(mrb, "SComplex"), x); VkBuffer bitrev_bufs[2] = {a->buffer, x->buffer}; VkDeviceSize bitrev_sizes[2] = {a->bytes, x->bytes}; struct { uint32_t n, log2n; } bitrev_push = {n, log2n}; - dispatch_compute(PIPE_FFT_BITREV, bitrev_bufs, bitrev_sizes, 2, + dispatch_compute(mrb, PIPE_FFT_BITREV, bitrev_bufs, bitrev_sizes, 2, &bitrev_push, sizeof(bitrev_push), (n + 255) / 256, 1, 1); VkBuffer stage_bufs[1] = {x->buffer}; @@ -293,11 +321,11 @@ static mrb_value narray_rfft(mrb_state *mrb, mrb_value self) { uint32_t groups = (n / 2 + 255) / 256; for (uint32_t h = 1; h < n; h <<= 1) { struct { uint32_t n, h; } stage_push = {n, h}; - dispatch_compute(PIPE_FFT_STAGE, stage_bufs, stage_sizes, 1, + dispatch_compute(mrb, PIPE_FFT_STAGE, stage_bufs, stage_sizes, 1, &stage_push, sizeof(stage_push), groups, 1, 1); } - return wrap_buffer(mrb, scomplex, x); + return result; } /* Shared by GPU::SComplex#magnitude and #power_spectrum. @@ -313,14 +341,14 @@ static mrb_value complex_reduce(mrb_state *mrb, mrb_value self, uint32_t square) if (count > (mrb_int)points) count = points; ensure_pipeline(mrb, PIPE_CMAG); - struct RClass *sfloat = gpu_class(mrb, "SFloat"); GpuBuffer *out = create_buffer(mrb, (uint32_t)count); + mrb_value result = wrap_buffer(mrb, gpu_class(mrb, "SFloat"), out); VkBuffer bufs[2] = {x->buffer, out->buffer}; VkDeviceSize sizes[2] = {x->bytes, out->bytes}; struct { uint32_t out_n, square; } push = {(uint32_t)count, square}; - dispatch_compute(PIPE_CMAG, bufs, sizes, 2, &push, sizeof(push), + dispatch_compute(mrb, PIPE_CMAG, bufs, sizes, 2, &push, sizeof(push), ((uint32_t)count + 255) / 256, 1, 1); - return wrap_buffer(mrb, sfloat, out); + return result; } /* #magnitude(count = size / 2) -> GPU::SFloat of sqrt(re^2 + im^2) */ @@ -340,7 +368,7 @@ static mrb_value complex_power_spectrum(mrb_state *mrb, mrb_value self) { static mrb_value gpu_s_init(mrb_state *mrb, mrb_value self) { const char *path; mrb_get_args(mrb, "z", &path); - gpu_init(path); + gpu_init(mrb, path); return mrb_nil_value(); } @@ -369,6 +397,10 @@ static mrb_value gpu_s_info(mrb_state *mrb, mrb_value self) { mrb_str_new_cstr(mrb, api_ver)); mrb_hash_set(mrb, h, mrb_symbol_value(mrb_intern_cstr(mrb, "backend")), mrb_str_new_cstr(mrb, "Vulkan")); + /* The dispatch bound, so callers (and bug reports) can see the largest + * array a single operation can cover: max_workgroups * 256 elements. */ + mrb_hash_set(mrb, h, mrb_symbol_value(mrb_intern_cstr(mrb, "max_workgroups")), + mrb_fixnum_value((mrb_int)g_ctx.max_workgroups)); return h; } diff --git a/src/gpu_vulkan.c b/src/gpu_vulkan.c index 8331680..5525461 100644 --- a/src/gpu_vulkan.c +++ b/src/gpu_vulkan.c @@ -33,6 +33,39 @@ const char *gpu_pipe_name(PipeId pipe_id) { return pipe_names[pipe_id]; } +/* ---- Error handling ---- + * + * The Vulkan SDK ships vk_enum_string_helper.h for this, but relying on it + * would add an SDK dependency to a gem whose only link dependency is the + * loader, so the handful of codes that can actually reach us live here. */ +const char *gpu_result_name(VkResult r) { + switch (r) { + case VK_SUCCESS: return "success"; + case VK_NOT_READY: return "not ready"; + case VK_TIMEOUT: return "timeout"; + case VK_INCOMPLETE: return "incomplete"; + case VK_ERROR_OUT_OF_HOST_MEMORY: return "out of host memory"; + case VK_ERROR_OUT_OF_DEVICE_MEMORY: return "out of device memory"; + case VK_ERROR_INITIALIZATION_FAILED: return "initialization failed"; + case VK_ERROR_DEVICE_LOST: return "device lost"; + case VK_ERROR_MEMORY_MAP_FAILED: return "memory map failed"; + case VK_ERROR_LAYER_NOT_PRESENT: return "layer not present"; + case VK_ERROR_EXTENSION_NOT_PRESENT: return "extension not present"; + case VK_ERROR_FEATURE_NOT_PRESENT: return "feature not present"; + case VK_ERROR_INCOMPATIBLE_DRIVER: return "incompatible driver"; + case VK_ERROR_TOO_MANY_OBJECTS: return "too many objects"; + case VK_ERROR_FRAGMENTED_POOL: return "descriptor pool fragmented"; + case VK_ERROR_UNKNOWN: return "unknown error"; + default: return "unrecognised VkResult"; + } +} + +void gpu_check(mrb_state *mrb, VkResult r, const char *call) { + if (r == VK_SUCCESS) return; + mrb_raisef(mrb, E_RUNTIME_ERROR, "%s failed: %s (VkResult %d)", + call, gpu_result_name(r), (int)r); +} + /* Portability (MoltenVK on macOS) support. Absent on the Raspberry Pi's native * V3D driver, so everything below is gated on runtime extension detection and * has no effect there. Provide fallback macros for older SDK headers. */ @@ -44,11 +77,15 @@ const char *gpu_pipe_name(PipeId pipe_id) { #endif #define GPU_PORTABILITY_SUBSET_EXT "VK_KHR_portability_subset" +/* An extension that cannot be enumerated is treated as absent: the caller only + * uses these to opt into optional behaviour, so failing closed is correct and + * lets the Pi's plain path keep working. */ static int has_instance_ext(const char *name) { uint32_t count = 0; - vkEnumerateInstanceExtensionProperties(NULL, &count, NULL); + if (vkEnumerateInstanceExtensionProperties(NULL, &count, NULL) != VK_SUCCESS) return 0; if (count == 0) return 0; VkExtensionProperties *props = malloc(sizeof(VkExtensionProperties) * count); + if (!props) return 0; vkEnumerateInstanceExtensionProperties(NULL, &count, props); int found = 0; for (uint32_t i = 0; i < count; i++) { @@ -60,9 +97,10 @@ static int has_instance_ext(const char *name) { static int has_device_ext(VkPhysicalDevice dev, const char *name) { uint32_t count = 0; - vkEnumerateDeviceExtensionProperties(dev, NULL, &count, NULL); + if (vkEnumerateDeviceExtensionProperties(dev, NULL, &count, NULL) != VK_SUCCESS) return 0; if (count == 0) return 0; VkExtensionProperties *props = malloc(sizeof(VkExtensionProperties) * count); + if (!props) return 0; vkEnumerateDeviceExtensionProperties(dev, NULL, &count, props); int found = 0; for (uint32_t i = 0; i < count; i++) { @@ -89,13 +127,28 @@ static uint8_t *load_spv(const char *path, size_t *size) { /* ---- Generic Compute Dispatch ---- */ void dispatch_compute( - PipeId pipe_id, + mrb_state *mrb, PipeId pipe_id, VkBuffer *buffers, VkDeviceSize *sizes, int num_buffers, const void *push_data, uint32_t push_size, uint32_t group_x, uint32_t group_y, uint32_t group_z) { LayoutId lid = pipe_to_layout[pipe_id]; + /* Exceeding maxComputeWorkGroupCount is invalid usage, and what happens is + * up to the driver: lavapipe runs the dispatch anyway and returns the right + * answer, so the limit is easy to miss in testing, while a driver bounded by + * a hardware register can clamp or fault instead. Reject it here rather than + * let the answer depend on which GPU the code lands on. */ + if (group_x > g_ctx.max_workgroups) { + /* Widened to mrb_int first: the element count overflows uint32_t on + * devices that report a workgroup limit in the billions. */ + mrb_raisef(mrb, E_ARGUMENT_ERROR, + "array too large for one dispatch: %i workgroups needed, device allows %i " + "(about %i elements)", + (mrb_int)group_x, (mrb_int)g_ctx.max_workgroups, + (mrb_int)g_ctx.max_workgroups * 256); + } + /* Allocate descriptor set */ VkDescriptorSetAllocateInfo dsai = { .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO, @@ -104,7 +157,7 @@ void dispatch_compute( .pSetLayouts = &g_ctx.desc_layouts[lid] }; VkDescriptorSet desc_set; - vkAllocateDescriptorSets(g_ctx.device, &dsai, &desc_set); + VK_CHECK(mrb, vkAllocateDescriptorSets(g_ctx.device, &dsai, &desc_set)); /* Update descriptor set */ VkDescriptorBufferInfo buf_infos[3]; @@ -130,44 +183,74 @@ void dispatch_compute( .commandBufferCount = 1 }; VkCommandBuffer cmd; - vkAllocateCommandBuffers(g_ctx.device, &cbai, &cmd); + VkResult r = vkAllocateCommandBuffers(g_ctx.device, &cbai, &cmd); + if (r != VK_SUCCESS) { + vkFreeDescriptorSets(g_ctx.device, g_ctx.desc_pool, 1, &desc_set); + gpu_check(mrb, r, "vkAllocateCommandBuffers"); + } + /* Record, submit and wait. Everything from here shares one exit path so the + * fence, command buffer and descriptor set are released whatever fails -- + * gpu_check below raises, and a raise unwinds past any cleanup after it. */ VkCommandBufferBeginInfo begin = { .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, .flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT }; - vkBeginCommandBuffer(cmd, &begin); - vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, g_ctx.pipelines[pipe_id]); - vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, - g_ctx.pipe_layouts[lid], 0, 1, &desc_set, 0, NULL); - if (push_size > 0) { - vkCmdPushConstants(cmd, g_ctx.pipe_layouts[lid], VK_SHADER_STAGE_COMPUTE_BIT, - 0, push_size, push_data); - } - vkCmdDispatch(cmd, group_x, group_y, group_z); - vkEndCommandBuffer(cmd); - - /* Submit and wait */ VkFenceCreateInfo fi = {.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; - VkFence fence; - vkCreateFence(g_ctx.device, &fi, NULL, &fence); - VkSubmitInfo si = { .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, .commandBufferCount = 1, .pCommandBuffers = &cmd }; - vkQueueSubmit(g_ctx.queue, 1, &si, fence); - vkWaitForFences(g_ctx.device, 1, &fence, VK_TRUE, UINT64_MAX); + VkFence fence = VK_NULL_HANDLE; + const char *step = "vkBeginCommandBuffer"; + + r = vkBeginCommandBuffer(cmd, &begin); + if (r == VK_SUCCESS) { + vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, g_ctx.pipelines[pipe_id]); + vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, + g_ctx.pipe_layouts[lid], 0, 1, &desc_set, 0, NULL); + if (push_size > 0) { + vkCmdPushConstants(cmd, g_ctx.pipe_layouts[lid], VK_SHADER_STAGE_COMPUTE_BIT, + 0, push_size, push_data); + } + vkCmdDispatch(cmd, group_x, group_y, group_z); + step = "vkEndCommandBuffer"; + r = vkEndCommandBuffer(cmd); + } + if (r == VK_SUCCESS) { + step = "vkCreateFence"; + r = vkCreateFence(g_ctx.device, &fi, NULL, &fence); + } + if (r == VK_SUCCESS) { + step = "vkQueueSubmit"; + r = vkQueueSubmit(g_ctx.queue, 1, &si, fence); + } + if (r == VK_SUCCESS) { + /* Without this check a lost device returns immediately and the caller + * reads whatever is in the buffer as a valid result. */ + step = "vkWaitForFences"; + r = vkWaitForFences(g_ctx.device, 1, &fence, VK_TRUE, UINT64_MAX); + } - vkDestroyFence(g_ctx.device, fence, NULL); + if (fence != VK_NULL_HANDLE) vkDestroyFence(g_ctx.device, fence, NULL); vkFreeCommandBuffers(g_ctx.device, g_ctx.cmd_pool, 1, &cmd); vkFreeDescriptorSets(g_ctx.device, g_ctx.desc_pool, 1, &desc_set); + + gpu_check(mrb, r, step); } /* ---- Init ---- */ -void gpu_init(const char *shader_dir) { +void gpu_init(mrb_state *mrb, const char *shader_dir) { if (g_ctx.initialized) return; + /* A half-built context cannot be reused, and retrying would leak everything + * the failed attempt created -- once per operation, since initialization is + * lazy. One attempt per process; fix the environment and restart. */ + if (g_ctx.init_failed) { + mrb_raise(mrb, E_RUNTIME_ERROR, + "GPU initialization already failed in this process and is not retried"); + } + g_ctx.init_failed = 1; /* cleared only on the success path at the end */ /* Instance */ VkApplicationInfo app_info = { @@ -191,20 +274,25 @@ void gpu_init(const char *shader_dir) { .enabledExtensionCount = inst_ext_count, .ppEnabledExtensionNames = inst_ext_count ? inst_exts : NULL }; - vkCreateInstance(&inst_info, NULL, &g_ctx.instance); + VK_CHECK(mrb, vkCreateInstance(&inst_info, NULL, &g_ctx.instance)); /* Physical Device: prefer a real GPU over a software rasterizer. * The Pi exposes both V3D (hardware) and llvmpipe (CPU); pick the first * non-CPU device so compute actually runs on the GPU. Falls back to the * first device if every device is CPU-type. */ uint32_t dev_count = 0; - vkEnumeratePhysicalDevices(g_ctx.instance, &dev_count, NULL); + VK_CHECK(mrb, vkEnumeratePhysicalDevices(g_ctx.instance, &dev_count, NULL)); if (dev_count == 0) { - fprintf(stderr, "mruby-gpu-narray: no Vulkan physical device found\n"); - return; + mrb_raise(mrb, E_RUNTIME_ERROR, + "no Vulkan physical device found (is a Vulkan driver installed?)"); } VkPhysicalDevice *devs = malloc(sizeof(VkPhysicalDevice) * dev_count); - vkEnumeratePhysicalDevices(g_ctx.instance, &dev_count, devs); + if (!devs) mrb_raise(mrb, E_RUNTIME_ERROR, "out of memory enumerating devices"); + VkResult dev_r = vkEnumeratePhysicalDevices(g_ctx.instance, &dev_count, devs); + if (dev_r != VK_SUCCESS) { + free(devs); + gpu_check(mrb, dev_r, "vkEnumeratePhysicalDevices"); + } g_ctx.physical_device = devs[0]; for (uint32_t i = 0; i < dev_count; i++) { VkPhysicalDeviceProperties p; @@ -216,19 +304,32 @@ void gpu_init(const char *shader_dir) { } free(devs); + /* Remember the dispatch limit, so an oversized array is reported rather + * than silently truncated (see dispatch_compute). */ + VkPhysicalDeviceProperties dev_props; + vkGetPhysicalDeviceProperties(g_ctx.physical_device, &dev_props); + g_ctx.max_workgroups = dev_props.limits.maxComputeWorkGroupCount[0]; + /* Queue Family (compute) */ uint32_t qf_count = 0; vkGetPhysicalDeviceQueueFamilyProperties(g_ctx.physical_device, &qf_count, NULL); VkQueueFamilyProperties *qf_props = malloc(sizeof(VkQueueFamilyProperties) * qf_count); + if (!qf_props) mrb_raise(mrb, E_RUNTIME_ERROR, "out of memory enumerating queue families"); vkGetPhysicalDeviceQueueFamilyProperties(g_ctx.physical_device, &qf_count, qf_props); + int have_compute = 0; g_ctx.queue_family = 0; for (uint32_t i = 0; i < qf_count; i++) { if (qf_props[i].queueFlags & VK_QUEUE_COMPUTE_BIT) { g_ctx.queue_family = i; + have_compute = 1; break; } } free(qf_props); + if (!have_compute) { + mrb_raisef(mrb, E_RUNTIME_ERROR, + "device '%s' has no compute-capable queue family", dev_props.deviceName); + } /* Device + Queue */ float priority = 1.0f; @@ -252,7 +353,7 @@ void gpu_init(const char *shader_dir) { .enabledExtensionCount = dev_ext_count, .ppEnabledExtensionNames = dev_ext_count ? dev_exts : NULL }; - vkCreateDevice(g_ctx.physical_device, &dev_info, NULL, &g_ctx.device); + VK_CHECK(mrb, vkCreateDevice(g_ctx.physical_device, &dev_info, NULL, &g_ctx.device)); vkGetDeviceQueue(g_ctx.device, g_ctx.queue_family, 0, &g_ctx.queue); /* Command Pool */ @@ -261,7 +362,7 @@ void gpu_init(const char *shader_dir) { .flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT, .queueFamilyIndex = g_ctx.queue_family }; - vkCreateCommandPool(g_ctx.device, &pool_info, NULL, &g_ctx.cmd_pool); + VK_CHECK(mrb, vkCreateCommandPool(g_ctx.device, &pool_info, NULL, &g_ctx.cmd_pool)); /* Descriptor Set Layouts: 3, 2 and 1 storage buffers respectively */ int buf_counts[LAYOUT_COUNT] = {3, 2, 1}; @@ -280,7 +381,8 @@ void gpu_init(const char *shader_dir) { .bindingCount = buf_counts[l], .pBindings = bindings }; - vkCreateDescriptorSetLayout(g_ctx.device, &dl_info, NULL, &g_ctx.desc_layouts[l]); + VK_CHECK(mrb, vkCreateDescriptorSetLayout(g_ctx.device, &dl_info, NULL, + &g_ctx.desc_layouts[l])); } /* Pipeline Layouts (one per descriptor layout, shared push constant range). @@ -299,7 +401,8 @@ void gpu_init(const char *shader_dir) { .pushConstantRangeCount = 1, .pPushConstantRanges = &push_range }; - vkCreatePipelineLayout(g_ctx.device, &pl_info, NULL, &g_ctx.pipe_layouts[l]); + VK_CHECK(mrb, vkCreatePipelineLayout(g_ctx.device, &pl_info, NULL, + &g_ctx.pipe_layouts[l])); } /* Load shaders and create pipelines */ @@ -311,18 +414,29 @@ void gpu_init(const char *shader_dir) { uint8_t *spv_code = load_spv(spv_path, &spv_size); if (!spv_code) { fprintf(stderr, "mruby-gpu-narray: could not load %s (run `make -C shader`)\n", spv_path); - g_ctx.pipelines[p] = VK_NULL_HANDLE; + g_ctx.pipelines[p] = VK_NULL_HANDLE; + g_ctx.pipe_state[p] = PIPE_MISSING_SPV; continue; } + /* A missing shader is not fatal -- only the operations that need it fail, + * and ensure_pipeline reports why at that point. The distinction between + * "no file" and "the driver rejected it" is recorded here because both + * leave the handle VK_NULL_HANDLE but call for opposite fixes. */ VkShaderModuleCreateInfo sm_info = { .sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO, .codeSize = spv_size, .pCode = (uint32_t *)spv_code }; VkShaderModule shader; - vkCreateShaderModule(g_ctx.device, &sm_info, NULL, &shader); + VkResult r = vkCreateShaderModule(g_ctx.device, &sm_info, NULL, &shader); free(spv_code); + if (r != VK_SUCCESS) { + g_ctx.pipelines[p] = VK_NULL_HANDLE; + g_ctx.pipe_state[p] = PIPE_CREATE_FAILED; + g_ctx.pipe_error[p] = r; + continue; + } LayoutId lid = pipe_to_layout[p]; VkComputePipelineCreateInfo cp_info = { @@ -335,8 +449,16 @@ void gpu_init(const char *shader_dir) { }, .layout = g_ctx.pipe_layouts[lid] }; - vkCreateComputePipelines(g_ctx.device, VK_NULL_HANDLE, 1, &cp_info, NULL, &g_ctx.pipelines[p]); + r = vkCreateComputePipelines(g_ctx.device, VK_NULL_HANDLE, 1, &cp_info, NULL, + &g_ctx.pipelines[p]); vkDestroyShaderModule(g_ctx.device, shader, NULL); + if (r == VK_SUCCESS) { + g_ctx.pipe_state[p] = PIPE_READY; + } else { + g_ctx.pipelines[p] = VK_NULL_HANDLE; + g_ctx.pipe_state[p] = PIPE_CREATE_FAILED; + g_ctx.pipe_error[p] = r; + } } /* Descriptor Pool */ @@ -351,7 +473,8 @@ void gpu_init(const char *shader_dir) { .poolSizeCount = 1, .pPoolSizes = &pool_size }; - vkCreateDescriptorPool(g_ctx.device, &dp_info, NULL, &g_ctx.desc_pool); + VK_CHECK(mrb, vkCreateDescriptorPool(g_ctx.device, &dp_info, NULL, &g_ctx.desc_pool)); g_ctx.initialized = 1; + g_ctx.init_failed = 0; } diff --git a/test/narray_test.rb b/test/narray_test.rb index a571602..0ec432f 100644 --- a/test/narray_test.rb +++ b/test/narray_test.rb @@ -172,6 +172,25 @@ def tone(len, freq, sine = false) GPU::SFloat[1, 2, 3] * "nope" end +# Element counts are uint32_t on the GPU side; a larger request must be +# refused rather than wrapped around into a different length. +assert_raise("size past uint32 -> ArgumentError", ArgumentError) do + GPU::SFloat.new(2**32 + 10) +end + +# One dispatch covers 256 elements per workgroup, bounded by the device's +# maxComputeWorkGroupCount. Past that the result is up to the driver, so it is +# rejected instead. Sized off the device, and skipped where the limit is high +# enough that probing it would mean a multi-gigabyte allocation. +max_elems = GPU.info[:max_workgroups] * 256 +if max_elems <= 64_000_000 + assert_raise("array past one dispatch -> ArgumentError", ArgumentError) do + GPU::SFloat.new(max_elems + 256).sum + end +else + puts "SKIP array past one dispatch (device allows #{max_elems} elements per dispatch)" +end + # ---- summary ---- puts puts "#{$pass + $fail} tests, #{$pass} passed, #{$fail} failed" diff --git a/test/shader_error_test.rb b/test/shader_error_test.rb new file mode 100644 index 0000000..0f4c9f2 --- /dev/null +++ b/test/shader_error_test.rb @@ -0,0 +1,63 @@ +# mruby-gpu-narray -- shader loading failures. +# +# Kept out of narray_test.rb because the GPU context is a process-wide +# singleton: pointing GPU.init at a bad shader directory here would leave every +# later test without pipelines. Run this file on its own: +# +# mruby test/shader_error_test.rb +# +# With no argument it checks the missing-file case only. Pass a directory of +# unreadable-but-present .spv files to also check the driver-rejects case; the +# harness that generates those lives in the CI workflow. + +$pass = 0 +$fail = 0 + +def ok(label) + $pass += 1 + puts "PASS #{label}" +end + +def ng(label, detail) + $fail += 1 + puts "FAIL #{label}: #{detail}" +end + +# Asserts that the block raises, and that the message mentions `expect` but not +# `avoid` -- the point of these cases is *which* fix the message sends you to. +def assert_message(label, expect, avoid) + begin + yield + rescue => e + if !e.message.include?(expect) + return ng(label, "expected the message to mention #{expect.inspect}, got #{e.message.inspect}") + end + if avoid && e.message.include?(avoid) + return ng(label, "message should not mention #{avoid.inspect}: #{e.message.inspect}") + end + return ok(label) + end + ng(label, "nothing was raised") +end + +bad_dir = ARGV[0] + +if bad_dir + # The .spv files exist and load, and the driver refuses them. Reporting this + # as "not compiled" would send the user to re-run a build step that already + # ran, so the message has to say the opposite. + GPU.init(bad_dir) + assert_message("driver-rejected shader names the driver", "driver rejected", "make -C shader") do + GPU::SFloat[1, 2] + GPU::SFloat[3, 4] + end +else + # No .spv files at all: here "run make" *is* the right advice. + GPU.init("/nonexistent/mruby-gpu-narray-shaders") + assert_message("missing shader tells you to build it", "make -C shader", nil) do + GPU::SFloat[1, 2] + GPU::SFloat[3, 4] + end +end + +puts +puts "#{$pass + $fail} tests, #{$pass} passed, #{$fail} failed" +puts($fail > 0 ? "SOME TESTS FAILED" : "ALL TESTS PASSED") From ac6799c6ab57edfbe0d667af0b86ff0314fb3392 Mon Sep 17 00:00:00 2001 From: yujiteshima Date: Sat, 12 Sep 2026 22:18:32 +0900 Subject: [PATCH 2/2] Record dispatches into one batch and submit when a result is read Every operator used to be its own round trip: build a command buffer and a descriptor set, submit, wait on a fence, free. Almost all of the time went into that, not into arithmetic -- a 4-operator chain cost 4 waits. Now dispatch_pipeline() records into one long-lived command buffer with a memory barrier after each dispatch, and gpu_flush() submits and waits once, at the first sync point: a host read or write of a buffer the batch touches (map_buffer), a full descriptor pool, GPU.sync, or shutdown. Buffers carry the epoch they were last bound in, which is how map_buffer decides whether to flush and how the GC finalizer knows to park a still-referenced buffer in a graveyard until the batch has run. GPU.pending shows what is queued; GPU.sync_mode = :eager restores a wait per dispatch for measurement. dispatch_pipeline() is exported so add-on gems (mruby-gpu-kernel) join the same batch. Apple M5, 1024 elements: a * 2 + 1 - 3 + 4 goes from 0.95 ms (4 waits) to 0.25 ms (1 wait); power_spectrum from 2.69 ms (12 waits) to 0.29 ms. Co-Authored-By: Claude Fable 5.1 --- README.md | 52 +++++++++- src/gpu_buffer.c | 38 ++++++- src/gpu_internal.h | 63 +++++++++-- src/gpu_narray.c | 94 +++++++++++++---- src/gpu_vulkan.c | 248 +++++++++++++++++++++++++++++++------------- test/narray_test.rb | 58 +++++++++++ 6 files changed, 442 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index bf35c00..7d5d2c5 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,8 @@ GPU::SFloat (mruby) mrblib/gpu_narray.rb (shape, mean, ▼ src/gpu_narray.c dtype methods; picks a pipeline; keeps results on the GPU ▼ -src/gpu_vulkan.c dispatch_compute(): descriptor set → command buffer → submit → fence +src/gpu_vulkan.c dispatch_pipeline(): descriptor set → recorded into one command buffer, + barrier + gpu_flush(): submit → fence, once per batch, when a result is read ▼ shader/*.comp add / sub / mul / div / scale / adds / sum fft_bitrev / fft_stage / cmag (GLSL → SPIR-V) @@ -150,10 +151,55 @@ buffer of `2n` floats (interleaved re/im, wrapped as `GPU::SComplex`): order, so the butterflies afterwards run in natural order and in place. 2. `fft_stage` runs once per pass with a doubling half-size `h = 1, 2, … n/2`. Each invocation owns one butterfly, and a pass's pairs partition `[0, n)` exactly — so - the in-place writes don't race, and one dispatch per pass is the barrier between - passes. + the in-place writes don't race, and the memory barrier recorded after each pass + orders it before the next. 3. `cmag` reduces the spectrum to a real `GPU::SFloat` of magnitudes or powers. +### Deferred submission + +A dispatch is not a round trip. Every operation is *recorded* into one long-lived +command buffer, followed by a memory barrier so the next dispatch sees its writes, +and nothing is submitted until the host actually needs a result. Then everything +recorded so far goes to the GPU in one `vkQueueSubmit`, with one fence wait. + +```ruby +b = a * 2 + 1 - 3 + 4 # four dispatches recorded, nothing submitted +GPU.pending #=> 4 +b.to_a # one submit, one wait, then the copy +GPU.pending #=> 0 +``` + +The sync points, where the batch is flushed: + +- reading a result (`to_a`, `head`, `sum`, `mean`, `inspect`, …); +- a host write (`seq`, `fill`) to a buffer that a queued dispatch reads or writes — + a buffer the batch never touched is written at once; +- the descriptor pool running dry (256 dispatches), which just flushes early; +- `GPU.sync`, explicitly — useful when timing a batch; +- shutdown. + +A buffer whose Ruby object is collected while a queued dispatch still references it +is kept until that batch has run, then freed. `rfft` benefits without changes: its +`1 + log2(n)` passes are one submit. + +`GPU.sync_mode = :eager` restores a submit and a wait after every dispatch — the +behaviour this library had before batching, kept so the two can be measured +against each other (`GPU.sync_mode` reads it back; the default is `:deferred`). + +| Apple M5, 100-run mean | `a * 2 + 1 - 3 + 4` (4 dispatches) | `power_spectrum` | +|---|---|---| +| 1,024 elements, eager (4 / 12 waits) | 0.95 ms | 2.69 ms | +| 1,024 elements, deferred (1 wait) | 0.25 ms | 0.29 ms | +| 1,048,576 elements, eager | 2.33 ms | 7.13 ms | +| 1,048,576 elements, deferred | 2.12 ms | 2.74 ms | + +At 1M elements the four passes over memory dominate, and batching alone cannot +remove those — that is what fusing the expression into one shader is for +([mruby-gpu-kernel](https://github.com/yujiteshima/mruby-gpu-kernel): 0.35 ms). + +Add-on gems can put their own pipelines into the same batch through +`dispatch_pipeline()` in `src/gpu_internal.h`. + ### Limits and failures One operation is one dispatch, covering 256 elements per workgroup up to the diff --git a/src/gpu_buffer.c b/src/gpu_buffer.c index ba80291..ab27079 100644 --- a/src/gpu_buffer.c +++ b/src/gpu_buffer.c @@ -9,6 +9,14 @@ * Callers wrap as soon as create_buffer returns, before anything that can * raise, so a Vulkan failure unwinds without stranding GPU memory. * destroy_buffer is the finalizer itself, plus create_buffer's own unwind. + * + * With deferred submission a buffer can be referenced by a command buffer that + * has not run yet. Two rules keep that safe: + * - map_buffer flushes the batch first if it touches this buffer, so the + * host never reads a result that is still queued, and never overwrites an + * input that a queued dispatch has yet to read. + * - destroy_buffer parks such a buffer in the graveyard instead of freeing + * it; gpu_flush frees the graveyard once the batch has completed. */ #include "gpu_internal.h" @@ -30,6 +38,7 @@ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) { buf->buffer = VK_NULL_HANDLE; buf->memory = VK_NULL_HANDLE; buf->bytes = sizeof(float) * (VkDeviceSize)n; + buf->epoch = 0; /* never bound; no batch can be waiting on it */ if (buf->bytes == 0) buf->bytes = sizeof(float); /* avoid zero-size allocation */ VkBufferCreateInfo bi = { @@ -93,10 +102,30 @@ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n) { return buf; } -/* ---- Free a buffer's GPU resources + the struct itself ---- */ +/* ---- Free a buffer's GPU resources + the struct itself ---- + * + * Runs as the GC finalizer, so it must not raise and must not block: a buffer + * the pending batch still references is parked in the graveyard and freed by + * the next gpu_flush, after the GPU is done with it. */ void destroy_buffer(mrb_state *mrb, GpuBuffer *buf) { if (!buf) return; if (g_ctx.initialized) { + if (g_ctx.batch_recording && buf->epoch == g_ctx.epoch) { + if (g_ctx.graveyard_len == g_ctx.graveyard_cap) { + size_t cap = g_ctx.graveyard_cap ? g_ctx.graveyard_cap * 2 : 64; + GpuBuffer **g = realloc(g_ctx.graveyard, cap * sizeof(GpuBuffer *)); + if (!g) { + /* Cannot park it and must not free it under the GPU's feet. Leaking + * one buffer is the least bad outcome inside a finalizer. */ + fprintf(stderr, "mruby-gpu-narray: out of memory deferring a buffer free; leaking it\n"); + return; + } + g_ctx.graveyard = g; + g_ctx.graveyard_cap = cap; + } + g_ctx.graveyard[g_ctx.graveyard_len++] = buf; + return; + } vkDestroyBuffer(g_ctx.device, buf->buffer, NULL); vkFreeMemory(g_ctx.device, buf->memory, NULL); } @@ -112,8 +141,13 @@ mrb_value wrap_buffer(mrb_state *mrb, struct RClass *klass, GpuBuffer *buf) { /* ---- Host mapping helpers (coherent memory, no flush needed) ---- * * Never returns NULL: every caller writes through the pointer immediately, so - * an unreported map failure would be a NULL dereference inside the VM. */ + * an unreported map failure would be a NULL dereference inside the VM. + * + * This is the sync point of the whole library: the host is about to look at + * (or change) the bytes, so any queued dispatch that reads or writes this + * buffer has to run first. A buffer the batch never touched maps at once. */ float *map_buffer(mrb_state *mrb, GpuBuffer *buf) { + gpu_sync_buffer(mrb, buf); float *mapped = NULL; VK_CHECK(mrb, vkMapMemory(g_ctx.device, buf->memory, 0, buf->bytes, 0, (void **)&mapped)); if (!mapped) { diff --git a/src/gpu_internal.h b/src/gpu_internal.h index 78b70cb..d6923ba 100644 --- a/src/gpu_internal.h +++ b/src/gpu_internal.h @@ -48,6 +48,21 @@ typedef enum { PIPE_CREATE_FAILED /* the driver rejected it; see pipe_error[] */ } PipeState; +/* ---- GPU Buffer (FP32, 1-D) ---- */ +typedef struct { + VkBuffer buffer; + VkDeviceMemory memory; + uint32_t n; /* element count */ + VkDeviceSize bytes; /* n * sizeof(float) */ + uint32_t epoch; /* batch this buffer was last bound in (0 = never); see dispatch_pipeline */ +} GpuBuffer; + +extern const struct mrb_data_type gpu_buffer_type; + +/* Descriptor sets the pool holds. A batch that needs more is flushed early, + * which is just an earlier sync point, not an error. */ +#define GPU_MAX_DESC_SETS 256 + /* ---- GPU Context (singleton) ---- */ typedef struct { VkInstance instance; @@ -65,20 +80,25 @@ typedef struct { uint32_t max_workgroups; /* maxComputeWorkGroupCount[0] */ int initialized; int init_failed; /* a previous gpu_init raised; do not retry */ + + /* ---- Deferred submission (see dispatch_pipeline / gpu_flush) ---- + * + * Dispatches are recorded into one long-lived command buffer and submitted + * together the first time the host needs a result. */ + VkCommandBuffer batch_cmd; /* allocated at init, reset by every begin */ + VkFence batch_fence; /* reused across flushes */ + int batch_recording; /* batch_cmd is between vkBegin and vkEnd */ + uint32_t batch_dispatches; /* recorded since the last flush (GPU.pending) */ + uint32_t batch_sets; /* descriptor sets taken since the last pool reset */ + uint32_t epoch; /* batch number; bumped by every flush, never 0 */ + int eager; /* GPU.sync_mode = :eager -> flush after every dispatch */ + GpuBuffer **graveyard; /* buffers whose Ruby owner died while a batch still referenced them */ + size_t graveyard_len; + size_t graveyard_cap; } GpuCtx; extern GpuCtx g_ctx; -/* ---- GPU Buffer (FP32, 1-D) ---- */ -typedef struct { - VkBuffer buffer; - VkDeviceMemory memory; - uint32_t n; /* element count */ - VkDeviceSize bytes; /* n * sizeof(float) */ -} GpuBuffer; - -extern const struct mrb_data_type gpu_buffer_type; - /* ---- Error handling ---- * * Every Vulkan call that returns a VkResult goes through VK_CHECK, which turns @@ -95,11 +115,32 @@ const char *gpu_result_name(VkResult r); /* ---- gpu_vulkan.c ---- */ void gpu_init(mrb_state *mrb, const char *shader_dir); const char *gpu_pipe_name(PipeId pipe_id); + +/* Record one compute dispatch of `pipeline` (created against + * g_ctx.pipe_layouts[lid]) over `bufs`, in binding order. Nothing is submitted + * here unless GPU.sync_mode is :eager; see gpu_flush. Exported so that add-on + * gems (mruby-gpu-kernel) can run pipelines of their own through the same + * batch. */ +void dispatch_pipeline(mrb_state *mrb, VkPipeline pipeline, LayoutId lid, + GpuBuffer **bufs, int num_buffers, + const void *push_data, uint32_t push_size, + uint32_t group_x, uint32_t group_y, uint32_t group_z); + +/* Same, for one of the built-in pipelines. */ void dispatch_compute(mrb_state *mrb, PipeId pipe_id, - VkBuffer *buffers, VkDeviceSize *sizes, int num_buffers, + GpuBuffer **bufs, int num_buffers, const void *push_data, uint32_t push_size, uint32_t group_x, uint32_t group_y, uint32_t group_z); +/* Submit everything recorded so far, wait for it, and reset for the next batch. + * Also frees buffers parked in the graveyard. Safe to call with nothing + * pending. */ +void gpu_flush(mrb_state *mrb); + +/* Flush only if the pending batch references `buf`. Every host-side read or + * write of a buffer goes through this (map_buffer calls it). */ +void gpu_sync_buffer(mrb_state *mrb, GpuBuffer *buf); + /* ---- gpu_buffer.c ---- */ GpuBuffer *create_buffer(mrb_state *mrb, uint32_t n); void destroy_buffer(mrb_state *mrb, GpuBuffer *buf); /* GC finalizer + create_buffer unwind */ diff --git a/src/gpu_narray.c b/src/gpu_narray.c index 095bc5b..45ad7d6 100644 --- a/src/gpu_narray.c +++ b/src/gpu_narray.c @@ -177,10 +177,9 @@ static mrb_value binop_nn(mrb_state *mrb, mrb_value self, GpuBuffer *rhs, PipeId /* Wrapped before dispatching: dispatch_compute raises on a Vulkan failure, * and the GC can only free what it owns. */ mrb_value result = wrap_buffer(mrb, mrb_obj_class(mrb, self), c); - VkBuffer bufs[3] = {a->buffer, rhs->buffer, c->buffer}; - VkDeviceSize sizes[3] = {a->bytes, rhs->bytes, c->bytes}; + GpuBuffer *bufs[3] = {a, rhs, c}; uint32_t push = a->n; - dispatch_compute(mrb, pipe, bufs, sizes, 3, &push, sizeof(uint32_t), + dispatch_compute(mrb, pipe, bufs, 3, &push, sizeof(uint32_t), (a->n + 255) / 256, 1, 1); return result; } @@ -191,10 +190,9 @@ static mrb_value scalar_op(mrb_state *mrb, mrb_value self, float scalar, PipeId ensure_pipeline(mrb, pipe); GpuBuffer *b = create_buffer(mrb, a->n); mrb_value result = wrap_buffer(mrb, mrb_obj_class(mrb, self), b); - VkBuffer bufs[2] = {a->buffer, b->buffer}; - VkDeviceSize sizes[2] = {a->bytes, b->bytes}; + GpuBuffer *bufs[2] = {a, b}; struct { uint32_t n; float s; } push = {a->n, scalar}; - dispatch_compute(mrb, pipe, bufs, sizes, 2, &push, sizeof(push), + dispatch_compute(mrb, pipe, bufs, 2, &push, sizeof(push), (a->n + 255) / 256, 1, 1); return result; } @@ -262,12 +260,12 @@ static mrb_value narray_sum(mrb_state *mrb, mrb_value self) { uint32_t groups = (a->n + 255) / 256; GpuBuffer *partial = create_buffer(mrb, groups); wrap_buffer(mrb, mrb_obj_class(mrb, self), partial); - VkBuffer bufs[2] = {a->buffer, partial->buffer}; - VkDeviceSize sizes[2] = {a->bytes, partial->bytes}; + GpuBuffer *bufs[2] = {a, partial}; uint32_t push = a->n; - dispatch_compute(mrb, PIPE_SUM, bufs, sizes, 2, &push, sizeof(uint32_t), + dispatch_compute(mrb, PIPE_SUM, bufs, 2, &push, sizeof(uint32_t), groups, 1, 1); + /* map_buffer flushes the batch: the partials are its last dispatch. */ float *pm = map_buffer(mrb, partial); double total = 0.0; for (uint32_t i = 0; i < groups; i++) total += (double)pm[i]; @@ -290,8 +288,9 @@ static struct RClass *gpu_class(mrb_state *mrb, const char *name) { /* #rfft -> GPU::SComplex, the n-point DFT of this real array. * * Dispatches once to permute real -> complex into bit-reversed order, then - * once per butterfly pass (log2(n) of them). Each dispatch's submit/fence is - * the barrier between passes. Nothing is copied to the host. */ + * once per butterfly pass (log2(n) of them). The passes are recorded into the + * same batch with a memory barrier between them, so the whole transform is + * one submit. Nothing is copied to the host. */ static mrb_value narray_rfft(mrb_state *mrb, mrb_value self) { GpuBuffer *a = DATA_GET_PTR(mrb, self, &gpu_buffer_type, GpuBuffer); uint32_t n = a->n; @@ -310,18 +309,16 @@ static mrb_value narray_rfft(mrb_state *mrb, mrb_value self) { GpuBuffer *x = create_buffer(mrb, 2 * n); mrb_value result = wrap_buffer(mrb, gpu_class(mrb, "SComplex"), x); - VkBuffer bitrev_bufs[2] = {a->buffer, x->buffer}; - VkDeviceSize bitrev_sizes[2] = {a->bytes, x->bytes}; + GpuBuffer *bitrev_bufs[2] = {a, x}; struct { uint32_t n, log2n; } bitrev_push = {n, log2n}; - dispatch_compute(mrb, PIPE_FFT_BITREV, bitrev_bufs, bitrev_sizes, 2, + dispatch_compute(mrb, PIPE_FFT_BITREV, bitrev_bufs, 2, &bitrev_push, sizeof(bitrev_push), (n + 255) / 256, 1, 1); - VkBuffer stage_bufs[1] = {x->buffer}; - VkDeviceSize stage_sizes[1] = {x->bytes}; + GpuBuffer *stage_bufs[1] = {x}; uint32_t groups = (n / 2 + 255) / 256; for (uint32_t h = 1; h < n; h <<= 1) { struct { uint32_t n, h; } stage_push = {n, h}; - dispatch_compute(mrb, PIPE_FFT_STAGE, stage_bufs, stage_sizes, 1, + dispatch_compute(mrb, PIPE_FFT_STAGE, stage_bufs, 1, &stage_push, sizeof(stage_push), groups, 1, 1); } @@ -343,10 +340,9 @@ static mrb_value complex_reduce(mrb_state *mrb, mrb_value self, uint32_t square) GpuBuffer *out = create_buffer(mrb, (uint32_t)count); mrb_value result = wrap_buffer(mrb, gpu_class(mrb, "SFloat"), out); - VkBuffer bufs[2] = {x->buffer, out->buffer}; - VkDeviceSize sizes[2] = {x->bytes, out->bytes}; + GpuBuffer *bufs[2] = {x, out}; struct { uint32_t out_n, square; } push = {(uint32_t)count, square}; - dispatch_compute(mrb, PIPE_CMAG, bufs, sizes, 2, &push, sizeof(push), + dispatch_compute(mrb, PIPE_CMAG, bufs, 2, &push, sizeof(push), ((uint32_t)count + 255) / 256, 1, 1); return result; } @@ -404,6 +400,45 @@ static mrb_value gpu_s_info(mrb_state *mrb, mrb_value self) { return h; } +/* GPU.sync -> nil. Run everything recorded so far and wait for it. Reading a + * result does this implicitly; call it yourself to time a batch, or before + * handing a buffer to something outside this library. */ +static mrb_value gpu_s_sync(mrb_state *mrb, mrb_value self) { + gpu_flush(mrb); + return mrb_nil_value(); +} + +/* GPU.pending -> Integer: dispatches recorded but not yet submitted. */ +static mrb_value gpu_s_pending(mrb_state *mrb, mrb_value self) { + return mrb_fixnum_value((mrb_int)g_ctx.batch_dispatches); +} + +/* GPU.sync_mode -> :deferred | :eager + * + * :deferred (default) records dispatches and submits them together when a + * result is needed. :eager submits and waits after every dispatch -- the + * behaviour before batching existed, kept for measuring the difference. */ +static mrb_value gpu_s_sync_mode(mrb_state *mrb, mrb_value self) { + /* Two literals, not one ternary: mrb_intern_lit takes sizeof its argument. */ + return mrb_symbol_value(g_ctx.eager ? mrb_intern_lit(mrb, "eager") + : mrb_intern_lit(mrb, "deferred")); +} + +static mrb_value gpu_s_set_sync_mode(mrb_state *mrb, mrb_value self) { + mrb_sym mode; + mrb_get_args(mrb, "n", &mode); + if (mode == mrb_intern_lit(mrb, "eager")) { + g_ctx.eager = 1; + gpu_flush(mrb); /* nothing may stay queued once the mode says nothing is */ + } else if (mode == mrb_intern_lit(mrb, "deferred")) { + g_ctx.eager = 0; + } else { + mrb_raisef(mrb, E_ARGUMENT_ERROR, + "GPU.sync_mode must be :deferred or :eager, got :%n", mode); + } + return mrb_symbol_value(mode); +} + /* ========================================================================= * gem init / final * ========================================================================= */ @@ -413,6 +448,10 @@ void mrb_mruby_gpu_narray_gem_init(mrb_state *mrb) { mrb_define_module_function(mrb, gpu, "init", gpu_s_init, MRB_ARGS_REQ(1)); mrb_define_module_function(mrb, gpu, "device_name", gpu_s_device_name, MRB_ARGS_NONE()); mrb_define_module_function(mrb, gpu, "info", gpu_s_info, MRB_ARGS_NONE()); + mrb_define_module_function(mrb, gpu, "sync", gpu_s_sync, MRB_ARGS_NONE()); + mrb_define_module_function(mrb, gpu, "pending", gpu_s_pending, MRB_ARGS_NONE()); + mrb_define_module_function(mrb, gpu, "sync_mode", gpu_s_sync_mode, MRB_ARGS_NONE()); + mrb_define_module_function(mrb, gpu, "sync_mode=", gpu_s_set_sync_mode, MRB_ARGS_REQ(1)); struct RClass *narray = mrb_define_class_under(mrb, gpu, "NArray", mrb->object_class); MRB_SET_INSTANCE_TT(narray, MRB_TT_CDATA); @@ -447,8 +486,21 @@ void mrb_mruby_gpu_narray_gem_init(mrb_state *mrb) { } void mrb_mruby_gpu_narray_gem_final(mrb_state *mrb) { - (void)mrb; if (g_ctx.initialized) { + /* Whatever is still recorded was never submitted and nobody can observe + * it now, so it is dropped rather than run. Nothing is in flight: every + * submit is waited on before gpu_flush returns. */ + g_ctx.batch_recording = 0; + for (size_t i = 0; i < g_ctx.graveyard_len; i++) { + vkDestroyBuffer(g_ctx.device, g_ctx.graveyard[i]->buffer, NULL); + vkFreeMemory(g_ctx.device, g_ctx.graveyard[i]->memory, NULL); + mrb_free(mrb, g_ctx.graveyard[i]); + } + g_ctx.graveyard_len = 0; + free(g_ctx.graveyard); + g_ctx.graveyard = NULL; + g_ctx.graveyard_cap = 0; + vkDestroyFence(g_ctx.device, g_ctx.batch_fence, NULL); vkDestroyDescriptorPool(g_ctx.device, g_ctx.desc_pool, NULL); for (int p = 0; p < PIPE_COUNT; p++) { if (g_ctx.pipelines[p] != VK_NULL_HANDLE) { diff --git a/src/gpu_vulkan.c b/src/gpu_vulkan.c index 5525461..4ce0619 100644 --- a/src/gpu_vulkan.c +++ b/src/gpu_vulkan.c @@ -125,15 +125,69 @@ static uint8_t *load_spv(const char *path, size_t *size) { return buf; } -/* ---- Generic Compute Dispatch ---- */ -void dispatch_compute( - mrb_state *mrb, PipeId pipe_id, - VkBuffer *buffers, VkDeviceSize *sizes, int num_buffers, +/* ---- Deferred submission ---- + * + * A dispatch is no longer a round trip. It is recorded into one long-lived + * command buffer, followed by a memory barrier so the next dispatch sees its + * writes, and nothing is submitted until the host actually needs a result: + * a host read or write of a buffer the batch touches (map_buffer), a + * descriptor pool that has run dry, an explicit GPU.sync, or shutdown. Then + * gpu_flush ends the command buffer, submits it once, waits once, and resets + * the pool for the next batch. + * + * `a * 2 + 1 - 3 + 4` therefore costs four dispatches and one wait instead of + * four waits; an FFT's log2(n) passes cost one wait instead of log2(n). + * GPU.sync_mode = :eager flushes after every dispatch -- the pre-batching + * behaviour, kept so the two can be measured against each other. + * + * A buffer remembers the epoch (batch number) it was last bound in. That is + * how map_buffer knows whether a flush is due, and how the GC finalizer knows + * a buffer must outlive its Ruby owner until the batch has run. */ + +static void batch_begin(mrb_state *mrb) { + if (g_ctx.batch_recording) return; + VkCommandBufferBeginInfo begin = { + .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, + .flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT + }; + /* The pool was created with RESET_COMMAND_BUFFER_BIT, so beginning a + * command buffer that has already been submitted resets it implicitly. */ + VK_CHECK(mrb, vkBeginCommandBuffer(g_ctx.batch_cmd, &begin)); + g_ctx.batch_recording = 1; +} + +/* One descriptor set from the pool. A full pool just means the batch is + * flushed a little early: the sets it held are released by the reset. */ +static VkDescriptorSet alloc_desc_set(mrb_state *mrb, LayoutId lid) { + if (g_ctx.batch_sets >= GPU_MAX_DESC_SETS) gpu_flush(mrb); + VkDescriptorSetAllocateInfo dsai = { + .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO, + .descriptorPool = g_ctx.desc_pool, + .descriptorSetCount = 1, + .pSetLayouts = &g_ctx.desc_layouts[lid] + }; + VkDescriptorSet set = VK_NULL_HANDLE; + VkResult r = vkAllocateDescriptorSets(g_ctx.device, &dsai, &set); + if (r == VK_ERROR_FRAGMENTED_POOL +#ifdef VK_ERROR_OUT_OF_POOL_MEMORY + || r == VK_ERROR_OUT_OF_POOL_MEMORY +#endif + ) { + /* Our count and the driver's disagree; a flush resets the pool either way. */ + gpu_flush(mrb); + r = vkAllocateDescriptorSets(g_ctx.device, &dsai, &set); + } + gpu_check(mrb, r, "vkAllocateDescriptorSets"); + g_ctx.batch_sets++; + return set; +} + +void dispatch_pipeline( + mrb_state *mrb, VkPipeline pipeline, LayoutId lid, + GpuBuffer **bufs, int num_buffers, const void *push_data, uint32_t push_size, uint32_t group_x, uint32_t group_y, uint32_t group_z) { - LayoutId lid = pipe_to_layout[pipe_id]; - /* Exceeding maxComputeWorkGroupCount is invalid usage, and what happens is * up to the driver: lavapipe runs the dispatch anyway and returns the right * answer, so the limit is easy to miss in testing, while a driver bounded by @@ -148,22 +202,19 @@ void dispatch_compute( (mrb_int)group_x, (mrb_int)g_ctx.max_workgroups, (mrb_int)g_ctx.max_workgroups * 256); } + if (pipeline == VK_NULL_HANDLE) { + mrb_raise(mrb, E_RUNTIME_ERROR, "dispatch of a pipeline that was never created"); + } + if (num_buffers < 1 || num_buffers > 3) { + mrb_raisef(mrb, E_ARGUMENT_ERROR, "dispatch takes 1..3 buffers, got %d", num_buffers); + } - /* Allocate descriptor set */ - VkDescriptorSetAllocateInfo dsai = { - .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO, - .descriptorPool = g_ctx.desc_pool, - .descriptorSetCount = 1, - .pSetLayouts = &g_ctx.desc_layouts[lid] - }; - VkDescriptorSet desc_set; - VK_CHECK(mrb, vkAllocateDescriptorSets(g_ctx.device, &dsai, &desc_set)); + VkDescriptorSet desc_set = alloc_desc_set(mrb, lid); - /* Update descriptor set */ VkDescriptorBufferInfo buf_infos[3]; VkWriteDescriptorSet writes[3]; for (int i = 0; i < num_buffers; i++) { - buf_infos[i] = (VkDescriptorBufferInfo){buffers[i], 0, sizes[i]}; + buf_infos[i] = (VkDescriptorBufferInfo){bufs[i]->buffer, 0, bufs[i]->bytes}; writes[i] = (VkWriteDescriptorSet){ .sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET, .dstSet = desc_set, @@ -175,69 +226,102 @@ void dispatch_compute( } vkUpdateDescriptorSets(g_ctx.device, num_buffers, writes, 0, NULL); - /* Command buffer */ - VkCommandBufferAllocateInfo cbai = { - .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO, - .commandPool = g_ctx.cmd_pool, - .level = VK_COMMAND_BUFFER_LEVEL_PRIMARY, - .commandBufferCount = 1 + batch_begin(mrb); + VkCommandBuffer cmd = g_ctx.batch_cmd; + vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline); + vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, + g_ctx.pipe_layouts[lid], 0, 1, &desc_set, 0, NULL); + if (push_size > 0) { + vkCmdPushConstants(cmd, g_ctx.pipe_layouts[lid], VK_SHADER_STAGE_COMPUTE_BIT, + 0, push_size, push_data); + } + vkCmdDispatch(cmd, group_x, group_y, group_z); + + /* Make this dispatch's writes visible to whatever is recorded next. One + * global barrier per dispatch is coarser than tracking buffers one by one, + * but it costs nothing next to the submit + fence wait it replaces. */ + VkMemoryBarrier barrier = { + .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, + .srcAccessMask = VK_ACCESS_SHADER_WRITE_BIT, + .dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT }; - VkCommandBuffer cmd; - VkResult r = vkAllocateCommandBuffers(g_ctx.device, &cbai, &cmd); - if (r != VK_SUCCESS) { - vkFreeDescriptorSets(g_ctx.device, g_ctx.desc_pool, 1, &desc_set); - gpu_check(mrb, r, "vkAllocateCommandBuffers"); + vkCmdPipelineBarrier(cmd, + VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, + 0, 1, &barrier, 0, NULL, 0, NULL); + + for (int i = 0; i < num_buffers; i++) bufs[i]->epoch = g_ctx.epoch; + g_ctx.batch_dispatches++; + + if (g_ctx.eager) gpu_flush(mrb); +} + +void dispatch_compute( + mrb_state *mrb, PipeId pipe_id, + GpuBuffer **bufs, int num_buffers, + const void *push_data, uint32_t push_size, + uint32_t group_x, uint32_t group_y, uint32_t group_z) +{ + dispatch_pipeline(mrb, g_ctx.pipelines[pipe_id], pipe_to_layout[pipe_id], + bufs, num_buffers, push_data, push_size, + group_x, group_y, group_z); +} + +/* Free what the GC could not: buffers whose owners died while a batch still + * referenced them. Only called once no work is pending. */ +static void bury_graveyard(mrb_state *mrb) { + for (size_t i = 0; i < g_ctx.graveyard_len; i++) { + GpuBuffer *b = g_ctx.graveyard[i]; + vkDestroyBuffer(g_ctx.device, b->buffer, NULL); + vkFreeMemory(g_ctx.device, b->memory, NULL); + mrb_free(mrb, b); } + g_ctx.graveyard_len = 0; +} - /* Record, submit and wait. Everything from here shares one exit path so the - * fence, command buffer and descriptor set are released whatever fails -- - * gpu_check below raises, and a raise unwinds past any cleanup after it. */ - VkCommandBufferBeginInfo begin = { - .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, - .flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT - }; - VkFenceCreateInfo fi = {.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; - VkSubmitInfo si = { - .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, - .commandBufferCount = 1, - .pCommandBuffers = &cmd - }; - VkFence fence = VK_NULL_HANDLE; - const char *step = "vkBeginCommandBuffer"; - - r = vkBeginCommandBuffer(cmd, &begin); - if (r == VK_SUCCESS) { - vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, g_ctx.pipelines[pipe_id]); - vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, - g_ctx.pipe_layouts[lid], 0, 1, &desc_set, 0, NULL); - if (push_size > 0) { - vkCmdPushConstants(cmd, g_ctx.pipe_layouts[lid], VK_SHADER_STAGE_COMPUTE_BIT, - 0, push_size, push_data); - } - vkCmdDispatch(cmd, group_x, group_y, group_z); +void gpu_flush(mrb_state *mrb) { + if (!g_ctx.initialized) return; + VkResult r = VK_SUCCESS; + const char *step = NULL; + + if (g_ctx.batch_recording) { step = "vkEndCommandBuffer"; - r = vkEndCommandBuffer(cmd); - } - if (r == VK_SUCCESS) { - step = "vkCreateFence"; - r = vkCreateFence(g_ctx.device, &fi, NULL, &fence); - } - if (r == VK_SUCCESS) { - step = "vkQueueSubmit"; - r = vkQueueSubmit(g_ctx.queue, 1, &si, fence); + r = vkEndCommandBuffer(g_ctx.batch_cmd); + if (r == VK_SUCCESS) { + VkSubmitInfo si = { + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, + .commandBufferCount = 1, + .pCommandBuffers = &g_ctx.batch_cmd + }; + step = "vkQueueSubmit"; + r = vkQueueSubmit(g_ctx.queue, 1, &si, g_ctx.batch_fence); + } + if (r == VK_SUCCESS) { + /* Without this check a lost device returns immediately and the caller + * reads whatever is in the buffer as a valid result. */ + step = "vkWaitForFences"; + r = vkWaitForFences(g_ctx.device, 1, &g_ctx.batch_fence, VK_TRUE, UINT64_MAX); + if (r == VK_SUCCESS) vkResetFences(g_ctx.device, 1, &g_ctx.batch_fence); + } + /* Whatever happened, this command buffer is spent; the next batch_begin + * resets it. */ + g_ctx.batch_recording = 0; } - if (r == VK_SUCCESS) { - /* Without this check a lost device returns immediately and the caller - * reads whatever is in the buffer as a valid result. */ - step = "vkWaitForFences"; - r = vkWaitForFences(g_ctx.device, 1, &fence, VK_TRUE, UINT64_MAX); + + /* No work is pending from here on (either it completed or it never left + * the host), so the sets and the graveyard can go. */ + g_ctx.batch_dispatches = 0; + if (g_ctx.batch_sets > 0) { + vkResetDescriptorPool(g_ctx.device, g_ctx.desc_pool, 0); + g_ctx.batch_sets = 0; } + g_ctx.epoch++; + bury_graveyard(mrb); - if (fence != VK_NULL_HANDLE) vkDestroyFence(g_ctx.device, fence, NULL); - vkFreeCommandBuffers(g_ctx.device, g_ctx.cmd_pool, 1, &cmd); - vkFreeDescriptorSets(g_ctx.device, g_ctx.desc_pool, 1, &desc_set); + if (step) gpu_check(mrb, r, step); +} - gpu_check(mrb, r, step); +void gpu_sync_buffer(mrb_state *mrb, GpuBuffer *buf) { + if (g_ctx.batch_recording && buf->epoch == g_ctx.epoch) gpu_flush(mrb); } /* ---- Init ---- */ @@ -364,6 +448,22 @@ void gpu_init(mrb_state *mrb, const char *shader_dir) { }; VK_CHECK(mrb, vkCreateCommandPool(g_ctx.device, &pool_info, NULL, &g_ctx.cmd_pool)); + /* The one command buffer every dispatch is recorded into, and the fence + * each flush waits on. Both live as long as the context. */ + VkCommandBufferAllocateInfo cbai = { + .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO, + .commandPool = g_ctx.cmd_pool, + .level = VK_COMMAND_BUFFER_LEVEL_PRIMARY, + .commandBufferCount = 1 + }; + VK_CHECK(mrb, vkAllocateCommandBuffers(g_ctx.device, &cbai, &g_ctx.batch_cmd)); + VkFenceCreateInfo fi = {.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; + VK_CHECK(mrb, vkCreateFence(g_ctx.device, &fi, NULL, &g_ctx.batch_fence)); + g_ctx.batch_recording = 0; + g_ctx.batch_dispatches = 0; + g_ctx.batch_sets = 0; + g_ctx.epoch = 1; /* a fresh buffer carries epoch 0, which never matches */ + /* Descriptor Set Layouts: 3, 2 and 1 storage buffers respectively */ int buf_counts[LAYOUT_COUNT] = {3, 2, 1}; for (int l = 0; l < LAYOUT_COUNT; l++) { @@ -464,12 +564,12 @@ void gpu_init(mrb_state *mrb, const char *shader_dir) { /* Descriptor Pool */ VkDescriptorPoolSize pool_size = { .type = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, - .descriptorCount = 768 + .descriptorCount = 3 * GPU_MAX_DESC_SETS }; VkDescriptorPoolCreateInfo dp_info = { .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO, .flags = VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT, - .maxSets = 256, + .maxSets = GPU_MAX_DESC_SETS, .poolSizeCount = 1, .pPoolSizes = &pool_size }; diff --git a/test/narray_test.rb b/test/narray_test.rb index 0ec432f..bdbe6b2 100644 --- a/test/narray_test.rb +++ b/test/narray_test.rb @@ -191,6 +191,64 @@ def tone(len, freq, sine = false) puts "SKIP array past one dispatch (device allows #{max_elems} elements per dispatch)" end +def assert_true(label, cond, detail = "false") + cond ? ok(label) : ng(label, detail) +end + +# ---- deferred submission ---- +# +# Dispatches are recorded and submitted together when a result is read. +# These check the batching itself, the sync points, and the two things that +# could silently go wrong: a host write racing a queued read, and a buffer +# freed by the GC while a queued dispatch still references it. + +GPU.sync_mode = :deferred +GPU.sync # earlier tests may have left work queued; start the count from zero +assert_true("default sync mode is :deferred", GPU.sync_mode == :deferred, GPU.sync_mode.inspect) + +dv = GPU::SFloat.new(1024).seq +chain = dv * 2 + 1 - 3 + 4 +assert_true("four operators are recorded, not submitted", GPU.pending == 4, "pending = #{GPU.pending}") +assert_ary("deferred chain reads back correctly", + [2.0, 4.0, 6.0, 8.0, 10.0, 12.0, 14.0, 16.0], chain.head(8)) +assert_true("reading a result flushes the batch", GPU.pending == 0, "pending = #{GPU.pending}") + +dr = dv * 2 +dv.fill(0.0) # host write to the input of a queued dispatch +assert_ary("a host write flushes the queued reader first", [0.0, 2.0, 4.0, 6.0], dr.head(4)) +dv.seq + +fresh = GPU::SFloat.new(4) +dv * 3 # queued +fresh.fill(1.0) # a buffer the batch never touched must not force a flush +assert_true("writing an untouched buffer keeps the batch pending", GPU.pending == 1, "pending = #{GPU.pending}") +GPU.sync + +long = dv +300.times { long = long * 1.0 } # more dispatches than the descriptor pool holds +assert_ary("a batch longer than the descriptor pool still runs", [0.0, 1.0, 2.0, 3.0], long.head(4)) + +gy = nil +40.times { gy = (dv + 1) * 2 - 2 } # (dv + 1) and its double are garbage on the next iteration +GC.start +assert_ary("garbage intermediates outlive the batch that reads them", [0.0, 2.0, 4.0, 6.0], gy.head(4)) + +ds = GPU::SFloat.cast([1.0, 2.0, 3.0, 4.0]) +assert_near("sum flushes and reads the partials", 20.0, (ds * 2).sum) + +dsig = GPU::SFloat.new(16).seq +spec_deferred = dsig.power_spectrum.to_a +GPU.sync_mode = :eager +spec_eager = dsig.power_spectrum.to_a +assert_ary("rfft agrees between deferred and eager", spec_eager, spec_deferred) +dsig * 2 +assert_true("eager mode submits every dispatch", GPU.pending == 0, "pending = #{GPU.pending}") +assert_true("sync_mode reports :eager", GPU.sync_mode == :eager, GPU.sync_mode.inspect) +GPU.sync_mode = :deferred +GPU.sync +assert_true("GPU.sync with nothing pending is a no-op", GPU.pending == 0) +assert_raise("unknown sync mode -> ArgumentError", ArgumentError) { GPU.sync_mode = :later } + # ---- summary ---- puts puts "#{$pass + $fail} tests, #{$pass} passed, #{$fail} failed"