Skip to content

Move OrtEp sanity checks to plugin EP creation and remove check for OrtEp::ort_version_supported upper bound#28601

Merged
edgchen1 merged 3 commits into
mainfrom
edgchen1/plugin_ep_checks
May 21, 2026
Merged

Move OrtEp sanity checks to plugin EP creation and remove check for OrtEp::ort_version_supported upper bound#28601
edgchen1 merged 3 commits into
mainfrom
edgchen1/plugin_ep_checks

Conversation

@edgchen1
Copy link
Copy Markdown
Contributor

Description

  • Move OrtEp sanity checks from op kernel creation to EP creation.
  • Remove check for upper bound of OrtEp::ort_version_supported.
  • Add tests covering sanity checks.

Motivation and Context

Revert rejection of OrtEp instances that specify a version in ort_version_supported larger than ORT's ORT_API_VERSION. The guidance is for plugin EPs to set it to the ORT_API_VERSION value from their ORT header, but plugin EPs may optionally support earlier versions of ORT as well.

edgchen1 and others added 2 commits May 20, 2026 10:28
…ve check for upper bound of OrtEp::ort_version_supported.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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 adjusts plugin Execution Provider (EP) initialization so that basic OrtEp validation happens at EP creation time (instead of per-kernel creation), and it relaxes version validation to allow OrtEp::ort_version_supported values higher than the runtime’s ORT_API_VERSION. It also adds unit tests to verify the new sanity-check behavior and correct OrtEp lifetime handling on failure.

Changes:

  • Added early OrtEp sanity checks during PluginExecutionProviderFactory::CreatePluginExecutionProvider().
  • Removed the OrtEp::ort_version_supported <= ORT_API_VERSION upper-bound rejection (while still rejecting too-low versions).
  • Added unit tests covering sanity-check failure modes and ensuring ReleaseEp is called appropriately.

Reviewed changes

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

File Description
onnxruntime/test/framework/ep_plugin_provider_test.cc Adds a test EP factory/context and new unit tests for EP-creation sanity checks and ReleaseEp behavior.
onnxruntime/core/session/plugin_ep/ep_plugin_provider_interfaces.cc Introduces SanityCheckOrtEp() and applies it at EP creation; wraps returned OrtEp in RAII earlier.
onnxruntime/core/session/plugin_ep/ep_kernel_registration.cc Removes now-redundant OrtEp sanity checks from kernel creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@tianleiwu tianleiwu left a comment

Choose a reason for hiding this comment

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

Approve

Clean refactoring that correctly moves OrtEp validation from per-kernel creation to EP creation time. Key observations:

RAII correctness: Wrapping ort_ep_raw in UniqueOrtEp immediately after the null check and before SanityCheckOrtEp guarantees proper ReleaseEp cleanup on all failure paths. The tests verify this explicitly.

Version check relaxation: Removing the ort_version_supported <= ORT_API_VERSION upper bound is well-motivated — a plugin EP compiled against a newer ORT header may still work at runtime if it supports backward compatibility. The lower bound (>= 22) correctly rejects values that cannot represent a legitimate plugin EP.

Dropped name-consistency check: The old code verified ort_ep->GetName(ort_ep) == ep->Type(). This is no longer possible at the new call site since PluginExecutionProvider hasn't been constructed yet. This is low-risk because the name is obtained from GetName during PluginExecutionProvider construction anyway, so a name mismatch would be caught downstream. Just noting for awareness.

Tests: Good coverage of all three sanity-check branches (null GetName, GetName returns null, too-low version) plus the new accepted-high-version path. The ReleaseEp tracking in the test factory is a nice touch.

@edgchen1 edgchen1 merged commit 67156b8 into main May 21, 2026
88 checks passed
@edgchen1 edgchen1 deleted the edgchen1/plugin_ep_checks branch May 21, 2026 15:05
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