Skip to content

Conversation

@apwojcik
Copy link
Contributor

After cherry-picking from win-onnxruntime (#25481), the MIGraphX EP stopped compiling on the main branch.

nieubank
nieubank previously approved these changes Jul 23, 2025
@TedThemistokleous
Copy link
Contributor

TedThemistokleous commented Jul 24, 2025

This is still failing unit tests on Linux side . Tried a build with main + this fix as part of some other integration and UTs are flagging this as a fail. There's a gap in CI here between Linux builds for some reason.

image

@TedThemistokleous
Copy link
Contributor

Weird, running the test by hand I get this.

image

Your CI is saying this passed but it doesn't look like you're building anything at all related to ROCm, is there something cached somewhere being invoked?

image

@apwojcik
Copy link
Contributor Author

I made the test available only for Windows. A similar test for TensorRT is also available only for Windows.

@TedThemistokleous
Copy link
Contributor

Apparently CI has been removed for MIGraphX builds entirely?

#25418

@TedThemistokleous
Copy link
Contributor

I made the test available only for Windows. A similar test for TensorRT is also available only for Windows.

Doesn't change the fact that we don't have a pipeline for either OS so we don't cause mainline build breaks.

@psakhamoori
Copy link
Contributor

psakhamoori commented Jul 24, 2025

Screenshot 2025-07-24 135223

Manual validation report with this fix

@nieubank
Copy link
Contributor

Can we get an issue filed to track this not being covered in CI?

Once the checks we do have pass and given the manual validation performed, I'm okay to approve on this but we need to get CI back up and running so we don't break this unknowingly again.

@TedThemistokleous
Copy link
Contributor

TedThemistokleous commented Jul 25, 2025

Can we get an issue filed to track this not being covered in CI?

Once the checks we do have pass and given the manual validation performed, I'm okay to approve on this but we need to get CI back up and running so we don't break this unknowingly again.

@nieubank You need to discuss this with your internal security team and allocation of GPU, as well as ITARs. Thats explanation I got when asking on this PR from @snnn after seeing that CI was ripped here without a peep.

#25418

Just get back to us when this is running. We have an almalinux image thats updated every ROCm release that we actively maintain that meets your requirements. Not sure why nobody reached out, or even bothered looking, it already seems like you pull Nvidia docker images from dockerhub so more unsure why nobody bothered to even look.

https://hub.docker.com/r/rocm/dev-almalinux-8/tags

We're stopping integration until you guys decide what you're doing on your end with security. Atleast I am from the Linux side with mainline broken. Too much risk in the middle of release.

Our windows team and devs are happy to help you integrate and start changes for your needs, but I'm more surprised how this has been handled today. This is a large glaring security and program risk and we're flying blind in terms of coverage

@TedThemistokleous
Copy link
Contributor

@nieubank issue is here: #25532

@snnn snnn merged commit 87f1499 into microsoft:main Jul 29, 2025
102 of 111 checks passed
psakhamoori pushed a commit to psakhamoori/onnxruntime that referenced this pull request Aug 5, 2025
…me (microsoft#25516)

After cherry-picking from win-onnxruntime (microsoft#25481), the MIGraphX EP
stopped compiling on the main branch.
nieubank pushed a commit that referenced this pull request Aug 7, 2025
Cherry-pick MiGraphX EP fixes from upstream for rel-1.23.0

This PR cherry-picks three critical fixes for the MiGraphX Execution
Provider:

1. Fix compilation after cherry-picking from win-onnxruntime (#25516)
- Adds ORT_UNUSED_PARAMETER(num_devices) to fix unused parameter warning
   - Corrects struct usage in CreateIExecutionProvider method
   
2. Fix CreateExecutionProviderFactory with correct struct and change
vendor_id (#25625)
- Updates vendor_id from 0x1002 to 0x9999 to allow DML EP to be default
   - Ensures proper device ordering in provider_policy_context.cc

3. Update OrtEpFactory in MiGraphX EP (#25567)
   - Adds complete OrtEpFactory infrastructure for auto EP selection
   - Implements all required factory methods with noexcept specifiers
   - Sets ort_version_supported to ORT_API_VERSION
- Enables MiGraphX/AMDGPU EP integration with hardware device detection

These fixes ensure MiGraphX EP builds correctly and integrates properly
with
the ORT execution provider selection framework in the 1.23.0 release.

Cherry-picked commits:
- 87f1499
- 14ca6df  
- 131cf40

---------

Co-authored-by: Artur Wojcik <[email protected]>
Co-authored-by: Owen Zhang <[email protected]>
Co-authored-by: ozhang <[email protected]>
sanketkaleoss pushed a commit to sanketkaleoss/onnxruntime that referenced this pull request Aug 11, 2025
…me (microsoft#25516)

After cherry-picking from win-onnxruntime (microsoft#25481), the MIGraphX EP
stopped compiling on the main branch.
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.

5 participants