[EP API] extract common code for EP API adapter#26879
Conversation
fs-eire
force-pushed
the
fs-eire/extract-common-code
branch
2 times, most recently
from
cfa6c99 to
c1e98e9
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors common base classes used by ONNX Runtime operators to use templates instead of concrete types, enabling code reuse across different execution provider implementations. The refactoring primarily converts OpKernelInfo and OpKernelContext parameters to template parameters like KernelInfoType and KernelContextType, allowing these base classes to work with different EP API implementations (particularly for WebGPU and CUDA EPs).
Key changes include:
- Converting constructors and methods in operator base classes from using
OpKernelInfoto templatedKernelInfoType - Moving implementation code from .cc files to .h files as template methods (e.g.,
ComputePadsImpl,PrepareForComputeImpl) - Adding
templatekeyword before template member function calls on dependent types (e.g.,info.template GetAttr<T>)
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/core/providers/cpu/tensor/upsamplebase.h | Templated UpsampleBase constructor with KernelInfoType, added template keywords for GetAttr/GetAttrOrDefault calls |
| onnxruntime/core/providers/cpu/tensor/unsqueeze.h | Templated UnsqueezeBase constructor with KernelInfoType |
| onnxruntime/core/providers/cpu/tensor/transpose.h | Templated TransposeBase constructor with KernelInfoType, added template keyword for GetAttrs call |
| onnxruntime/core/providers/cpu/tensor/squeeze.h | Templated SqueezeBase constructor with KernelInfoType |
| onnxruntime/core/providers/cpu/tensor/split.h | Templated SplitBase constructor with KernelInfoType, added template keywords for GetAttrOrDefault calls |
| onnxruntime/core/providers/cpu/tensor/padbase.h | Added templated ComputePadsImpl method, templated PadBase constructor, moved ComputePadWithAxes to be static, added conditional includes for SHARED_PROVIDER |
| onnxruntime/core/providers/cpu/tensor/pad.cc | Refactored ComputePads to call templated ComputePadsImpl, removed ComputePadWithAxes (moved to header) |
| onnxruntime/core/providers/cpu/tensor/gatherbase.h | Added templated PrepareForComputeImpl method, templated GatherBase constructor, added conditional includes |
| onnxruntime/core/providers/cpu/tensor/gather.cc | Refactored PrepareForCompute to call templated PrepareForComputeImpl |
| onnxruntime/core/providers/cpu/tensor/concatbase.h | Added large templated PrepareForComputeImpl method, templated ConcatBase constructor, added template keywords |
| onnxruntime/core/providers/cpu/tensor/concat.cc | Refactored PrepareForCompute to call templated PrepareForComputeImpl |
| onnxruntime/core/providers/cpu/reduction/reduction_kernel_base.h | Templated ReduceKernelBase constructor with KernelInfoType, added template keywords |
| onnxruntime/core/providers/cpu/nn/pool_base.h | Templated PoolBase constructor, changed API from GetKernelDef().OpName() to node().OpType() |
| onnxruntime/core/providers/cpu/nn/pool_attributes.h | Templated PoolAttributes constructor, added template keywords for GetAttr calls |
| onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h | Templated ConvTransposeAttributes constructor |
| onnxruntime/core/providers/cpu/nn/conv_attributes.h | Templated ConvAttributes constructor, changed from GetAttrsAsSpan to GetAttrs with vector intermediary, added template keywords |
| onnxruntime/contrib_ops/cpu/bert/attention_base.h | Templated AttentionBase constructor with KernelInfoType, added template keywords for GetAttr/GetAttrs/GetAttrOrDefault calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fs-eire
changed the title
fs-eire
changed the title
fs-eire
force-pushed
the
fs-eire/extract-common-code
branch
from
c1e98e9 to
927b20d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::vector<int64_t> pads_attr; | ||
| if (!info.GetAttrs("pads", pads_attr).IsOK()) |
There was a problem hiding this comment.
The change from GetAttrsAsSpan to GetAttrs changes the semantics from getting a view (span) of the attribute data to getting a copy (vector). This could impact performance for large attribute arrays, as GetAttrsAsSpan was specifically designed to avoid copying large attributes. Consider whether this change is necessary or if the templated type could also support GetAttrsAsSpan.
| std::vector<int64_t> pads_attr; | |
| if (!info.GetAttrs("pads", pads_attr).IsOK()) | |
| gsl::span<const int64_t> pads_attr; | |
| if (!info.GetAttrsAsSpan<int64_t>("pads", pads_attr).IsOK()) |
There was a problem hiding this comment.
An equivalent implementation of GetAttrsAsSpan is not able to be done because ORT C API defines the behavior of function KernelInfoGetAttributeArray_float to write to the buffer that user specified instead of get the memory address that stores the data.
As a result, we need to change the code to use GetAttrs which writes to a vector. Not a big performance concern because the length of pads are usually small enough.
|
|
||
| protected: | ||
| AttentionBase(const OpKernelInfo& info, bool require_same_hidden_size) { | ||
| template <typename KernelInfoType> |
There was a problem hiding this comment.
We should probably have some comment or documentation somewhere that explains the need for this templated type. Someone reading this code may find this confusing because normally a OrtKernelInfo can be reinterpret_cast to a onnxruntime::OpKernelInfo. However, if I'm not mistaken, this templated type is meant to be either a OrtKernelInfo or a adapter::OpKernelInfo from #26919.
I'm not sure where this documentation should exist but I think it should probably exist somewhere.
There was a problem hiding this comment.
the KernelInfoType should be either a onnxruntime::OpKernelInfo or a onnxruntime::ep::adapter::OpKernelInfo. It's a little different from using OrtKernelInfo. onnxruntime::ep::adapter::OpKernelInfo is a wrapper class of OrtKernelInfo which implement identical facade of onnxruntime::OpKernelInfo
It can be documented in include/onnxruntime/ep/README.md.
2 participants
Description
This PR generalizes the class
OpKernelInfoandOpKernelContextusing template so that the code is able to be used on a different type, which helps for future changes that supports migration of WebGPU EP and CUDA EP to the EP API implementation.Currently only applies to base classes that is reused by WebGPU EP.