Create a dedicated location for PCI vendor ID constants - #32434
Open
maxim-davgalev wants to merge 1 commit into
Open
Create a dedicated location for PCI vendor ID constants#32434maxim-davgalev wants to merge 1 commit into
maxim-davgalev wants to merge 1 commit into
Conversation
OrtDevice::VendorIds, Apple device discovery, and other core call sites previously carried duplicate PCI vendor ID constants. Add include/onnxruntime/core/common/pci_vendor_ids.h to hold the shared definitions. A vendor may have more than one PCI-SIG assignment. kAmdAti (0x1002) is the former ATI assignment used by AMD GPUs, while kAmd (0x1022) is the AMD assignment used by CPUs and Ryzen AI NPUs. Qualcomm has two assignments: kQualcommInc (0x5143) and kQualcommTechnologies (0x17CB). Qualcomm Snapdragon CPUs use the ACPI vendor identifier 'QCOM' (0x4D4F4351), not a PCI vendor ID. This value remains local to the CPU vendor table. Windows device discovery and the QNN EP use the same encoding.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
All reviewed changes consistently centralize constants without unresolved issues.
Pull request overview
Centralizes PCI vendor ID constants while preserving existing device-discovery behavior and compatibility aliases.
Changes:
- Adds a shared PCI vendor ID catalog.
- Replaces duplicated constants across core code.
- Preserves existing public vendor values.
File summaries
| File | Description |
|---|---|
onnxruntime/core/platform/windows/device_discovery.cc |
Uses the shared Microsoft vendor ID. |
onnxruntime/core/platform/linux/device_discovery.cc |
Uses the shared NVIDIA vendor ID. |
onnxruntime/core/platform/apple/device_discovery.cc |
Uses the shared Apple vendor ID. |
onnxruntime/core/common/cpuid_info_vendor.cc |
Reuses shared CPU vendor identifiers. |
include/onnxruntime/core/framework/ortdevice.h |
Maps compatibility aliases to shared constants. |
include/onnxruntime/core/common/pci_vendor_ids.h |
Defines canonical PCI vendor ID constants. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Xavier Dupré (xadupre)
approved these changes
Sep 4, 2026
Author
|
Xavier Dupré (@xadupre) The macOS arm64 Debug failed in onnxruntime4j_test only: Gradle could not resolve Maven Central ("repo.maven.apache.org: nodename nor servname provided, or not known"), so the Java tests never compiled. All C++ tests passed, and Release was only cancelled because Debug failed first. This PR does not touch the Java bindings. Could you re-run the failed jobs? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
OrtDevice::VendorIds, Apple device discovery, and other core call sites previously carried duplicate PCI vendor ID constants. Add include/onnxruntime/core/common/pci_vendor_ids.h to hold the shared definitions.
A vendor may have more than one PCI-SIG assignment. kAmdAti (0x1002) is the former ATI assignment used by AMD GPUs, while kAmd (0x1022) is the AMD assignment used by CPUs and Ryzen AI NPUs. Qualcomm has two assignments: kQualcommInc (0x5143) and kQualcommTechnologies (0x17CB).
Qualcomm Snapdragon CPUs use the ACPI vendor identifier 'QCOM' (0x4D4F4351), not a PCI vendor ID. This value remains local to the CPU vendor table. Windows device discovery and the QNN EP use the same encoding.