Skip to content

[windows] Use is_versioned = true consistently in both Comgr::LoadLib paths. #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: amd-staging
Choose a base branch
from

Conversation

stellaraccident
Copy link
Contributor

Prior to this, RTCProgram was using is_version = true and device initialization was using false. On Windows, this was causing us to attempt to load amd_comgr_3.dll at device load and amd_comgr0605.dll for RTC programs. Obviously, there is only one DLL and it must be used consistently.

… paths.

Prior to this, RTCProgram was using is_version = true and device initialization was using false. On Windows, this was causing us to attempt to load amd_comgr_3.dll at device load and amd_comgr0605.dll for RTC programs. Obviously, there is only one DLL and it must be used consistently.
stellaraccident added a commit to ROCm/TheRock that referenced this pull request May 10, 2025
This reverts commit 4b59196.

Adds clr patch to use is_versioned consistently (upstreamed as ROCm/clr#162).
stellaraccident added a commit to ROCm/TheRock that referenced this pull request May 10, 2025
This reverts commit 4b59196.

Adds clr patch to use is_versioned consistently (upstreamed as
ROCm/clr#162).
stellaraccident pushed a commit to stellaraccident/clr that referenced this pull request May 10, 2025
@zichguan-amd zichguan-amd requested a review from chrispaquot June 26, 2025 20:58
@zichguan-amd
Copy link
Contributor

@stellaraccident please see @chrispaquot's comment:

I don't think this is correct. On Windows, hipRTC uses the versioned comgr provided by the HIP SDK (bundled with the app) for bitcode linking.
But the HIP runtime will still use the non-versioned one provided by driver installation.

@stellaraccident
Copy link
Contributor Author

@stellaraccident please see @chrispaquot's comment:

I don't think this is correct. On Windows, hipRTC uses the versioned comgr provided by the HIP SDK (bundled with the app) for bitcode linking.
But the HIP runtime will still use the non-versioned one provided by driver installation.

hipsdk is actually not the primary windows build system anymore. We should figure out how to support it, but the primary build system that is being used for shipping ROCm on windows is https://github.com/ROCm/TheRock -- and that lays the files out like Linux does, without the driver/app split. We should figure out how to support hipsdk, but we cannot block OSS patches to a proprietary part of the stack that can only be built at AMD.

@stellaraccident
Copy link
Contributor Author

@stellaraccident please see @chrispaquot's comment:

I don't think this is correct. On Windows, hipRTC uses the versioned comgr provided by the HIP SDK (bundled with the app) for bitcode linking.
But the HIP runtime will still use the non-versioned one provided by driver installation.

hipsdk is actually not the primary windows build system anymore. We should figure out how to support it, but the primary build system that is being used for shipping ROCm on windows is https://github.com/ROCm/TheRock -- and that lays the files out like Linux does, without the driver/app split. We should figure out how to support hipsdk, but we cannot block OSS patches to a proprietary part of the stack that can only be built at AMD.

(we've been going through and stripping out all cases where the assumption was made that WINDOWS = hipsdk)

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.

2 participants