Add support for specialization constants by ndgrigorian · Pull Request #2304 · IntelPython/dpctl

ndgrigorian · 2026-04-28T22:44:35Z

This PR introduces support for specialization constants in dpctl, including both a Cython class SpecializationConstant for construction and passing of the constructed class to create_kernel_bundle_from_spirv via a new specializations keyword argument.

The SpecializationConstant class supports multiple constructors, including from Python buffers, a dtype string and a Python buffer (casting to the dtype via NumPy), and a number of bytes and a pointer as integers.

Also introduces dpctl.program.utils with parse_spirv_specializations utility function, allowing the user to query a SPIR-V directly from Python.

Have you provided a meaningful PR description?
Have you added a test, reproducer or referred to an issue with a reproducer?
Have you tested your changes locally for CPU and GPU devices?
Have you made sure that new changes do not introduce compiler warnings?
Have you checked performance impact of proposed changes?
Have you added documentation for your changes, if necessary?
Have you added your changes to the changelog?
If this PR is a work in progress, are you opening the PR as a draft?

github-actions · 2026-04-28T22:48:35Z

View rendered docs @ https://intelpython.github.io/dpctl/pulls/2304/index.html

coveralls · 2026-04-28T23:31:41Z

coverage: 75.205% (-0.5%) from 75.677% — feature/specialization-constants into master

also removes "v" as a permitted specialization constant intermediate data type, as composite specialization constants are broken into multiple specialization constants, so structs end up passed as a single constant while the program expects multiple, and therefore, doesn't work as intended

also adds spec_id, itemsize, and default_value fields

copy inputs to SpecializationConstant to prevent dangling pointers

also adds a test

antonwolfy · 2026-06-23T07:45:28Z

+                "Failed to allocate memory for specialization constants."
+            )
+        for i, spconst in enumerate(specializations):
+            if not isinstance(spconst, SpecializationConstant):


The loop variable is declared cdef SpecializationConstant spconst, so for i, spconst in enumerate(specializations) performs the type coercion at the loop assignment.
A non-SpecializationConstant element raises Cython's own TypeError before reaching the isinstance guard at line 555 — so the custom error message and the free(spconsts) cleanup never run, leaking the spconsts allocation.

antonwolfy · 2026-06-23T07:55:16Z

+
+# add submodules
+__all__ += [
+    "utils",


"utils" in __all__ is never imported

Tests only pass because they do from dpctl.program.utils import ... explicitly. Needs to add from . import utils

antonwolfy · 2026-06-23T08:02:50Z

+
+        if word_count == 0:
+            raise ValueError(f"Invalid SPIR-V instruction at word index {i}")
+        if i + word_count > len(words):


SPIR-V parser raises uncaught IndexError / UnicodeDecodeError on malformed input.

The only bounds check is that the declared word_count fits the buffer; per-opcode operand reads (OpTypeInt needs i+3, OpDecorate needs i+3, OpName decodes UTF-8) are unchecked. A truncated instruction throws raw IndexError, and invalid UTF-8 in OpName throws UnicodeDecodeError — both contradicting the documented ValueError("Invalid SPIR-V binary") contract for a function whose explicit job is parsing untrusted binaries. Neither path is tested.

Should that be covered in the tests?

antonwolfy · 2026-06-23T08:06:41Z

+    }
    ze_module_constants_t ZeSpecConstants = {};
-    ZeSpecConstants.numConstants = 0;
+    ZeSpecConstants.numConstants = static_cast<std::uint32_t>(NumSpecConsts);


numConstants set even when arrays are empty.

ZeSpecConstants.numConstants is set to NumSpecConsts unconditionally, while pConstantIds/pConstantValues become nullptr when the vectors are empty. If a caller passes NumSpecConsts > 0 with SpecConsts == nullptr, L0 gets a nonzero count with null arrays → UB. The OCL path silently ignores the same input, so the two backends diverge. (While the Cython layer never produces this, so it's a hardening/API-consistency issue.)

antonwolfy · 2026-06-23T08:08:02Z

+                f"{len(args) + 1}."
+            )
+
+        self._spec_const.id = <uint32_t>spec_id


It wraps silently for out-of-range/negative IDs. SPIR-V SpecId is a 32-bit decoration; an out-of-range value should raise, not wrap.

antonwolfy · 2026-06-23T08:12:15Z

+    def __repr__(self):
+        return f"SpecializationConstant({self._spec_const.id})"
+
+    def __eq__(self, other):


Do we need to add definition for __hash__, since defining __eq__ might drop hashability?

antonwolfy · 2026-06-23T08:14:11Z

+
+    def __eq__(self, other):
+        if not isinstance(other, SpecializationConstant):
+            return False


To unblock reflected comparison:

Suggested change

return False

return NotImplemented

antonwolfy · 2026-06-23T08:17:42Z

+                isinstance(args[0], numbers.Integral) and
+                isinstance(args[1], numbers.Integral)
+            ):
+                target_obj = PyBytes_FromStringAndSize(


No nbytes >= 0 check.

A negative Py_ssize_t into PyBytes_FromStringAndSize is undefined; the unsafe-pointer path is inherently risky but should at least range-check.

antonwolfy · 2026-06-23T08:19:47Z

    case backend::opencl:
        KBRef = _CreateKernelBundleWithIL_ocl_impl(*SyclCtx, *SyclDev, IL,
-                                                   length, CompileOpts);
+                                                   length, CompileOpts,


No try/catch across the C ABI boundary in the OpenCL/top-level path.
The spec-constant build failures enlarge the throwing input space.

antonwolfy · 2026-06-23T08:28:49Z

-                                  const char *CompileOpts);
+                                  const char *CompileOpts,
+                                  size_t NumSpecConsts,
+                                  const DPCTLSpecConst *SpecConsts);


Do we need to add __dpctl_keep?

ndgrigorian changed the title ~~Feature/specialization constants~~ Add support for specialization constants Apr 28, 2026

ndgrigorian force-pushed the feature/specialization-constants branch 3 times, most recently from b7f8d82 to 8c5651d Compare May 5, 2026 06:23

ndgrigorian marked this pull request as ready for review May 5, 2026 06:28

ndgrigorian requested review from antonwolfy and vlad-perevezentsev as code owners May 5, 2026 06:28

ndgrigorian force-pushed the feature/specialization-constants branch 3 times, most recently from 8dbb320 to 043aa83 Compare May 5, 2026 06:52

ndgrigorian force-pushed the rename-sycl-program-sycl-kernel-bundle branch from 82736b8 to 1a0a910 Compare May 7, 2026 01:12

ndgrigorian force-pushed the feature/specialization-constants branch from 55d4458 to 97871d9 Compare May 7, 2026 01:12

Base automatically changed from rename-sycl-program-sycl-kernel-bundle to master May 21, 2026 17:11

antonwolfy reviewed May 22, 2026

View reviewed changes

ndgrigorian added 7 commits May 23, 2026 18:07

Implement SpecializationConstant class

7946152

hook specialization constants into kernel bundle interface

b5befc3

add test for SpecializationConstant use in kernel

8ddc440

fix libsyclinterface tests

9e264f2

add program.utils namespace with SPIRV parser

b100cf5

Refactor specialiazation constants to use dataclass

e2e4826

also adds spec_id, itemsize, and default_value fields

ndgrigorian force-pushed the feature/specialization-constants branch from 97871d9 to e2e4826 Compare May 24, 2026 01:07

ndgrigorian added 2 commits June 7, 2026 23:49

add note leading to SPIRV spec

86d21b0

release clPrograms when builds fail

7e6065f

ndgrigorian force-pushed the feature/specialization-constants branch from 79cd9fd to 53be0e1 Compare June 9, 2026 05:31

ndgrigorian added 2 commits June 8, 2026 22:36

apply review feedback

2de1c72

copy inputs to SpecializationConstant to prevent dangling pointers

add dpctl.program.utils to docs

d9f386b

ndgrigorian force-pushed the feature/specialization-constants branch from 53be0e1 to d9f386b Compare June 9, 2026 05:36

vlad-perevezentsev reviewed Jun 10, 2026

View reviewed changes

ndgrigorian added 6 commits June 22, 2026 12:11

Merge branch 'master' into feature/specialization-constants

f7daf45

fix copyright dates in new dpctl.program files

4519d08

add addressof_ref to SpecializationConstant

0d9814a

also adds a test

address review feedback

5b28b86

add test for specialization constant equality

6d17a7b

make error clearer in SpecializationConstant

baa91b5

ndgrigorian requested review from antonwolfy and vlad-perevezentsev June 22, 2026 19:39

antonwolfy reviewed Jun 23, 2026

View reviewed changes

Conversation

ndgrigorian commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

coveralls commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ndgrigorian commented Apr 28, 2026 •

edited

Loading

coveralls commented Apr 28, 2026 •

edited

Loading