Skip to content

Decouple plugin execution providers (EPs) from the USE_WINML pre-processor macro - #2038

Merged
baijumeswani merged 18 commits into
mainfrom
baijumeswani/remove-winml
Mar 26, 2026
Merged

Decouple plugin execution providers (EPs) from the USE_WINML pre-processor macro#2038
baijumeswani merged 18 commits into
mainfrom
baijumeswani/remove-winml

Conversation

@baijumeswani

@baijumeswani baijumeswani commented Mar 19, 2026

Copy link
Copy Markdown
Collaborator

Previously, plugin execution providers were restricted to the WinML package because they relied on a build-time macro. This PR replaces that static check with dynamic discovery. The system now verifies if a requested provider name exists within the ONNX Runtime EP device list:

  • If found: It utilizes the V2 API for appending the execution provider.
  • If not found: It falls back to the provider-bridge logic using the legacy API.
Execution Provider Status
CPU 🟢 Pass
CUDA 🟢 Pass
WebGPU 🟢 Pass
NvTensorRtRtx 🟢 Pass
QNN 🟢 Pass
OpenVINO 🟢 Pass
VitisAI 🟢 Pass

Copilot AI review requested due to automatic review settings March 19, 2026 06:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 execution provider (EP) registration to no longer depend on the USE_WINML build-time macro, instead using runtime discovery of registered EP devices to decide whether to append via the V2 API or fall back to the legacy/provider-bridge path.

Changes:

  • Replaced USE_WINML-guarded EP selection/append logic with dynamic EP device discovery and V2 append where available.
  • Added OrtHardwareDevice / OrtEpDevice::Device() wrappers to support device-based filtering and metadata access.
  • Adjusted allocator creation to handle an ORT WebGPU memory-type rename by retrying with the legacy name.

Reviewed changes

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

File Description
src/openvino/interface.cpp Switches OpenVINO EP append logic to prefer V2 when an OrtEpDevice is found, otherwise falls back to the legacy append path.
src/models/onnxruntime_inline.h Adds inline wrappers for OrtEpDevice::Device() and OrtHardwareDevice accessors.
src/models/onnxruntime_api.h Introduces the OrtHardwareDevice wrapper type and exposes it through OrtEpDevice.
src/models/model.cpp Refactors EP append logic to use registered EP device discovery, adds device filtering for plugin EPs, and improves WebGPU allocator compatibility.

Comment thread src/openvino/interface.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated

This comment was marked as duplicate.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread src/openvino/interface.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/session_options.h Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/session_options.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread src/models/model.cpp Outdated
Comment thread src/models/session_options.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 6 out of 6 changed files in this pull request and generated 4 comments.

Comment thread src/models/session_options.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/session_options.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated
Comment thread src/models/model.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 28 out of 28 changed files in this pull request and generated 4 comments.

Comment thread src/cuda/session_options.cpp
Comment thread src/models/session_options.cpp
Comment thread src/vitisai/session_options.cpp Outdated
Comment thread src/openvino/session_options.cpp
@baijumeswani

Copy link
Copy Markdown
Collaborator Author

@copilot Please address the comments from this feedback

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

@baijumeswani I've opened a new pull request, #2041, to work on those changes. Once the pull request is ready, I'll request review from you.

…brary, add missing cctype include (#2041)

Addresses review feedback from the EP decoupling PR.

- **CUDA arena cfg use-after-free** (`src/cuda/session_options.cpp`):
`arena_cfg` was scoped inside the `if (use_arena_management)` block and
destroyed before `AppendExecutionProvider_CUDA_V2` consumed it. Moved
declaration outside the block.

- **ROCm path regression** (new `src/rocm/session_options.{h,cpp}`):
ROCm had no dispatch entry and would silently fall through to the
generic V2/V1 path, which doesn't support
`AppendExecutionProvider_ROCM`. Added a dedicated provider
implementation matching the original behavior, wired into the dispatch
table and cmake glob.

- **VitisAI LoadLibrary** (`src/vitisai/session_options.cpp`):
`LoadLibrary` result was unchecked—`GetProcAddress` would be called with
a null handle on failure. Added null-check with error, replaced C-style
cast with `reinterpret_cast`.

- **Missing `<cctype>`** (`src/openvino/session_options.cpp`):
`RemoveAllWhitespace` calls `std::isspace` without including `<cctype>`,
relying on transitive includes.

```cpp
// Before: arena_cfg destroyed before use
if (use_arena_management) {
    auto arena_cfg = OrtArenaCfg::Create(...);
    ort_provider_options->UpdateValue("default_memory_arena_cfg", arena_cfg.get());
}  // arena_cfg destroyed here
session_options.AppendExecutionProvider_CUDA_V2(*ort_provider_options);  // dangling pointer

// After: arena_cfg outlives the append call
std::unique_ptr<OrtArenaCfg> arena_cfg;
if (use_arena_management) {
    arena_cfg = OrtArenaCfg::Create(...);
    ort_provider_options->UpdateValue("default_memory_arena_cfg", arena_cfg.get());
}
session_options.AppendExecutionProvider_CUDA_V2(*ort_provider_options);
```

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 Send tasks to Copilot coding agent from
[Slack](https://gh.io/cca-slack-docs) and
[Teams](https://gh.io/cca-teams-docs) to turn conversations into code.
Copilot posts an update in your thread when it's finished.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: baijumeswani <12852605+baijumeswani@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 30 out of 30 changed files in this pull request and generated 4 comments.

Comment thread src/dml/session_options.cpp
Comment thread src/vitisai/session_options.cpp
Comment thread src/vitisai/session_options.cpp
Comment thread src/openvino/session_options.cpp Outdated
baijumeswani and others added 6 commits March 21, 2026 01:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread src/models/model.cpp
@baijumeswani
baijumeswani enabled auto-merge (squash) March 24, 2026 07:29
@baijumeswani
baijumeswani merged commit 07d21ba into main Mar 26, 2026
16 of 18 checks passed
@baijumeswani
baijumeswani deleted the baijumeswani/remove-winml branch March 26, 2026 03:32
qjia7 pushed a commit that referenced this pull request Apr 3, 2026
…essor macro (#2038)

Previously, plugin execution providers were restricted to the WinML
package because they relied on a build-time macro. This PR replaces that
static check with dynamic discovery. The system now verifies if a
requested provider name exists within the ONNX Runtime EP device list:

- If found: It utilizes the V2 API for appending the execution provider.
- If not found: It falls back to the provider-bridge logic using the
legacy API.

| Execution Provider | Status |
| :--- | :--- |
| **CPU** | 🟢 Pass |
| **CUDA** | 🟢 Pass |
| **WebGPU** | 🟢 Pass |
| **NvTensorRtRtx** | 🟢 Pass |
| **QNN** | 🟢 Pass |
| **OpenVINO** | 🟢 Pass |
| **VitisAI** | 🟢 Pass |

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: baijumeswani <12852605+baijumeswani@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
baijumeswani pushed a commit that referenced this pull request Jun 9, 2026
## Fix: Skip CPU provider in SetProviderSessionOptions

### Problem
When users call `config.append_provider('cpu')`, onnxruntime-genai
0.13.x throws:
`
RuntimeError: [ErrorCode:InvalidArgument] Unknown provider name 'cpu'.
Possible options: ...
`

This worked in v0.12.1 but regressed in v0.13.1.

### Root Cause
PR #2038 (*Decouple plugin execution providers from USE_WINML*, March
2026) introduced `src/models/session_options.cpp` with a fallthrough
path that passes unrecognized provider names to ORT's
`AppendExecutionProviderV1`. ORT's strict whitelist (added in commit
`4d03aeff0e`) rejects `cpu` since CPU EP is always implicitly registered
and never needs an explicit append call.

### Fix
In `SetProviderSessionOptions`, detect CPU provider names
(case-insensitive: `cpu`, `cpuexecutionprovider`) and:
- **If no provider options are set**: skip silently (backward-compatible
no-op)
- **If provider options are present**: throw a clear error explaining
that CPU EP does not support provider options and does not need explicit
registration

This ensures:
1. Existing code using `append_provider('cpu')` continues to work
2. Misconfigurations (setting CPU provider options that would be
silently lost) are caught early with a helpful error message

### Testing
Added regression test `TEST(CAPITests, AppendCpuProvider)` that verifies
`cpu`, `CPU`, and `CPUExecutionProvider` all succeed without error.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants