Validate CPU kernel registrations at compile time#29649
Open
GopalakrishnanN wants to merge 4 commits into
Open
Validate CPU kernel registrations at compile time#29649GopalakrishnanN wants to merge 4 commits into
GopalakrishnanN wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes malformed CPU kernel factory tables a compile-time error by adding a consteval validation helper and applying it to the CPU EP’s ONNX, optional FP16, and ONNX-ML registration tables. It also adds compile-time unit tests for the validation logic plus a small runtime sanity test that the real CPU kernel registry is non-empty.
Changes:
- Add
cpu::registration_internal::IsKernelRegistrationTableValid(consteval) to enforce that the disabledBuildKernelCreateInfo<void>entry is first and that no table entries are null. - Mark the CPU EP’s ONNX / FP16 / ONNX-ML
function_tablearrays asconstexprand addstatic_assertvalidation for each. - Add unit tests covering valid/invalid tables and a runtime check that the default CPU kernel registry initializes non-empty.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| onnxruntime/test/framework/cpu_kernel_registration_test.cc | Adds compile-time table-validation tests plus a runtime non-empty registry check. |
| onnxruntime/core/providers/cpu/cpu_kernel_registration.h | Introduces a consteval helper to validate kernel registration tables at compile time. |
| onnxruntime/core/providers/cpu/cpu_execution_provider.cc | Makes the registration tables constexpr and enforces compile-time invariants via static_assert. |
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.
Description
constexpr.constevalinvariant that requires the disabledBuildKernelCreateInfo<void>entry first and rejects null factory pointers.Motivation and Context
CPU kernel registration is a fixed build-time descriptor set, but malformed tables were only discoverable while constructing the runtime registry. These checks make sentinel placement and factory-pointer validity compile-time requirements across the normal, reduced-ops, optional FP16, and optional ONNX-ML table variants.
The existing compact runtime registration loops are intentionally unchanged. A templated shared loop increased the provider library by 5,006 bytes, and an inline span version increased it by 33,970 bytes; keeping the loops produced the smallest measured result.
Measurement
Windows RelWithDebInfo, MSVC 19.51, touched-file rebuild of
cpu_execution_provider.cc:cpu_execution_provider.objonnxruntime_providers.libThe single baseline rebuild was 8.55 s; final post-change samples were 6.78 s, 6.54 s, and 6.44 s. Timing is noisy, so this PR does not claim a compile-time improvement.
Testing
cmake --build build/Windows/RelWithDebInfo --config RelWithDebInfo --target onnxruntime_test_all --parallel 8onnxruntime_test_all.exe --gtest_filter=CpuKernelRegistrationTest.*:KernelRegistryTests.*from the test output directory: 9 tests from 2 suites passed.lintrunner -aon all three changed files: no lint issues.