Skip to content

Conversation

@skottmckay
Copy link
Contributor

Description

Update OrtEpFactory in new EPs to add allocator, data transfer and stream stubs.

Motivation and Context

@skottmckay skottmckay requested review from Copilot and nieubank July 23, 2025 06:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the OrtEpFactory implementations in new execution providers (VitisAI, OpenVINO, and NvTensorRtRtx) to add stub implementations for allocator, data transfer, and stream-related functionality.

  • Adds stub implementations for CreateAllocator/ReleaseAllocator functions that return appropriate error statuses
  • Implements CreateDataTransfer stubs that return nullptr to indicate no support
  • Adds IsStreamAware and CreateSyncStreamForDevice implementations that indicate no stream support

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
vitisai_provider_factory.cc Adds allocator, data transfer, and stream stub implementations to VitisAIEpFactory
ov_factory.h Adds stub method implementations and reorganizes GetVersionImpl for OpenVINOEpPluginFactory
ov_factory.cc Updates constructor to assign the new stub function pointers
nv_provider_factory.cc Adds allocator, data transfer, and stream stub implementations to NvTensorRtRtxEpFactory
Comments suppressed due to low confidence (1)

onnxruntime/core/providers/openvino/ov_factory.h:160

  • Parameter name 'this' is inconsistent with other functions that use 'this_ptr'. Should be renamed to 'this_ptr' for consistency.
  static void ORT_API_CALL ReleaseAllocatorImpl(OrtEpFactory* this, OrtAllocator* allocator) noexcept {

@jywu-msft
Copy link
Member

build errors

/onnxruntime_src/onnxruntime/core/providers/openvino/ov_factory.h: In static member function ‘static bool onnxruntime::openvino_ep::OpenVINOEpPluginFactory::IsStreamAwareImpl(const OrtEpFactory*)’:
/onnxruntime_src/onnxruntime/core/providers/openvino/ov_factory.h:170:66: error: unused parameter ‘this_ptr’ [-Werror=unused-parameter]
170 | static bool ORT_API_CALL IsStreamAwareImpl(const OrtEpFactory* this_ptr) noexcept {
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/onnxruntime_src/onnxruntime/core/providers/openvino/ov_factory.h: In static member function ‘static OrtStatus* onnxruntime::openvino_ep::OpenVINOEpPluginFactory::CreateSyncStreamForDeviceImpl(OrtEpFactory*, const OrtMemoryDevice*, const OrtKeyValuePairs*, OrtSyncStreamImpl**)’:
/onnxruntime_src/onnxruntime/core/providers/openvino/ov_factory.h:175:87: error: unused parameter ‘memory_device’ [-Werror=unused-parameter]
175 | const OrtMemoryDevice* memory_device,
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
/onnxruntime_src/onnxruntime/core/providers/openvino/ov_factory.h:176:88: error: unused parameter ‘stream_options’ [-Werror=unused-parameter]
176 | const OrtKeyValuePairs* stream_options,
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~

@skottmckay skottmckay merged commit b8a630e into main Jul 23, 2025
113 of 119 checks passed
@skottmckay skottmckay deleted the skottmckay/UpdateOrtEpFactoryImplementations branch July 23, 2025 21:33
@snnn
Copy link
Contributor

snnn commented Jul 25, 2025

Hi there! We haven't cut the release branch for this version yet, so I'm removing the release:1.23.0 label for now to keep things tidy. Thanks so much for your contribution! We'll make sure this gets included when the release is prepared. 🤖

sanketkaleoss pushed a commit to sanketkaleoss/onnxruntime that referenced this pull request Aug 11, 2025
### Description
<!-- Describe your changes. -->
Update OrtEpFactory in new EPs to add allocator, data transfer and
stream stubs.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants