AMDGPU EP: add graph-capture opt-out and fix per-model teardown lifetime by KenLagos · Pull Request #2672 · microsoft/onnxruntime-genai

KenLagos · 2026-10-05T05:40:06Z

Bring the AMDGPU (DirectX/MIGraphX umbrella) OGA path in line with the DML path for graph capture and device teardown.

Graph capture:

Keep graph capture ON by default for AMDGPU, but add a per-model opt-out via the provider option enable_graph_capture="0" (matching DML). The MIGraphX backend relies on capture being on by default.
Force capture OFF for non-decoder encoder/joiner sub-sessions (Whisper, Marian, Nemotron, Parakeet) on the AMDGPU device only, since their control-flow graphs could be incompatible with captured-graph replay. DML's existing behavior is left unchanged.

Teardown / lifetime:

Add per-model CloseAMDGPUInterface() (from Model::~Model, mirroring CloseDmlInterface) that resets the cached device allocator + init session, destroys the interface singletons, releases the OrtEnv-shared allocators, and unregisters the umbrella EP library.
Refcount the shared interface singleton so a composite that keeps several models alive (e.g. speculative decoding's target + draft) only tears the device down on the last release.
Only release the shared allocators / unregister the EP when genai owns the registration, so a host that pre-registered the library keeps it.
Clear device-backed shared initializers before teardown so their GpuMemory frees through a live allocator.
Make the AMDGPU interface non-cached in OrtGlobals::GetDeviceInterface (like DML), so a lookup after per-model teardown rebuilds a fresh instance instead of returning a dangling pointer.

Bring the AMDGPU (DirectX/MIGraphX umbrella) OGA path in line with the DML path for graph capture and device teardown. Graph capture: - Keep graph capture ON by default for AMDGPU, but add a per-model opt-out via the provider option enable_graph_capture="0" (matching DML). The MIGraphX backend relies on capture being on by default. - Force capture OFF for non-decoder encoder/joiner sub-sessions (Whisper, Marian, Nemotron, Parakeet) on the AMDGPU device only, since their control-flow graphs are incompatible with captured-graph replay. DML's existing behavior is left unchanged. Teardown / lifetime: - Add per-model CloseAMDGPUInterface() (from Model::~Model, mirroring CloseDmlInterface) that resets the cached device allocator + init session, destroys the interface singletons, releases the OrtEnv-shared allocators, and unregisters the umbrella EP library. - Refcount the shared interface singleton so a composite that keeps several models alive (e.g. speculative decoding's target + draft) only tears the device down on the last release. - Only release the shared allocators / unregister the EP when genai owns the registration, so a host that pre-registered the library keeps it. - Clear device-backed shared initializers before teardown so their GpuMemory frees through a live allocator. - Make the AMDGPU interface non-cached in OrtGlobals::GetDeviceInterface (like DML), so a lookup after per-model teardown rebuilds a fresh instance instead of returning a dangling pointer.

azure-pipelines · 2026-10-05T05:40:15Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Copilot review overview

🟡 Changes recommended

Capture disabling, concurrent teardown, cross-device allocator reuse, and registration ownership across shutdown have unresolved correctness issues.

Review effort: Balanced
Findings: 2 · 2

Open (4)

Synchronize model lifetime and shared AMDGPU state access · New
Reset AMDGPU registration ownership after failed model construction · New
Apply graph capture opt-out to both execution providers · New
Prevent allocator reuse across incompatible selected devices · New

What changed in this PR

Updates ONNX Runtime GenAI’s AMDGPU integration to control graph capture and manage device resources across model lifetimes.

Changes:

Adds capture opt-out and non-decoder session exclusions.
Adds reference-counted teardown with registration ownership tracking.
Avoids caching AMDGPU interface pointers across teardown.

File	Description
src/models/whisper.cpp	Requests capture disabling for AMDGPU encoders.
src/models/parakeet.cpp	Requests capture disabling for encoders and joiners.
src/models/nemotron_speech.cpp	Requests capture disabling for encoders and joiners.
src/models/model.cpp	Acquires and releases shared AMDGPU resources.
src/models/marian.cpp	Requests capture disabling for AMDGPU encoders.
src/generator/generators.cpp	Fetches AMDGPU interfaces without caching.
src/ep/amdgpu/session_options.h	Declares owned-registration cleanup.
src/ep/amdgpu/session_options.cpp	Adds capture controls and ownership-aware cleanup.
src/ep/amdgpu/interface.h	Declares shared-interface lifetime operations.
src/ep/amdgpu/interface.cpp	Implements reference counting and teardown.
src/config.cpp	Recognizes AMDGPU capture opt-out.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

- Scope the EP registration-ownership flag to the OrtEnv lifetime (move from a file-static into OrtGlobals) so it can't leak stale ownership across OgaShutdown or a failed Model construction and tear down a host's registration. - Apply the effective graph-capture decision to both backend keys, so the opt-out and non-decoder force-off also gate ep.migraphx.hip_graph_enable, not just DirectML. - Reject allocator reuse when a live model selected a different device, instead of silently binding to the wrong device's allocator. - Document the refcount's unsynchronized, serialized-lifecycle assumption (matching the DML interface).

~Model dispatched teardown by reading p_device_->GetType(). For a DML model, CloseDmlInterface() frees the singleton p_device_ points to, so the following AMDGPU-path check re-read freed memory and aborted DML teardown (hit by the DML CI tests). Snapshot the type once before any teardown and dispatch on it.

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved critical lifetime/device-selection issues and moderate sub-session capture-selection issues remain.

Review effort: Lite
Findings: 2

Open (2)

Resolve all device filters before reusing the allocator · New
Track AMDGPU usage across all model sessions · New

Resolved since last review (4)

Reset AMDGPU registration ownership after failed model construction
Synchronize model lifetime and shared AMDGPU state access
Prevent allocator reuse across incompatible selected devices
Apply graph capture opt-out to both execution providers

…fore reusing the allocator' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot review overview

🟡 Changes recommended

Device validation mutates live model state, and shutdown can lose ownership of a surviving EP registration.

Review effort: Balanced
Findings: 1

Open (1)

Rejected device load mutates the live model's device ID · New

Previously missed (1)

In code that hasn't changed since last review

GenAI-owned AMDGPU registration leaks after model construction fails

src/generator/generators.h:224

An OrtEnv can outlive OgaShutdown() when the host retains it (ep/ryzenai/interface.cpp:123–125). If automatic AMDGPU registration succeeds but the base Model constructor throws, for example while loading shared initializers, ~Model() never releases the registration. OrtGlobals teardown then discards this flag without unregistering. After re-init, the surviving registration is treated as host-owned, so later model teardown skips its allocator and EP cleanup. Release genai-owned registrations during shutdown, including after failed construction, or preserve ownership while the environment survives.

…the live model's device ID' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…struction fails ReleaseOwnedUmbrellaEp only ran from Model::~Model, so an owned AMDGPU registration leaked when the host kept the OrtEnv past OgaShutdown or a Model ctor threw. Also call it from ~OrtGlobals before env_.reset(), passing env + the flag by reference so it doesn't re-lock the held g_ort_globals_mutex and deadlock.

Copilot

Copilot review overview

🔵 Needs a closer look

Device teardown has unresolved lifetime concerns and needs hardware-backed regression validation before approval.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Destroy session options before unregistering the AMDGPU provider · New
Add AMDGPU lifecycle tests for ownership, teardown, and reload · New

Resolved since last review (1)

Rejected device load mutates the live model's device ID

…e unregistering the AMDGPU provider' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Shared GPU allocator lifetimes and EP unloading require AMDGPU-backed validation and final human review.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Destroy session options before unregistering the AMDGPU provider
Add AMDGPU lifecycle tests for ownership, teardown, and reload

… through the device interface

Copilot

Copilot review overview

🟡 Changes recommended

The new virtual method requires an interface-version increment to reject incompatible CUDA add-ons safely.

Review effort: Balanced
Findings: 1

Open (1)

Bump device interface version for new virtual method · New

…ble slot

Copilot

🔵 Needs a closer look

Shared GPU allocator teardown and EP-library unloading require human validation across both AMDGPU backends and multi-model lifetimes.

0 open findings

1 resolved since last review

Bump device interface version for new virtual method

🧠 Review effort: Balanced

KenLagos marked this pull request as ready for review October 5, 2026 05:40

Copilot AI balanced review requested due to automatic review settings October 5, 2026 05:40

Copilot started reviewing on behalf of KenLagos October 5, 2026 05:41 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread src/ep/amdgpu/interface.cpp Outdated

Comment thread src/ep/amdgpu/session_options.cpp Outdated

Comment thread src/ep/amdgpu/session_options.cpp Outdated

Comment thread src/ep/amdgpu/session_options.cpp

KenLagos marked this pull request as draft October 5, 2026 13:40

KenLagos mentioned this pull request Oct 5, 2026

Make plugin EP ABI callbacks exception-safe; fix raw-host GPU copy onnxruntime/onnxruntime-ep-amdgpu#138

Open

KenLagos requested a lite review from Copilot October 5, 2026 18:57

Copilot started reviewing on behalf of KenLagos October 5, 2026 18:58 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread src/ep/amdgpu/session_options.cpp Outdated

Comment thread src/models/model.cpp

Potential fix for pull request finding 'Resolve all device filters be…

721812c

…fore reusing the allocator' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

KenLagos marked this pull request as ready for review October 5, 2026 19:26

KenLagos requested a lite review from Copilot October 5, 2026 20:31

Copilot AI reviewed Oct 5, 2026

KenLagos requested a balanced review from Copilot October 5, 2026 21:05

Copilot started reviewing on behalf of KenLagos October 5, 2026 21:29 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread src/ep/amdgpu/session_options.cpp Outdated

Potential fix for pull request finding 'Rejected device load mutates …

0df1321

…the live model's device ID' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started reviewing on behalf of KenLagos October 5, 2026 22:13 View session

KenLagos requested a balanced review from Copilot October 5, 2026 23:03

Copilot started reviewing on behalf of KenLagos October 5, 2026 23:04 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread src/models/model.cpp Outdated

Comment thread src/ep/amdgpu/interface.cpp

Potential fix for pull request finding 'Destroy session options befor…

d06ae1b

…e unregistering the AMDGPU provider' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

KenLagos requested a balanced review from Copilot October 6, 2026 00:04

Copilot started reviewing on behalf of KenLagos October 6, 2026 00:04 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

kunal-vaishnavi reviewed Oct 6, 2026

View reviewed changes

Comment thread src/ep/amdgpu/session_options.cpp Outdated

kunal-vaishnavi reviewed Oct 6, 2026

View reviewed changes

Comment thread src/ep/amdgpu/session_options.cpp

kunal-vaishnavi reviewed Oct 6, 2026

View reviewed changes

Comment thread src/ep/amdgpu/session_options.cpp Outdated

kunal-vaishnavi reviewed Oct 6, 2026

View reviewed changes

Comment thread src/models/marian.cpp Outdated

kunal-vaishnavi requested a review from Baiju Meswani (baijumeswani) October 6, 2026 22:22

AMDGPU EP: route C-API calls through C++ wrappers and encoder capture…

8bbe76d

… through the device interface

KenLagos requested a balanced review from Copilot October 7, 2026 05:53

Copilot started reviewing on behalf of KenLagos October 7, 2026 05:53 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread src/smartptrs.h

Bump kDeviceInterfaceVersion to 8 for DisablesEncoderGraphCapture vta…

f811d16

…ble slot

KenLagos requested a balanced review from Copilot October 7, 2026 14:59

Copilot started reviewing on behalf of KenLagos October 7, 2026 15:00 View session

Merge branch 'main' into amdgpu_fix_graph_capture_and_teardown

df8cfd7

Copilot AI reviewed Oct 7, 2026

View reviewed changes

KenLagos requested a review from kunal-vaishnavi October 7, 2026 15:26

Conversation

KenLagos commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Oct 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KenLagos commented Oct 5, 2026 •

edited

Loading