Repository navigation
[opencl] Pass crops to kernels as a byte offset instead of a sub-buffer - #9686
Open
mcourteaux wants to merge 1 commit into
Open
mcourteaux wants to merge 1 commit into
mcourteaux wants to merge 1 commit into
Conversation
A crop of an OpenCL buffer is its parent's cl_mem plus a byte offset. halide_opencl_run turned every such kernel argument into a clCreateSubBuffer at that offset, whose origin must be a multiple of CL_DEVICE_MEM_BASE_ADDR_ALIGN (up to 4096 bits): kernels could not run on crops starting at arbitrary elements, e.g. tiles of a larger image. Pass every buffer argument (but images) as its cl_mem and a ulong byte offset instead, and offset the pointer once at kernel entry. All indexing is unchanged, and the per-launch sub-buffer creation and release is gone. With crops at any element, the first element of an input or output buffer is only as aligned as its host_alignment promises, while the alignment of a load or store index is relative to that first element. The OpenCL code generator used a pointer to a vector type, which must be aligned to the vector's size, whenever the index was a multiple of the lanes, which faults on misaligned crops (it already did on drivers that accept unaligned sub-buffers). InjectGpuOffload now tells the device code generators how aligned each buffer argument's first element is (DeviceArgument::base_alignment: the pipeline's own allocations start at the start of a device allocation; others have their host_alignment, or the element size), and the OpenCL code generator falls back to vloadN/vstoreN unless the access is aligned in absolute terms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011FRemi1N2Cn82dNxq2aZkF
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenCL kernels could not reliably run on crops that start at an arbitrary element. A crop only records the parent
cl_memplus a byte offset (opencl_device_crop_from_offset), and at every launchhalide_opencl_runturned each buffer argument with a non-zero offset into aclCreateSubBufferat exactly that offset. That fails in two ways:CL_DEVICE_MEM_BASE_ADDR_ALIGN, so the launch fails withCL_MISALIGNED_SUB_BUFFER_OFFSET.((uchar4 *)p)[i]) use an index relative to the crop's first element, which is only element-aligned. That faults the GPU (CL_INVALID_COMMAND_QUEUE).So code that tiles a large image has to compute each tile into a separate buffer and copy it back, instead of writing into a crop in place.
Change
Byte offset as a kernel argument. Each buffer argument (except images) is now passed as the parent
cl_memplus aconst ulongbyte offset, applied once at kernel entry:All indexing stays as it was. A byte offset is exact whatever type the kernel reads the buffer as, and
size_tisn't allowed as a kernel argument type, henceulong. The pointer derived from therestrictparameter keeps its no-alias information.Runtime.
halide_opencl_runkeeps a separate kernel-argument index (buffers take two slots, images one, shared memory last). The per-launchclCreateSubBufferand its bookkeeping are gone;clReleaseKernelis added on the error paths. Copies, crop/slice/release_crop,wrap_cl_mem,get_crop_offsetand images already worked through the offset and are unchanged.Alignment. The OpenCL codegen used a vector-pointer access whenever the index was a multiple of the lane count, assuming an aligned base. A new
DeviceArgument::base_alignment, filled in byInjectGpuOffload, records what is known about the base:host_alignmentfor inputs and outputs, otherwise the element size.The codegen adds this to the index alignment and falls back to
vloadN/vstoreNwhen the absolute address can't be proven aligned. This matches how the CPU and PTX codegens treathost_alignment. Other backends are unchanged.Tests
New
test/correctness/opencl_misaligned_crop.cpp: uint8/uint16/int32/float; scalar, 4- and 8-wide vectors; crops of slices at odd x offsets; crops aliased to min 0 with known strides (indices provably aligned, base not); 3D crops of crops, with two crops of the samecl_memread in one kernel; and a check that nothing outside the output crop is written. Before: GPU fault (CL_INVALID_COMMAND_QUEUE). After: passes.233 correctness tests (gpu, device, crop, slice, buffer, copy, interleave, async, tiled, vector, align, atomics, float16, bounds, …) run directly with
HL_JIT_TARGET=host-openclon an RTX 4070 (ctestforcesHL_JIT_TARGET=hostand skips the GPU tests): before 222 pass, 10 skip, 1 fail (the new test); after 223 pass, 10 skip, no regressions.Real-world effect
A tiled denoiser over a 24 MP image (~1000 tiles), which had to copy every tile back from a separate buffer on OpenCL, can now crop the tiles in place: the data sent to the device drops from 304 MB to 49 MB per image and the total from 855 to 818 ms, bit-identical. With
vloadN/vstoreNon inputs withouthost_alignment, the kernel time stayed within noise.Things to be aware of
host_alignmentnow usevloadN/vstoreN. That was free on NVIDIA, but can be slower on some drivers;set_host_alignmentbrings back the aligned path. A device-only buffer whose crop breaks its declaredhost_alignmentis not caught at runtime.-no_runtimeruntime).ulong), which embedded-profile OpenCL may lack.Related: #9679 (strided copies with
clEnqueue*BufferRect), which makes the copy-back cheap for code that still needs it.🤖 Generated with Claude Code
https://claude.ai/code/session_011FRemi1N2Cn82dNxq2aZkF