-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[EP ABI] Add OrtKernelInfo APIs for kernel-based plugin EPs #26803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ft/onnxruntime into adrianl/plugin-ep-kernel-prepack
…) receives an unexpected call (return error instead of setting a flag to false)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
onnxruntime/test/autoep/library/example_plugin_ep_kernel_registry/kernels/binary_op.cc:38
- Typo in the comment. The word 'implementations' should be singular 'implementation' when preceded by 'an actual' to maintain grammatical correctness.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fs-eire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the added APIs.
onnxruntime/test/autoep/library/example_plugin_ep_kernel_registry/kernels/binary_op.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
onnxruntime/test/autoep/library/example_plugin_ep_kernel_registry/kernels/binary_op.cc:136
- The comment says "pre-pack mul initializers" but this kernel now handles both Mul and Add operators. The comment should be updated to say "pre-pack initializers" or "pre-pack mul/add initializers".
onnxruntime/test/autoep/library/example_plugin_ep_kernel_registry/kernels/binary_op.cc:163 - This cast assumes GetEp() returns a valid ExampleKernelEp pointer without any type checking. If GetEp() returns a different plugin EP type (unlikely in this test scenario but possible in general usage), this cast would be unsafe and could lead to undefined behavior. Consider adding a safety check or documentation that this kernel implementation is only compatible with ExampleKernelEp.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
onnxruntime/test/autoep/library/example_plugin_ep_kernel_registry/kernels/binary_op.cc:54
- The ONNX domain is represented as an empty string (""), not "ai.onnx". The condition
op_domain != "ai.onnx"will always evaluate to true for ONNX operators (which have domain ""), causing the validation to incorrectly reject valid Mul and Sub operators from the ONNX domain. This should be checking against an empty string instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Adds new
OrtKernelInfoAPIs to enable plugin EP kernel implementations to retrieve operator metadata and access the parent EP instance.KernelInfo_GetOperatorDomain(),KernelInfo_GetOperatorType(),KernelInfo_GetOperatorSinceVersion(), andKernelInfo_GetEp()Motivation and Context