Skip to content

Add onnxruntime_DISABLE_DEVICE_DISCOVERY to skip platform device probing - #32592

Merged
Edward Chen (edgchen1) merged 1 commit into
microsoft:mainfrom
rvandermeulen:device-discovery-option
Oct 5, 2026
Merged

Edward Chen (edgchen1) merged 1 commit into
microsoft:mainfrom
rvandermeulen:device-discovery-option

Conversation

@rvandermeulen

@rvandermeulen Ryan VanderMeulen (rvandermeulen) commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

New CMake option onnxruntime_DISABLE_DEVICE_DISCOVERY (default OFF). When set, onnxruntime_common compiles device_discovery_default.cc, which reports only the CPU device from CPUID, instead of the platform-specific discovery file. No C++ changes; the default implementation already exists for platforms without a probe. The Linux PCI/NPU discovery unit tests are skipped in that configuration, since the helpers they exercise live in the platform file; the generic DeviceDiscoveryTest still runs.

Motivation and Context

The platform device discovery on Windows opens DXGI/D3D12/DXCore and SetupDi, and Linux/macOS have their own probes. A CPU-only embedding gains nothing from them and would rather not load those system libraries at all. Firefox builds ONNX Runtime this way and has hit crashes in the DXGI path on display-less Windows CI workers; it currently carries a local patch that disables the Windows probe.

Copilot AI balanced review requested due to automatic review settings September 14, 2026 18:56
@azure-pipelines

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

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.

🟡 Changes recommended

Linux test builds fail to link because platform-specific discovery tests remain enabled after their implementation is removed.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an opt-out for platform hardware probing, using CPU-only device discovery instead.

Changes:

  • Adds onnxruntime_DISABLE_DEVICE_DISCOVERY.
  • Selects the default CPU-only discovery implementation when enabled.
File summaries
File Description
cmake/CMakeLists.txt Declares the new CMake option.
cmake/onnxruntime_common.cmake Selects the CPU-only discovery source.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmake/onnxruntime_common.cmake
Comment thread cmake/CMakeLists.txt

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.

🟢 Approval recommended

The source selection and corresponding test exclusion are consistent, with no unresolved correctness issues found.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@rvandermeulen

Copy link
Copy Markdown
Contributor Author

Edward Chen (@edgchen1) This is one of the related PRs from #32597, if you have a chance to take a look.

Edward Chen (edgchen1) pushed a commit that referenced this pull request Sep 28, 2026
…-minimal build (#32597)

### Description
Convert the raw `try`/`catch`/`throw` statements that a CPU-only,
non-minimal build still compiles to the `ORT_TRY` / `ORT_CATCH` /
`ORT_HANDLE_EXCEPTION` / `ORT_RETHROW` / `ORT_THROW` macros the rest of
the code base uses: `graph_partitioner`, `attention_fusion`,
`constant_folding`, `stft_decomposition`, `model_package_context`,
`ep_library_plugin`, `provider_bridge_ort`, and the `ORT_ENABLE_STREAM`
config reader/writer in `allocation_planner` (whose writer already used
`ORT_CATCH` but read the exception outside `ORT_HANDLE_EXCEPTION`).

`provider_bridge_ort.cc`'s 18 function-try-blocks have no macro
equivalent, so each becomes an `ORT_TRY` over the body followed by a
file-local `ORT_CATCH_LOG_RETURN_NULLPTR` that logs and returns
`nullptr`, the tail they all shared. `ProviderLibrary::Load`/`Get` keep
their unload-and-rethrow via `ORT_RETHROW`. The `allocation_planner`
handler that used `EXIT_ON_ERR` is expanded inline so its `return` stays
in the function rather than inside the handler lambda.

With exceptions enabled this is a spelling change with identical control
flow. With `onnxruntime_DISABLE_EXCEPTIONS` the result compiles and
links on Linux x64/aarch64, Windows x64/x86/arm64, macOS x64/arm64 and
Android arm/arm64/x64 (Firefox's CI, with #32591, #32592, #32593 and
#32595 applied alongside).

### Motivation and Context
Firefox builds ONNX Runtime with exceptions disabled and carries these
conversions as local patches, together with a hack that disables
`ORT_ENABLE_STREAM` only because of the `allocation_planner` sites.
Together with the other open PRs this leaves the
`onnxruntime_DISABLE_EXCEPTIONS` + `onnxruntime_MINIMAL_BUILD` coupling
as the only remaining obstacle to a non-minimal no-exceptions build.
Comment thread cmake/onnxruntime_unittests.cmake Outdated
The platform device discovery on Windows opens DXGI/D3D12/DXCore and SetupDi, and Linux/macOS have their own probes. A CPU-only embedding (Firefox builds ONNX Runtime this way) gains nothing from them and would rather not load those system libraries, and Firefox's CI has hit crashes in the DXGI path on display-less Windows workers.

With the option on, the common library compiles device_discovery_default.cc, which reports only the CPU device from CPUID, instead of the platform file. No source changes; the default implementation already exists for platforms without a probe. The Linux PCI/NPU discovery unit tests are skipped in that configuration since the helpers they test are in the platform file.
This was referenced Oct 13, 2026
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.

3 participants