Skip to content

[WIP] ggml-opencl: op args init refactoring - #18701

Draft
chraac wants to merge 20 commits into
ggml-org:masterfrom
chraac:dev-ocl-test-fix
Draft

chraac wants to merge 20 commits into
ggml-org:masterfrom
chraac:dev-ocl-test-fix

Conversation

@chraac

@chraac chraac commented Jan 8, 2026 •

Copy link
Copy Markdown
Contributor

Refactors the OpenCL kernel argument initialization process by introducing a template helper, significantly reducing boilerplate code and improving readability.

Key Changes

  • Introduced cl_set_kernel_args: A variadic template function that automatically iterates through arguments and sets them using clSetKernelArg.
  • Type-Specific Setters: Added cl_kernel_arg_setter struct specializations to handle different types:
    • ggml_tensor: Automatically extracts the OpenCL buffer (data_device) and calculates the offset (offset + view_offs), setting both as kernel arguments.
    • Arrays (ne, nb): Automatically unrolls int64_t[4] and size_t[4] arrays into individual scalar arguments (e.g., ne0, ne1, ne2, ne3).
    • Primitives: Direct handling for int, float, cl_ulong.

@chraac
chraac marked this pull request as draft January 8, 2026 16:49
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning OpenCL Issues specific to the OpenCL backend labels Jan 8, 2026
CL_CHECK(clSetKernelArg(kernel, 11, sizeof(cl_ulong), &nb21));
CL_CHECK(clSetKernelArg(kernel, 12, sizeof(int), &ne0));
CL_CHECK(clSetKernelArg(kernel, 13, sizeof(int), &ne1));
cl_set_kernel_args(kernel, src0, src1, src2, dst, nb01, nb02, nb11, nb21, ne0, ne1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth considering a shared header for kernel signatures to prevent mismatches between the host and device.
Knowing the refactoring effort would be high, but it would guarantee consistency.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we use a simple script to read a kernel file and make it into a C++ string in a header file to embed the kernels into executable. Using headers in kernel files will greatly increase the complexity of embedding kernels.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we use a simple script to read a kernel file and make it into a C++ string in a header file to embed the kernels into executable.

Yeah, I saw the embed_kernel.py script that converts the CL source into string literals.
If we introduce a new header here, I think we could extend that script to handle #include directives by simply replacing them with the content of the header file from the same directory.

@github-actions github-actions Bot added the python python script changes label Jan 14, 2026
src1->nb,
ne0,
dst->nb
);

@chraac chraac Jan 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heres a safer version of this function that'll check if the parameters match the cl kernel func signiture

@chraac

chraac commented Jan 17, 2026 •

Copy link
Copy Markdown
Contributor Author

@lhez
Also have a thought regarding load_cl_kernels: Consider refactoring this to use a data structure (like an attribute table) to store the program sources and kernel name.
This approach would significantly reduce code duplication and improve maintainability as new kernels are added.

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

ggml changes relating to the ggml tensor library for machine learning OpenCL Issues specific to the OpenCL backend python python script changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants